diff --git a/src/laybasic/laybasic/layAbstractMenu.cc b/src/laybasic/laybasic/layAbstractMenu.cc index 40577bbba..bf2766744 100644 --- a/src/laybasic/laybasic/layAbstractMenu.cc +++ b/src/laybasic/laybasic/layAbstractMenu.cc @@ -44,6 +44,27 @@ namespace lay { +// --------------------------------------------------------------- + +#if defined(__APPLE__) + +// On MacOS, the main menu bar and it's decendent children +// can't be modified using "removeAction", followrd by "addAction" +// to achieve a move operation.If we try to do so, segmentation faults happen +// in the timer event that presumably tries to merge the menu bar +// with the application menu. +// The fallback is to only allow add/delete, not move operations on the +// menu. In effect, the order of the menu items may not be the one desired +// if menus are dynamically created. However, this will only happen when +// new packages or macros are installed. +static const bool s_can_move_menu = false; + +#else + +static const bool s_can_move_menu = true; + +#endif + // --------------------------------------------------------------- // Helper function to parse a title with potential shortcut and // icon specification @@ -102,13 +123,13 @@ parse_menu_title (const std::string &s, std::string &title, std::string &shortcu // AbstractMenuItem implementation AbstractMenuItem::AbstractMenuItem () - : mp_menu (0), m_has_submenu (false), m_remove_on_empty (false) + : m_has_submenu (false), m_remove_on_empty (false) { // ... nothing yet .. } AbstractMenuItem::AbstractMenuItem (const AbstractMenuItem &) - : mp_menu (0), m_has_submenu (false), m_remove_on_empty (false) + : m_has_submenu (false), m_remove_on_empty (false) { // ... nothing yet .. } @@ -160,6 +181,10 @@ AbstractMenuItem::set_action (const Action &a, bool copy_properties) m_action.set_visible (visible); m_action.set_object_name (m_basename); + + if (m_action.menu ()) { + m_action.menu ()->setObjectName (tl::to_qstring (m_basename)); + } } void @@ -180,15 +205,6 @@ AbstractMenuItem::set_remove_on_empty () m_remove_on_empty = true; } -void -AbstractMenuItem::set_menu (QMenu *menu) -{ - mp_menu = menu; - if (mp_menu) { - mp_menu->setObjectName (tl::to_qstring (m_basename)); - } -} - // --------------------------------------------------------------- // Action implementation @@ -238,7 +254,8 @@ public: }; ActionHandle::ActionHandle (QWidget *parent) - : mp_action (new ActionObject (parent)), + : mp_menu (0), + mp_action (new ActionObject (parent)), m_ref_count (0), m_owned (true), m_visible (true), @@ -254,7 +271,8 @@ ActionHandle::ActionHandle (QWidget *parent) } ActionHandle::ActionHandle (QAction *action, bool owned) - : mp_action (action), + : mp_menu (0), + mp_action (action), m_ref_count (0), m_owned (owned), m_visible (true), @@ -269,6 +287,23 @@ ActionHandle::ActionHandle (QAction *action, bool owned) connect (mp_action, SIGNAL (destroyed (QObject *)), this, SLOT (destroyed (QObject *))); } +ActionHandle::ActionHandle (QMenu *menu, bool owned) + : mp_menu (menu), + mp_action (menu->menuAction ()), + m_ref_count (0), + m_owned (owned), + m_visible (true), + m_hidden (false) +{ + if (! sp_actionHandles) { + sp_actionHandles = new std::set (); + } + sp_actionHandles->insert (this); + + // catch the destroyed signal to tell if the QAction object is deleted. + connect (mp_action, SIGNAL (destroyed (QObject *)), this, SLOT (destroyed (QObject *))); +} + ActionHandle::~ActionHandle () { if (sp_actionHandles) { @@ -279,7 +314,14 @@ ActionHandle::~ActionHandle () } } - if (mp_action) { + if (mp_menu) { + if (m_owned) { + delete mp_menu; + m_owned = false; + } + mp_menu = 0; + mp_action = 0; + } else if (mp_action) { if (m_owned) { delete mp_action; m_owned = false; @@ -308,6 +350,12 @@ ActionHandle::ptr () const return mp_action; } +QMenu * +ActionHandle::menu () const +{ + return mp_menu; +} + void ActionHandle::destroyed (QObject * /*obj*/) { @@ -582,6 +630,12 @@ Action::qaction () const return mp_handle ? mp_handle->ptr () : 0; } +QMenu * +Action::menu () const +{ + return mp_handle ? mp_handle->menu () : 0; +} + void Action::add_to_exclusive_group (lay::AbstractMenu *menu, const std::string &group_name) { @@ -870,7 +924,7 @@ AbstractMenu::AbstractMenu (AbstractMenuProvider *provider) AbstractMenu::~AbstractMenu () { - reset_menu_objects (m_root); + // .. nothing yet .. } void @@ -880,15 +934,6 @@ AbstractMenu::init (const MenuLayoutEntry *layout) transfer (layout, m_root); } -void -AbstractMenu::reset_menu_objects (AbstractMenuItem &item) -{ - for (std::list::iterator c = item.children.begin (); c != item.children.end (); ++c) { - reset_menu_objects (*c); - } - item.set_menu (0); -} - QActionGroup * AbstractMenu::make_exclusive_group (const std::string &name) { @@ -935,10 +980,9 @@ AbstractMenu::build_detached (const std::string &name, QFrame *mbar) menu_button->setText (tl::to_qstring (c->action ().get_title ())); if (c->menu () == 0) { - QMenu *menu = new QMenu (mbar); + QMenu *menu = new QMenu (); menu_button->setMenu (menu); - c->set_menu (menu); - c->set_action (Action (new ActionHandle (menu->menuAction (), false)), true); + c->set_action (Action (new ActionHandle (menu)), true); } else { menu_button->setMenu (c->menu ()); } @@ -967,9 +1011,12 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar) tl_assert (mp_provider != 0); m_helper_menu_items.clear (); - mbar->clear (); tbar->clear (); + std::set present_actions; + QList a = mbar->actions (); + present_actions.insert (a.begin (), a.end ()); + for (std::list::iterator c = m_root.children.begin (); c != m_root.children.end (); ++c) { if (c->has_submenu ()) { @@ -985,14 +1032,13 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar) } else if (c->name ().find ("@") == 0) { if (c->menu () == 0) { - QMenu *menu = new QMenu (tl::to_qstring (c->action ().get_title ()), mp_provider->menu_parent_widget ()); + QMenu *menu = new QMenu (tl::to_qstring (c->action ().get_title ())); // 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. mp_provider->menu_parent_widget ()->addAction (menu->menuAction ()); - c->set_menu (menu); - c->set_action (Action (new ActionHandle (c->menu ()->menuAction (), false)), true); + c->set_action (Action (new ActionHandle (menu)), true); } // prepare a detached menu which can be used as context menues @@ -1001,10 +1047,22 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar) } else { if (c->menu () == 0) { - c->set_menu (mbar->addMenu (tl::to_qstring (c->action ().get_title ()))); - c->set_action (Action (new ActionHandle (c->menu ()->menuAction (), false)), true); + QMenu *menu = new QMenu (); + menu->setTitle (tl::to_qstring (c->action ().get_title ())); + mbar->addMenu (menu); + c->set_action (Action (new ActionHandle (menu)), true); } else { - mbar->addMenu (c->menu ()); + // Move the action to the end if present in the menu already + std::set::iterator a = present_actions.find (c->menu ()->menuAction ()); + if (a != present_actions.end ()) { + if (s_can_move_menu) { + mbar->removeAction (*a); + mbar->addMenu (c->menu ()); + } + present_actions.erase (*a); + } else { + mbar->addMenu (c->menu ()); + } } build (c->menu (), c->children); @@ -1012,30 +1070,82 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar) } } else { - mbar->addAction (c->action ().qaction ()); + // Move the action to the end if present in the menu already + std::set::iterator a = present_actions.find (c->action ().qaction ()); + if (a != present_actions.end ()) { + if (s_can_move_menu) { + mbar->removeAction (*a); + mbar->addAction (c->action ().qaction ()); + } + present_actions.erase (*a); + } else { + mbar->addAction (c->action ().qaction ()); + } } } + + // Remove all actions that have vanished + for (std::set::iterator a = present_actions.begin (); a != present_actions.end (); ++a) { + mbar->removeAction (*a); + } } void AbstractMenu::build (QMenu *m, std::list &items) { - m->clear (); + std::set present_actions; + QList a = m->actions (); + present_actions.insert (a.begin (), a.end ()); for (std::list::iterator c = items.begin (); c != items.end (); ++c) { + if (c->has_submenu ()) { - // HINT: the action acts as a container for the title. Unfortunately, we cannot create a - // menu with a given action. The action is provided by addMenu instead. - QMenu *menu = m->addMenu (tl::to_qstring (c->action ().get_title ())); - c->set_action (Action (new ActionHandle (menu->menuAction (), false)), true); - // HINT: build must be done before set_menu because set_menu might delete all child QAction's .. - build (menu, c->children); - c->set_menu (menu); + + if (! c->menu ()) { + // HINT: the action acts as a container for the title. Unfortunately, we cannot create a + // menu with a given action. The action is provided by addMenu instead. + QMenu *menu = new QMenu (); + menu->setTitle (tl::to_qstring (c->action ().get_title ())); + m->addMenu (menu); + c->set_action (Action (new ActionHandle (menu)), true); + } else { + // Move the action to the end if present in the menu already + std::set::iterator a = present_actions.find (c->menu ()->menuAction ()); + if (a != present_actions.end ()) { + if (s_can_move_menu) { + m->removeAction (*a); + m->addMenu (c->menu ()); + } + present_actions.erase (*a); + } else { + m->addMenu (c->menu ()); + } + } + + build (c->menu (), c->children); + } else { - m->addAction (c->action ().qaction ()); + + // Move the action to the end if present in the menu already + std::set::iterator a = present_actions.find (c->action ().qaction ()); + if (a != present_actions.end ()) { + if (s_can_move_menu) { + m->removeAction (*a); + m->addAction (c->action ().qaction ()); + } + present_actions.erase (*a); + } else { + m->addAction (c->action ().qaction ()); + } + } } + + // Remove all actions that have vanished + for (std::set::iterator a = present_actions.begin (); a != present_actions.end (); ++a) { + m->removeAction (*a); + } } void @@ -1236,7 +1346,6 @@ AbstractMenu::delete_item (const std::string &p) break; } - reset_menu_objects (*p->second); p->first->children.erase (p->second); } diff --git a/src/laybasic/laybasic/layAbstractMenu.h b/src/laybasic/laybasic/layAbstractMenu.h index ecbf9091f..394c6b840 100644 --- a/src/laybasic/laybasic/layAbstractMenu.h +++ b/src/laybasic/laybasic/layAbstractMenu.h @@ -66,10 +66,12 @@ Q_OBJECT public: ActionHandle (QWidget *parent); ActionHandle (QAction *action, bool owned = true); + ActionHandle (QMenu *menu, bool owned = true); ~ActionHandle (); void add_ref (); void remove_ref (); QAction *ptr () const; + QMenu *menu () const; void set_visible (bool v); void set_hidden (bool h); @@ -87,6 +89,7 @@ protected slots: void destroyed (QObject *obj); private: + QMenu *mp_menu; QAction *mp_action; int m_ref_count; bool m_owned; @@ -344,6 +347,11 @@ public: */ QAction *qaction () const; + /** + * @brief Gets the QMenu object if the action is a menu action + */ + QMenu *menu () const; + /** * @brief Compares two action objects * @@ -565,11 +573,9 @@ struct LAYBASIC_PUBLIC AbstractMenuItem return m_action; } - void set_menu (QMenu *menu); - QMenu *menu () const { - return mp_menu; + return m_action.menu (); } void set_has_submenu (); @@ -589,7 +595,6 @@ struct LAYBASIC_PUBLIC AbstractMenuItem std::list children; private: - QMenu *mp_menu; bool m_has_submenu; bool m_remove_on_empty; Action m_action; @@ -845,7 +850,6 @@ private: void build (QMenu *menu, std::list &items); void build (QToolBar *tbar, std::list &items); void collect_group (std::vector &grp, const std::string &name, const AbstractMenuItem &item) const; - void reset_menu_objects (AbstractMenuItem &item); /** * @brief Create a action from a string