From af45b352d7c29c42035196bb2d7c5c76a2f303da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Mon, 11 Jan 2016 19:25:05 +0100 Subject: [PATCH] Fix reentrancy regression in QShortcutMap after c7e5e1d9e When closing a popup dialog using a shortcut, when the popup was originally opened using a shortcut, the closing-shortcut would interfere with the state of the first shortcut, and we ended up sending a key event for the original shortcut. Task-number: QTBUG-50360 Change-Id: I62e5ddb9ca43b28519ede629775bc0d7598dccc4 Reviewed-by: Simon Hausmann --- src/gui/kernel/qshortcutmap.cpp | 8 +++++-- .../kernel/qshortcut/tst_qshortcut.cpp | 23 +++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/gui/kernel/qshortcutmap.cpp b/src/gui/kernel/qshortcutmap.cpp index 9c8218b7b5..3941f7233f 100644 --- a/src/gui/kernel/qshortcutmap.cpp +++ b/src/gui/kernel/qshortcutmap.cpp @@ -336,12 +336,16 @@ bool QShortcutMap::tryShortcut(QKeyEvent *e) // For a partial match we don't know yet if we will handle the shortcut // but we need to say we did, so that we get the follow-up key-presses. return true; - case QKeySequence::ExactMatch: + case QKeySequence::ExactMatch: { + // Save number of identical matches before dispatching + // to keep QShortcutMap and tryShortcut reentrant. + const int identicalMatches = d->identicals.count(); resetState(); dispatchEvent(e); // If there are no identicals we've only found disabled shortcuts, and // shouldn't say that we handled the event. - return d->identicals.count() > 0; + return identicalMatches > 0; + } default: Q_UNREACHABLE(); } diff --git a/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp b/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp index f206a5fe9a..15aef8d503 100644 --- a/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp +++ b/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp @@ -72,7 +72,8 @@ public: TriggerSlot4, TriggerSlot5, TriggerSlot6, - TriggerSlot7 + TriggerSlot7, + SendKeyEvent }; enum Result { @@ -84,6 +85,7 @@ public: Slot5Triggered, Slot6Triggered, Slot7Triggered, + SentKeyEvent, Ambiguous } currentResult; @@ -104,6 +106,7 @@ public slots: void ambigSlot7() { currentResult = Ambiguous; ambigResult = Slot7Triggered; } void statusMessage( const QString& message ) { sbText = message; } void shortcutDestroyed(QObject* obj); + void sendKeyEvent() { sendKeyEvents(edit, Qt::CTRL + Qt::Key_B, 0); currentResult = SentKeyEvent; } public slots: void initTestCase(); @@ -981,6 +984,19 @@ void tst_QShortcut::keypressConsumption() QVERIFY(edit->toPlainText().endsWith("a")); clearAllShortcuts(); + edit->clear(); + QCOMPARE(edit->toPlainText().size(), 0); + + setupShortcut(edit, "first", SendKeyEvent, "Ctrl+A"); + + // Verify reentrancy when a non-shortcut is triggered as part + // of shortcut processing. + currentResult = NoResult; + ambigResult = NoResult; + sendKeyEvents(edit, Qt::CTRL + Qt::Key_A, 0); + QCOMPARE(currentResult, SentKeyEvent); + QCOMPARE(ambigResult, NoResult); + QCOMPARE(edit->toPlainText(), QString(QString(""))); } // ------------------------------------------------------------------ @@ -1182,9 +1198,12 @@ QShortcut *tst_QShortcut::setupShortcut(QWidget *parent, const char *name, int t normal = SLOT(slotTrig7()); ambig = SLOT(ambigSlot7()); break; + case SendKeyEvent: + normal = SLOT(sendKeyEvent()); } connect(cut, SIGNAL(activated()), this, normal); - connect(cut, SIGNAL(activatedAmbiguously()), this, ambig); + if (ambig) + connect(cut, SIGNAL(activatedAmbiguously()), this, ambig); connect(cut, SIGNAL(destroyed(QObject*)), this, SLOT(shortcutDestroyed(QObject*))); shortcuts.append(cut); return cut;