Fixed a potential crash

This bug was triggered during development and happened
because the tool bar build code was invalidating
QAction objects indirectly while they are used for
building the main menu.

Solution is to separate building of menu bar and
tool bar / context menus.
This commit is contained in:
Matthias Koefferlein 2026-05-24 14:09:08 +02:00
parent cb918203c7
commit 4cc1d2ff9a
1 changed files with 121 additions and 103 deletions

View File

@ -1280,20 +1280,16 @@ AbstractMenu::extra_menu_name ()
void void
AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar) AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
{ {
if (tbar) { // Build the menu bar
tbar->clear (); if (mbar) {
}
std::set<std::pair<size_t, QAction *> > present_actions; std::set<std::pair<size_t, QAction *> > present_actions;
if (mbar) {
QList<QAction *> a = mbar->actions (); QList<QAction *> a = mbar->actions ();
for (QList<QAction *>::const_iterator i = a.begin (); i != a.end (); ++i) { for (QList<QAction *>::const_iterator i = a.begin (); i != a.end (); ++i) {
present_actions.insert (std::make_pair (id_from_action (*i), *i)); present_actions.insert (std::make_pair (id_from_action (*i), *i));
} }
}
// NOTE: on MacOS, once the top level menu is created, we should not // NOTE: on MacOS, once the top level menu is created, we should not
// modify it and place menu items into the "Extras" menu instead. // modify it and place menu items into the "Extras" menu instead.
bool menu_frozen = ! s_can_move_menu && ! present_actions.empty (); bool menu_frozen = ! s_can_move_menu && ! present_actions.empty ();
@ -1310,35 +1306,11 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
if (c->has_submenu ()) { if (c->has_submenu ()) {
if (c->name () == "@toolbar") { if (c->name ().find ("@") == 0) {
if (tbar) { // done later.
build (tbar, c->children);
}
} else if (c->name ().find ("@@") == 0) { } else {
// nothing: let build_detached build the menu
} else if (c->name ().find ("@") == 0) {
if (c->menu () == 0) {
QMenu *menu = new QMenu (tl::to_qstring (c->action ()->get_title ()), mp_dispatcher->menu_parent_widget ());
c->action ()->set_menu (menu, true);
}
// HINT: it is necessary to add the menu action to a widget below the main window.
// Otherwise, the keyboard shortcuts do not work for menu items inside such a
// popup menu. It seems not to have a negative effect to add the menu to the
// main widget.
if (mp_dispatcher->menu_parent_widget ()) {
mp_dispatcher->menu_parent_widget ()->addAction (c->menu ()->menuAction ());
}
// prepare a detached menu which can be used as context menus
build (c->menu (), c->children);
} else if (mbar) {
if (c->menu () == 0) { if (c->menu () == 0) {
@ -1388,7 +1360,7 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
} }
} else if (mbar) { } else {
// Move the action to the end if present in the menu already // Move the action to the end if present in the menu already
std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.find (std::make_pair (id_from_action (c->action ()->qaction ()), c->action ()->qaction ())); std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.find (std::make_pair (id_from_action (c->action ()->qaction ()), c->action ()->qaction ()));
@ -1419,7 +1391,6 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
} }
// Remove all actions that have vanished // Remove all actions that have vanished
if (mbar) {
for (std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.begin (); a != present_actions.end (); ++a) { for (std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.begin (); a != present_actions.end (); ++a) {
if (s_can_move_menu) { if (s_can_move_menu) {
mbar->removeAction (a->second); mbar->removeAction (a->second);
@ -1431,6 +1402,53 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
a->second->setEnabled (false); a->second->setEnabled (false);
} }
} }
}
// Finally build the context menus and toolbar.
// NOTE: this is done afterwards, as generating the submenus, specifically for the toolbar menu buttons
// messes with the QActions stored in "present_actions".
if (tbar) {
tbar->clear ();
}
for (std::list<AbstractMenuItem>::iterator c = m_root.children.begin (); c != m_root.children.end (); ++c) {
if (c->has_submenu ()) {
if (c->name () == "@toolbar") {
if (tbar) {
build (tbar, c->children);
}
} else if (c->name ().find ("@@") == 0) {
// nothing: let build_detached build the menu
} else if (c->name ().find ("@") == 0) {
if (c->menu () == 0) {
QMenu *menu = new QMenu (tl::to_qstring (c->action ()->get_title ()), mp_dispatcher->menu_parent_widget ());
c->action ()->set_menu (menu, true);
}
// HINT: it is necessary to add the menu action to a widget below the main window.
// Otherwise, the keyboard shortcuts do not work for menu items inside such a
// popup menu. It seems not to have a negative effect to add the menu to the
// main widget.
if (mp_dispatcher->menu_parent_widget ()) {
mp_dispatcher->menu_parent_widget ()->addAction (c->menu ()->menuAction ());
}
// prepare a detached menu which can be used as context menus
build (c->menu (), c->children);
}
}
} }
} }