From 031adeaf42ddaef8d01338f6c59ba97170be5d53 Mon Sep 17 00:00:00 2001 From: jasplin Date: Mon, 20 Apr 2009 14:24:08 +0200 Subject: Fixed key sequence eating behavior for QShortcut and QAction. A disabled QShortcut should eat its key sequence even for complex sequences like "Ctrl-E 1". For example, a line edit with such a shortcut should not display 1 after typing "Ctrl-E" and then "1". The patch achieves this essentially by moving more of the decision making (of whether to eat the key secuence) from shortcutmap.cpp to qhortcut.cpp. Moreover, an invisible QAction should not eat any of its key sequences (primary nor alternates). In the example above, the line edit would display 1 when typing this sequence for an invisible action. The patch achieves this by temporarily unregistering all of the action's shortcuts while the action is invisible. Reviewed-by: mariusSO Task-number: 251246 --- src/gui/kernel/qaction.cpp | 47 +++++++++++++++++++++++++--------- src/gui/kernel/qaction_p.h | 1 + src/gui/kernel/qshortcut.cpp | 20 ++++++++------- src/gui/kernel/qshortcutmap.cpp | 9 +++---- tests/auto/qaction/tst_qaction.cpp | 33 ++++++++++++++++++++++++ tests/auto/qshortcut/tst_qshortcut.cpp | 7 +++-- 6 files changed, 86 insertions(+), 31 deletions(-) diff --git a/src/gui/kernel/qaction.cpp b/src/gui/kernel/qaction.cpp index abb17d7..4edc1ca 100644 --- a/src/gui/kernel/qaction.cpp +++ b/src/gui/kernel/qaction.cpp @@ -136,25 +136,27 @@ void QActionPrivate::redoGrab(QShortcutMap &map) void QActionPrivate::redoGrabAlternate(QShortcutMap &map) { Q_Q(QAction); - foreach (int id, alternateShortcutIds) - if (id) + for (int i = 0; i < alternateShortcutIds.size(); ++i) + if (int id = alternateShortcutIds.at(i)) map.removeShortcut(id, q); alternateShortcutIds.clear(); if (alternateShortcuts.isEmpty()) return; - foreach (const QKeySequence& alternate, alternateShortcuts) { + for (int i = 0; i < alternateShortcuts.size(); ++i) { + const QKeySequence &alternate = alternateShortcuts.at(i); if (!alternate.isEmpty()) alternateShortcutIds.append(map.addShortcut(q, alternate, shortcutContext)); else alternateShortcutIds.append(0); } + if (!enabled) { - foreach (int id, alternateShortcutIds) - map.setShortcutEnabled(false, id, q); + for (int i = 0; i < alternateShortcutIds.size(); ++i) + map.setShortcutEnabled(false, alternateShortcutIds.at(i), q); } if (!autorepeat) { - foreach (int id, alternateShortcutIds) - map.setShortcutAutoRepeat(false, id, q); + for (int i = 0; i < alternateShortcutIds.size(); ++i) + map.setShortcutAutoRepeat(false, alternateShortcutIds.at(i), q); } } @@ -163,10 +165,26 @@ void QActionPrivate::setShortcutEnabled(bool enable, QShortcutMap &map) Q_Q(QAction); if (shortcutId) map.setShortcutEnabled(enable, shortcutId, q); - foreach (int id, alternateShortcutIds) - if (id) + for (int i = 0; i < alternateShortcutIds.size(); ++i) + if (int id = alternateShortcutIds.at(i)) map.setShortcutEnabled(enable, id, q); } + +void QActionPrivate::removeAll(QShortcutMap &map) +{ + Q_Q(QAction); + if (shortcutId) { + map.removeShortcut(shortcutId, q); + shortcutId = 0; + } + + for (int i = 0; i < alternateShortcutIds.size(); ++i) + if (int id = alternateShortcutIds.at(i)) + map.removeShortcut(id, q); + + alternateShortcutIds.clear(); +} + #endif // QT_NO_SHORTCUT @@ -615,8 +633,8 @@ QAction::~QAction() #ifndef QT_NO_SHORTCUT if (d->shortcutId && qApp) { qApp->d_func()->shortcutMap.removeShortcut(d->shortcutId, this); - foreach (int id, d->alternateShortcutIds) - qApp->d_func()->shortcutMap.removeShortcut(id, this); + for (int i = 0; i < d->alternateShortcutIds.size(); ++i) + qApp->d_func()->shortcutMap.removeShortcut(d->alternateShortcutIds.at(i), this); } #endif } @@ -1049,7 +1067,12 @@ void QAction::setVisible(bool b) d->visible = b; d->enabled = b && !d->forceDisabled && (!d->group || d->group->isEnabled()) ; #ifndef QT_NO_SHORTCUT - d->setShortcutEnabled(d->enabled, qApp->d_func()->shortcutMap); + if (b) { + d->redoGrab(qApp->d_func()->shortcutMap); + d->redoGrabAlternate(qApp->d_func()->shortcutMap); + } else { + d->removeAll(qApp->d_func()->shortcutMap); + } #endif d->sendDataChanged(); } diff --git a/src/gui/kernel/qaction_p.h b/src/gui/kernel/qaction_p.h index 0617ef5..a5b3731 100644 --- a/src/gui/kernel/qaction_p.h +++ b/src/gui/kernel/qaction_p.h @@ -111,6 +111,7 @@ public: void redoGrab(QShortcutMap &map); void redoGrabAlternate(QShortcutMap &map); void setShortcutEnabled(bool enable, QShortcutMap &map); + void removeAll(QShortcutMap &map); static QShortcutMap *globalMap; #endif // QT_NO_SHORTCUT diff --git a/src/gui/kernel/qshortcut.cpp b/src/gui/kernel/qshortcut.cpp index 50b6e59..f3c93c6 100644 --- a/src/gui/kernel/qshortcut.cpp +++ b/src/gui/kernel/qshortcut.cpp @@ -385,19 +385,21 @@ bool QShortcut::event(QEvent *e) { Q_D(QShortcut); bool handled = false; - if (d->sc_enabled && e->type() == QEvent::Shortcut) { + if (e->type() == QEvent::Shortcut) { QShortcutEvent *se = static_cast(e); if (se->shortcutId() == d->sc_id && se->key() == d->sc_sequence){ + if (d->sc_enabled) { #ifndef QT_NO_WHATSTHIS - if (QWhatsThis::inWhatsThisMode()) { - QWhatsThis::showText(QCursor::pos(), d->sc_whatsthis); - handled = true; - } else + if (QWhatsThis::inWhatsThisMode()) { + QWhatsThis::showText(QCursor::pos(), d->sc_whatsthis); + handled = true; + } else #endif - if (se->isAmbiguous()) - emit activatedAmbiguously(); - else - emit activated(); + if (se->isAmbiguous()) + emit activatedAmbiguously(); + else + emit activated(); + } handled = true; } } diff --git a/src/gui/kernel/qshortcutmap.cpp b/src/gui/kernel/qshortcutmap.cpp index ed9654b..9766a69 100644 --- a/src/gui/kernel/qshortcutmap.cpp +++ b/src/gui/kernel/qshortcutmap.cpp @@ -370,9 +370,8 @@ bool QShortcutMap::tryShortcutEvent(QObject *o, QKeyEvent *e) default: break; } - // If nextState is QKeySequence::ExactMatch && identicals.count == 0 - // we've only found disabled shortcuts - return identicalMatches > 0 || result == QKeySequence::PartialMatch; + + return true; } /*! \internal @@ -494,9 +493,7 @@ QKeySequence::SequenceMatch QShortcutMap::find(QKeyEvent *e) // We don't need partials, if we have identicals if (d->identicals.size()) break; - // We only care about enabled partials, so we don't consume - // key events when all partials are disabled! - partialFound |= (*it).enabled; + partialFound = true; } } ++it; diff --git a/tests/auto/qaction/tst_qaction.cpp b/tests/auto/qaction/tst_qaction.cpp index 34f2dfd..8e4ae86 100644 --- a/tests/auto/qaction/tst_qaction.cpp +++ b/tests/auto/qaction/tst_qaction.cpp @@ -46,6 +46,7 @@ #include #include #include +#include //TESTED_CLASS= //TESTED_FILES= @@ -74,6 +75,7 @@ private slots: void setStandardKeys(); void alternateShortcuts(); void enabledVisibleInteraction(); + void invisibleActionWithComplexShortcut(); void task200823_tooltip(); private: @@ -322,5 +324,36 @@ void tst_QAction::task200823_tooltip() QCOMPARE(action->toolTip(), ref); } +void tst_QAction::invisibleActionWithComplexShortcut() +{ + QAction action(0); + action.setShortcut(QKeySequence(Qt::CTRL + Qt::Key_E, Qt::Key_1)); + + QLineEdit edit; + edit.addAction(&action); + edit.show(); + QTest::qWait(100); + + QSignalSpy spy(&action, SIGNAL(triggered())); + + action.setVisible(true); + QTest::keyPress(&edit, Qt::Key_E, Qt::ControlModifier); + QTest::keyRelease(&edit, Qt::Key_E, Qt::ControlModifier); + QTest::keyPress(&edit, Qt::Key_1, Qt::NoModifier); + QTest::keyRelease(&edit, Qt::Key_1, Qt::NoModifier); + QCOMPARE(spy.count(), 1); + QCOMPARE(edit.text(), QLatin1String("")); + + edit.clear(); + spy.clear(); + action.setVisible(false); + QTest::keyPress(&edit, Qt::Key_E, Qt::ControlModifier); + QTest::keyRelease(&edit, Qt::Key_E, Qt::ControlModifier); + QTest::keyPress(&edit, Qt::Key_1, Qt::NoModifier); + QTest::keyRelease(&edit, Qt::Key_1, Qt::NoModifier); + QCOMPARE(spy.count(), 0); + QCOMPARE(edit.text(), QLatin1String("1")); +} + QTEST_MAIN(tst_QAction) #include "tst_qaction.moc" diff --git a/tests/auto/qshortcut/tst_qshortcut.cpp b/tests/auto/qshortcut/tst_qshortcut.cpp index 69ebf74..cd80204 100644 --- a/tests/auto/qshortcut/tst_qshortcut.cpp +++ b/tests/auto/qshortcut/tst_qshortcut.cpp @@ -987,17 +987,16 @@ void tst_QShortcut::keypressConsumption() cut1->setEnabled(false); cut2->setEnabled(false); - // Make sure keypresses is passed on, since all multiple keysequences - // with Ctrl+I are disabled + edit->clear(); sendKeyEvents(edit, Qt::CTRL + Qt::Key_I, 0); // Send key to edit QCOMPARE( currentResult, NoResult ); QCOMPARE( ambigResult, NoResult ); - QVERIFY(edit->toPlainText().endsWith("")); + QVERIFY(edit->toPlainText().isEmpty()); sendKeyEvents(edit, Qt::Key_A, 'a'); // Send key to edit QCOMPARE( currentResult, NoResult ); QCOMPARE( ambigResult, NoResult ); - QVERIFY(edit->toPlainText().endsWith("a")); + QVERIFY(edit->toPlainText().isEmpty()); clearAllShortcuts(); } -- cgit v0.12