Skip to content

Commit

Permalink
Don't fire shortcut if the action only lives in an unreachable submenu
Browse files Browse the repository at this point in the history
Menus can be represented by a menu action, and if that menu action has
been hidden or disabled, then the submenu is not accessible from the
parent menu or menu bar to which it was added. Don't walk the menu
action chain further when checking whether the shortcut should trigger.

Note that this is unrelated to the menu being visible or not; we
obviously want to trigger shortcuts for actions that only live in a menu
that has not been shown, otherwise the shortcut would be rather
pointless.

Pick-to: 6.2
Fixes: QTBUG-25743
Change-Id: I48735e17352989bbc84a72263e4828f519b78095
Reviewed-by: Shawn Rutledge <[email protected]>
  • Loading branch information
vohi authored and ec1oud committed Oct 25, 2021
1 parent e022ff0 commit 01f96b1
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/widgets/kernel/qshortcut_widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ static bool correctActionContext(Qt::ShortcutContext context, QAction *a, QWidge
continue;
#endif
QAction *a = menu->menuAction();
if (correctActionContext(context, a, active_window))
if (a->isVisible() && a->isEnabled() && correctActionContext(context, a, active_window))
return true;
} else
#endif
Expand Down
74 changes: 74 additions & 0 deletions tests/auto/widgets/kernel/qaction/tst_qaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
#include <qaction.h>
#include <qactiongroup.h>
#include <qmenu.h>
#include <qmenubar.h>
#include <qtoolbar.h>
#include <qpa/qplatformtheme.h>
#include <qpa/qplatformintegration.h>
#include <private/qguiapplication_p.h>
Expand Down Expand Up @@ -65,6 +67,8 @@ private slots:
void disableShortcutsWithBlockedWidgets_data();
void disableShortcutsWithBlockedWidgets();
void shortcutFromKeyEvent(); // QTBUG-48325
void disableShortcutInMenuAction_data();
void disableShortcutInMenuAction();
#endif

private:
Expand Down Expand Up @@ -418,6 +422,76 @@ void tst_QAction::shortcutFromKeyEvent()
QCOMPARE(testWidget.shortcutOverrideCount, 1);
}

/*
Ignore actions in menus whose menu action has been hidden or disabled.
The menu entry will not be in the menu bar or parent menu, so the action
is not reachable through interactive means. QTBUG-25743
*/
void tst_QAction::disableShortcutInMenuAction_data()
{
QTest::addColumn<QByteArray>("property");

QTest::addRow("visible") << QByteArray("visible");
QTest::addRow("enabled") << QByteArray("enabled");
}

void tst_QAction::disableShortcutInMenuAction()
{
QFETCH(QByteArray, property);

QMainWindow mw;
QMenu *testMenu = mw.menuBar()->addMenu("Test");
QAction *testAction = testMenu->addAction("Test Action");
testAction->setShortcut(Qt::ControlModifier | Qt::Key_A);
QToolBar *toolBar = new QToolBar;
mw.addToolBar(toolBar);

mw.show();
QVERIFY(QTest::qWaitForWindowActive(&mw));

int expectedTriggerCount = 0;
QSignalSpy spy(testAction, &QAction::triggered);

QKeyEvent event(QEvent::KeyPress, Qt::Key_A, Qt::ControlModifier);
QApplication::sendEvent(&mw, &event);
QCOMPARE(spy.count(), ++expectedTriggerCount);

testMenu->menuAction()->setProperty(property, false);
QApplication::sendEvent(&mw, &event);
QCOMPARE(spy.count(), expectedTriggerCount);

testMenu->menuAction()->setProperty(property, true);
QApplication::sendEvent(&mw, &event);
QCOMPARE(spy.count(), ++expectedTriggerCount);

// If the action lives somewhere else, then keep firing even
// if the menu has been hidden or disabled.
toolBar->addAction(testAction);
QApplication::sendEvent(&mw, &event);
QCOMPARE(spy.count(), ++expectedTriggerCount);

testMenu->menuAction()->setProperty(property, false);
QApplication::sendEvent(&mw, &event);
QCOMPARE(spy.count(), ++expectedTriggerCount);

// unless all other widgets in which the action lives have
// been hidden...
toolBar->hide();
QApplication::sendEvent(&mw, &event);
QCOMPARE(spy.count(), expectedTriggerCount);

// ... or disabled
toolBar->show();
toolBar->setEnabled(false);
QApplication::sendEvent(&mw, &event);
QCOMPARE(spy.count(), expectedTriggerCount);

// back to normal
toolBar->setEnabled(true);
QApplication::sendEvent(&mw, &event);
QCOMPARE(spy.count(), ++expectedTriggerCount);
}

#endif // QT_CONFIG(shortcut)

QTEST_MAIN(tst_QAction)
Expand Down

0 comments on commit 01f96b1

Please sign in to comment.