From 8e553c7497762a642cd61bcf1db055f5ef066a2a Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Mon, 3 Mar 2014 12:03:56 -0800 Subject: [PATCH] fix potential crash in setIsOptionChecked() added support for positioning separators --- interface/src/Menu.cpp | 65 ++++++++++++++++--- interface/src/Menu.h | 5 +- interface/src/MenuScriptingInterface.cpp | 6 ++ interface/src/MenuScriptingInterface.h | 1 + .../script-engine/src/MenuItemProperties.cpp | 1 + .../script-engine/src/MenuItemProperties.h | 1 + 6 files changed, 70 insertions(+), 9 deletions(-) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index dc87cefb05..8c6cdacae1 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -509,9 +509,24 @@ void Menu::handleViewFrustumOffsetKeyModifier(int key) { } } -void Menu::addDisabledActionAndSeparator(QMenu* destinationMenu, const QString& actionName) { - destinationMenu->addSeparator(); - (destinationMenu->addAction(actionName))->setEnabled(false); +void Menu::addDisabledActionAndSeparator(QMenu* destinationMenu, const QString& actionName, int menuItemLocation) { + QAction* actionBefore = NULL; + if (menuItemLocation >= 0 && destinationMenu->actions().size() > menuItemLocation) { + actionBefore = destinationMenu->actions()[menuItemLocation]; + } + if (actionBefore) { + QAction* separator = new QAction("",destinationMenu); + destinationMenu->insertAction(actionBefore, separator); + separator->setSeparator(true); + + QAction* separatorText = new QAction(actionName,destinationMenu); + separatorText->setEnabled(false); + destinationMenu->insertAction(actionBefore, separatorText); + + } else { + destinationMenu->addSeparator(); + (destinationMenu->addAction(actionName))->setEnabled(false); + } } QAction* Menu::addActionToQMenuAndActionHash(QMenu* destinationMenu, @@ -572,7 +587,10 @@ void Menu::removeAction(QMenu* menu, const QString& actionName) { } void Menu::setIsOptionChecked(const QString& menuOption, bool isChecked) { - return _actionHash.value(menuOption)->setChecked(isChecked); + QAction* menu = _actionHash.value(menuOption); + if (menu) { + menu->setChecked(isChecked); + } } bool Menu::isOptionChecked(const QString& menuOption) { @@ -1243,6 +1261,18 @@ int Menu::findPositionOfMenuItem(QMenu* menu, const QString& searchMenuItem) { return UNSPECIFIED_POSITION; // not found } +int Menu::positionBeforeSeparatorIfNeeded(QMenu* menu, int requestedPosition) { + QList menuActions = menu->actions(); + if (requestedPosition > 1 && requestedPosition < menuActions.size()) { + QAction* beforeRequested = menuActions[requestedPosition - 1]; + if (beforeRequested->isSeparator()) { + requestedPosition--; + } + } + return requestedPosition; +} + + QMenu* Menu::addMenu(const QString& menuName) { QStringList menuTree = menuName.split(">"); QMenu* addTo = NULL; @@ -1288,6 +1318,21 @@ void Menu::addSeparator(const QString& menuName, const QString& separatorName) { } } +void Menu::removeSeparator(const QString& menuName, const QString& separatorName) { + QMenu* menu = getMenu(menuName); + if (menu) { + int textAt = findPositionOfMenuItem(menu, separatorName); + QList menuActions = menu->actions(); + QAction* separatorText = menuActions[textAt]; + QAction* separatorLine = menuActions[textAt - 1]; + if (separatorLine->isSeparator()) { + menu->removeAction(separatorText); + menu->removeAction(separatorLine); + } + } + QMenuBar::repaint(); +} + void Menu::addMenuItem(const MenuItemProperties& properties) { QMenu* menuObj = getMenu(properties.menuName); if (menuObj) { @@ -1300,6 +1345,8 @@ void Menu::addMenuItem(const MenuItemProperties& properties) { int requestedPosition = properties.position; if (requestedPosition == UNSPECIFIED_POSITION && !properties.beforeItem.isEmpty()) { requestedPosition = findPositionOfMenuItem(menuObj, properties.beforeItem); + // double check that the requested location wasn't a separator label + requestedPosition = positionBeforeSeparatorIfNeeded(menuObj, requestedPosition); } if (requestedPosition == UNSPECIFIED_POSITION && !properties.afterItem.isEmpty()) { int afterPosition = findPositionOfMenuItem(menuObj, properties.afterItem); @@ -1308,9 +1355,11 @@ void Menu::addMenuItem(const MenuItemProperties& properties) { } } - QAction* menuItemAction; - if (properties.isCheckable) { - menuItemAction = addCheckableActionToQMenuAndActionHash(menuObj, properties.menuItemName, + QAction* menuItemAction = NULL; + if (properties.isSeparator) { + addDisabledActionAndSeparator(menuObj, properties.menuItemName, requestedPosition); + } else if (properties.isCheckable) { + menuItemAction = addCheckableActionToQMenuAndActionHash(menuObj, properties.menuItemName, properties.shortcutKeySequence, properties.isChecked, MenuScriptingInterface::getInstance(), SLOT(menuItemTriggered()), requestedPosition); } else { @@ -1318,7 +1367,7 @@ void Menu::addMenuItem(const MenuItemProperties& properties) { MenuScriptingInterface::getInstance(), SLOT(menuItemTriggered()), QAction::NoRole, requestedPosition); } - if (shortcut) { + if (shortcut && menuItemAction) { connect(shortcut, SIGNAL(activated()), menuItemAction, SLOT(trigger())); } QMenuBar::repaint(); diff --git a/interface/src/Menu.h b/interface/src/Menu.h index 7b4cca3b33..9d4c7eaab9 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -124,6 +124,7 @@ public slots: QMenu* addMenu(const QString& menuName); void removeMenu(const QString& menuName); void addSeparator(const QString& menuName, const QString& separatorName); + void removeSeparator(const QString& menuName, const QString& separatorName); void addMenuItem(const MenuItemProperties& properties); void removeMenuItem(const QString& menuName, const QString& menuitem); @@ -152,7 +153,8 @@ private: void scanMenu(QMenu* menu, settingsAction modifySetting, QSettings* set); /// helper method to have separators with labels that are also compatible with OS X - void addDisabledActionAndSeparator(QMenu* destinationMenu, const QString& actionName); + void addDisabledActionAndSeparator(QMenu* destinationMenu, const QString& actionName, + int menuItemLocation = UNSPECIFIED_POSITION); QAction* addCheckableActionToQMenuAndActionHash(QMenu* destinationMenu, const QString& actionName, @@ -172,6 +174,7 @@ private: QAction* getMenuAction(const QString& menuName); int findPositionOfMenuItem(QMenu* menu, const QString& searchMenuItem); + int positionBeforeSeparatorIfNeeded(QMenu* menu, int requestedPosition); QMenu* getMenu(const QString& menuName); diff --git a/interface/src/MenuScriptingInterface.cpp b/interface/src/MenuScriptingInterface.cpp index f1a44422b3..ccb3fe5eb9 100644 --- a/interface/src/MenuScriptingInterface.cpp +++ b/interface/src/MenuScriptingInterface.cpp @@ -38,6 +38,12 @@ void MenuScriptingInterface::addSeparator(const QString& menuName, const QString Q_ARG(const QString&, separatorName)); } +void MenuScriptingInterface::removeSeparator(const QString& menuName, const QString& separatorName) { + QMetaObject::invokeMethod(Menu::getInstance(), "removeSeparator", + Q_ARG(const QString&, menuName), + Q_ARG(const QString&, separatorName)); +} + void MenuScriptingInterface::addMenuItem(const MenuItemProperties& properties) { QMetaObject::invokeMethod(Menu::getInstance(), "addMenuItem", Q_ARG(const MenuItemProperties&, properties)); } diff --git a/interface/src/MenuScriptingInterface.h b/interface/src/MenuScriptingInterface.h index 7379f2c00b..a4496e3054 100644 --- a/interface/src/MenuScriptingInterface.h +++ b/interface/src/MenuScriptingInterface.h @@ -32,6 +32,7 @@ public slots: void removeMenu(const QString& menuName); void addSeparator(const QString& menuName, const QString& separatorName); + void removeSeparator(const QString& menuName, const QString& separatorName); void addMenuItem(const MenuItemProperties& properties); void addMenuItem(const QString& menuName, const QString& menuitem, const QString& shortcutKey); diff --git a/libraries/script-engine/src/MenuItemProperties.cpp b/libraries/script-engine/src/MenuItemProperties.cpp index fc1e94f40c..f071723233 100644 --- a/libraries/script-engine/src/MenuItemProperties.cpp +++ b/libraries/script-engine/src/MenuItemProperties.cpp @@ -71,6 +71,7 @@ void menuItemPropertiesFromScriptValue(const QScriptValue& object, MenuItemPrope properties.menuItemName = object.property("menuItemName").toVariant().toString(); properties.isCheckable = object.property("isCheckable").toVariant().toBool(); properties.isChecked = object.property("isChecked").toVariant().toBool(); + properties.isSeparator = object.property("isSeparator").toVariant().toBool(); // handle the shortcut key options in order... QScriptValue shortcutKeyValue = object.property("shortcutKey"); diff --git a/libraries/script-engine/src/MenuItemProperties.h b/libraries/script-engine/src/MenuItemProperties.h index 6e0b3309b1..82f00a2bc4 100644 --- a/libraries/script-engine/src/MenuItemProperties.h +++ b/libraries/script-engine/src/MenuItemProperties.h @@ -39,6 +39,7 @@ public: // other properties bool isCheckable; bool isChecked; + bool isSeparator; }; Q_DECLARE_METATYPE(MenuItemProperties) QScriptValue menuItemPropertiesToScriptValue(QScriptEngine* engine, const MenuItemProperties& props);