Skip to content

Fix Parenthesis Drag Crash - #34513

Open
CubikingChill wants to merge 1 commit into
musescore:4.7from
CubikingChill:parenthesis-4.7
Open

Fix Parenthesis Drag Crash#34513
CubikingChill wants to merge 1 commit into
musescore:4.7from
CubikingChill:parenthesis-4.7

Conversation

@CubikingChill

@CubikingChill CubikingChill commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Resolves: #32072

  • I signed the CLA as username: cubifbgaming
  • The title of the PR describes the problem it addresses.
  • Each commit's message describes its purpose and effects, and references the issue it resolves. If changes are extensive, there is a sequence of easily reviewable commits.
  • The code in the PR follows the coding rules.
  • I understand all aspects of the code I'm contributing and I'm able to explain it if requested.
  • The code compiles and runs on my machine, preferably after each commit individually. I have manually tested and verified that my changes fulfil their intended purpose.
  • No prior attempts to resolve this problem exist, or if they do, I listed them in my PR description and described how I avoided repeating past mistakes.
  • There are no unnecessary changes.
  • I created a unit test or vtest to verify the changes I made (if applicable).

@CubikingChill
CubikingChill force-pushed the parenthesis-4.7 branch 3 times, most recently from c6c364d to 1297555 Compare August 10, 2026 01:31
Comment on lines +1415 to +1430
const EngravingItem* dragElement = element;
if (element->type() == ElementType::PARENTHESIS) {
if (!score()) {
return;
}

muse::ContextInject<muse::ui::IUiActionsRegister> actionsRegister = { this };
const muse::actions::ActionCode code = "add-parentheses";
const muse::ui::UiAction& action = actionsRegister()->action(code);

parenthesisActionIcon = std::make_unique<ActionIcon>(score()->dummy());
parenthesisActionIcon->setActionType(ActionIconType::PARENTHESES);
parenthesisActionIcon->setAction(code, static_cast<char16_t>(action.iconCode));
engravingRenderer()->layoutItem(parenthesisActionIcon.get());
dragElement = parenthesisActionIcon.get();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an extremely strange place to be registering an action. It would be good to know the reasoning behind adding this block (and your changes generally) - how does it relate to the crash?

@CubikingChill

Copy link
Copy Markdown
Contributor Author

(I am quite busy these 2 days so I will follow-up more thoroughly later)

Yes I think I did put my fix in the wrong architectural level, I will change it soon.

The main problem that I am fixing is that the element dragged from stave is of wrong data type. So there needs to be a bridge to convert it to the right one

Regarding other parts of this PR, I know the edits in palettecell.cpp is quite awkward. But it happens that parenthesis is a standalone data type, different from other accidentals. So the edits is simply to align the naming of the custom element created, to be Accidental instead of Action icon.

I can guarantee that as of now, for this poor approach, only the edits in notation interaction.cpp is enough to prevent the crash.

@CubikingChill

Copy link
Copy Markdown
Contributor Author

Please let me know if my latest approach looks better 🙏

const String name = (element->isFretDiagram()) ? toFretDiagram(element.get())->harmonyPlainText() : element->translatedTypeUserName();
String name = (element->isFretDiagram()) ? toFretDiagram(element.get())->harmonyPlainText() : element->translatedTypeUserName();
if (element->isActionIcon() && toActionIcon(element.get())->actionType() == ActionIconType::PARENTHESES) {
name = TConv::capitalizedUserName(ElementType::ACCIDENTAL).translated();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that this part of the fix is for consistency ONLY. It is not in a right direction, in the perspective of #34514

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants