Crash on cursor key after entering hairpin

• Jul 29, 2014 - 16:25
Type
Functional
Severity
S2 - Critical
Status
closed
Project

Ubuntu Studio 14.04, GIT commit: ccb04e0

1) create score, flute, 4/4
2) enter 4 quarter ntoes into first measure
3) exit note entry
4) click first note
5) press "<" to enter hairpin
6) press Right arrow

Result: crash


Comments

The problem is that immediately after adding a hairpin, the current selection is the hairpin itself, which has no parent, so we dereference a null pointer here:

https://github.com/musescore/MuseScore/blob/f9fb8bf7d604a4fd3d47dae1283…

The *easy* fix is to check to just return null immediately if el->parent() is null. But that left me wondering why it doesn't crash if you select a hairpin later on and try the cursor keys. Turns out that selecting a hairpin normally actually selects just the HairpinSegment, and it has a System for a parent. So that's why there is no crash.

If we implement the easy fix, the crash goes away, but we're left with a behavior (also in current code) that I think is a (minor) bug in itself: after pressing "<" to add the hairpin and then pressing "Esc", the hairpin appears to remain selected, and even appears to remain selected if you click an empty part of the score. It isn't actually selected, though - pressing Delete doesn't delete it. And who knows what other problems may be lurking as a side effect as well.

I guess you can't force a hairpin segment to be selected immediately upon adding it because it hasn't been laid out yet. So the alternate fix I propose is to not select the hairpin at all upon adding it (leaving the note or notes selected). In other words, remove this:

https://github.com/musescore/MuseScore/blob/f9fb8bf7d604a4fd3d47dae1283…

That's actually more consistent with what happens upon entering a slur. Although the slur is immediately put into Edit mode if "S" is pressed with only a single note selected, and I wonder if the same couldn't be done for hairpins. But that's a bigger change.

So I'm submitting a PR that does the check for null parent (just in case) but also skips selecting the hairpin in the first place. If you like that approach, feel free to merge it as is, but no worries if you have a better fix in mind.