Gui: fix shortcut context handling in ShortcutManager

Related #6097

Qt ignores shortcut of actions in invisible toolbar, but not for actions
in a hidden menu action of menu bar, which is likely a Qt bug. The
desired behavior should be that of toolbar actions, so that actions
belong to different workbenches can have the same shortcut without
conflict.

This commit works around this inconsistency by ensuring only the active
actions are added in menu bar. In addition, all active actions will be
added to a zero sized child widget of the main window, which ensures the
shortcuts of these actions being active regardless whether the action is
in toolbar or menu bar, visible or not.

src/Base/ConsoleObserver.cpp

@@ -34,6 +34,7 @@
 #include <frameobject.h>
 
 #include "ConsoleObserver.h"
+#include "Interpreter.h"
 
 
 using namespace Base;
@@ -261,6 +262,7 @@ std::stringstream &LogLevel::prefix(std::stringstream &str, const char *src, int
     }
     if (print_tag) str << '<' << tag << "> ";
     if (print_src==2) {
+        Base::PyGILStateLocker lock;
         PyFrameObject* frame = PyEval_GetFrame();
         if (frame) {
             line = PyFrame_GetLineNumber(frame);

src/Gui/MenuManager.cpp

@@ -209,6 +209,7 @@ void MenuManager::setup(MenuItem* menuItems) const
 
     QMenuBar* menuBar = getMainWindow()->menuBar();
 
+#if 0
 #if defined(FC_OS_MACOSX) && QT_VERSION >= 0x050900
     // Unknown Qt macOS bug observed with Qt >= 5.9.4 causes random crashes when viewing reused top level menus.
     menuBar->clear();
@@ -222,6 +223,31 @@ void MenuManager::setup(MenuItem* menuItems) const
         ("User parameter:BaseApp/Preferences/MainWindow")->GetBool("ClearMenuBar",false)) {
         menuBar->clear();
     }
+#else
+    // In addition to the reason described in the above comments, there is
+    // another more subtle one that's making clearing menu bar a necessity for
+    // all platforms.
+    //
+    // By right, it should be fine for more than one command action having the
+    // same shortcut but in different workbench. It should not require manual
+    // conflict resolving in this case, as the action in an inactive workbench
+    // is expected to be inactive as well, or else user may experience
+    // seemingly random shortcut miss firing based on the order he/she
+    // switches workbenches. In fact, this may be considered as an otherwise
+    // difficult to implement feature of context aware shortcut, where a
+    // specific shortcut can activate different actions under different
+    // workbenches.
+    //
+    // This works as expected for action adding to a toolbar. As Qt will ignore
+    // actions inside an invisible toolbar.  However, Qt refuse to do the same
+    // for actions in a hidden menu action of a menu bar. This is very likely a
+    // Qt bug, as the behavior does not seem to conform to Qt's documentation
+    // of Qt::ShortcutContext.
+    //
+    // Clearing the menu bar, and recreate it every time when switching
+    // workbench with only the active actions can solve this problem.
+    menuBar->clear();
+#endif
 
     QList<MenuItem*> items = menuItems->getItems();
     QList<QAction*> actions = menuBar->actions();

src/Gui/ShortcutManager.cpp

@@ -203,37 +203,35 @@ bool ShortcutManager::checkShortcut(QObject *o, const QKeySequence &key)
     if (iter == index.end())
         return false;
 
-    auto it = iter;
-    // skip to the first not exact matched key
-    for (; it != index.end() && key == it->key.shortcut; ++it);
+    // disable and enqueue the action in order to try other alternativeslll
+    action->setEnabled(false);
+    pendingActions.emplace_back(action, key.count(), 0);
 
     // check for potential partial match, i.e. longer key sequences
     bool flush = true;
-    for (; it != index.end(); ++it) {
+    bool found = false;
+    for (auto it = iter; it != index.end(); ++it) {
         if (key.matches(it->key.shortcut) == QKeySequence::NoMatch)
             break;
-        if (it->action && it->action->isEnabled()) {
+        if (action == it->action) {
+            // There maybe more than one action with the exact same shortcut.
+            // However, we only disable and enqueue the triggered action.
+            // Because, QAction::isEnabled() does not check if the action is
+            // active under its current ShortcutContext. We would have to check
+            // its parent widgets visibility which may or may not be reliable.
+            // Instead, we rely on QEvent::Shortcut to be sure to enqueue only
+            // active shortcuts. We'll fake the current key sequence below,
+            // which will trigger all possible matches one by one.
+            pendingActions.back().priority = getPriority(it->key.name);
+            found = true;
+        }
+        else if (it->action && it->action->isEnabled()) {
             flush = false;
-            break;
+            if (found)
+                break;
         }
     }
 
-    int count = 0;
-    for (it = iter; it != index.end() && key == it->key.shortcut; ++it) {
-        if (it->action && it->action->isEnabled()) {
-            if (!flush) {
-                // temporary disable the action so that we can try potential
-                // match with further keystrokes.
-                it->action->setEnabled(false);
-            }
-            pendingActions.emplace_back(it->action, key.count(), getPriority(it->key.name));
-            ++count;
-        }
-    }
-    if (!count) {
-        // action not found in the map, shouldn't happen!
-        pendingActions.emplace_back(action, key.count(), 0);
-    }
     if (flush) {
         // We'll flush now because there is no potential match with further
         // keystrokes, so no need to wait for timer.

src/Gui/ToolBarManager.cpp

@@ -26,6 +26,7 @@
 # include <QApplication>
 # include <QToolBar>
 # include <QToolButton>
+# include <QPointer>
 #endif
 
 #include "ToolBarManager.h"
@@ -56,7 +57,7 @@ void ToolBarItem::setCommand(const std::string& name)
     _name = name;
 }
 
-std::string ToolBarItem::command() const
+const std::string & ToolBarItem::command() const
 {
     return _name;
 }
@@ -178,6 +179,14 @@ void ToolBarManager::setup(ToolBarItem* toolBarItems)
     if (!toolBarItems)
         return; // empty menu bar
 
+    static QPointer<QWidget> _ActionWidget;
+    if (!_ActionWidget)
+        _ActionWidget = new QWidget(getMainWindow());
+    else {
+        for (auto action : _ActionWidget->actions())
+            _ActionWidget->removeAction(action);
+    }
+
     saveState();
     this->toolbarNames.clear();
 
@@ -222,6 +231,8 @@ void ToolBarManager::setup(ToolBarItem* toolBarItems)
 
         // setup the toolbar
         setup(*it, toolbar);
+        for (auto action : toolbar->actions())
+            _ActionWidget->addAction(action);
 
         // try to add some breaks to avoid to have all toolbars in one line
         if (toolbar_added) {

src/Gui/ToolBarManager.h

@@ -45,7 +45,7 @@ public:
     ~ToolBarItem();
 
     void setCommand(const std::string&);
-    std::string command() const;
+    const std::string &command() const;
 
     bool hasItems() const;
     ToolBarItem* findItem(const std::string&);