Move shortcut handling back into QPA and simplify delivery

Commit 7f5b94b47 moved shortcut handling into QGuiApplication (for all
platforms except OS X), due to crashes on Android where the events are
delivered from another thread.

Now that we have synchronous event delivery (also across threads) from
QPA, this is no longer needed, and we can move the code back to QPA,
where the platform has more control over when and how shortcut events
are delivered in relation to normal key events.

Handling shortcuts is as before a two step process. We first send a
QKeyEvent::ShortcutOverride event to the active window, which allows
clients (widgets e.g.) to signal that they want to handle the shortcut
themselves. If the override event is accepted, we treat it as the
shortcut not being handled as a regular shortcut, and send the event
as a key press instead, allowing the widget to handle the shortcut.

If nothing accepted the shortcut override event we pass it along to
the global shortcut map, which will treat the event as handled if
an exact or partial match is found.

The QShortcutMap::tryShortcutEvent() and nextState() implementation
has been simplified to not use the events accepted state for its
internal operation, which removes the need for saving the state
of the incoming event.

The QKeyEvent::ShortcutOverride event was also always sent with
the accepted state set to false, by calling ignore() on it before
sending it. This is now explicit by having shortcut override
events being ignored by default in their constructor, and the
documentation has been updated accordingly.

Change-Id: I9afa29dbc00bef09fd22ee6bf09661b06340d715
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@theqtcompany.com>
Reviewed-by: Christian Stromme <christian.stromme@theqtcompany.com>

1 files changed, 19 insertions, 40 deletions

diff --git a/src/gui/kernel/qshortcutmap.cpp b/src/gui/kernel/qshortcutmap.cpp
index 3e267f2e0b..3b2e6ffd29 100644
--- a/src/gui/kernel/qshortcutmap.cpp
+++ b/src/gui/kernel/qshortcutmap.cpp
@@ -309,59 +309,42 @@ QKeySequence::SequenceMatch QShortcutMap::state()
 }
 
 /*! \internal
-    Uses ShortcutOverride event to see if any widgets want to override
-    the event. If not, uses nextState(QKeyEvent) to check for a grabbed
-    Shortcut, and dispatchEvent() is found and identical.
+    Uses nextState(QKeyEvent) to check for a grabbed shortcut.
 
-    \note that this function should only be called from QWindowSystemInterface,
-    otherwise it will result in duplicate events.
+    If so, it is dispatched using dispatchEvent().
+
+    Returns true if a shortcut handled the event.
 
     \sa nextState, dispatchEvent
 */
-bool QShortcutMap::tryShortcutEvent(QObject *o, QKeyEvent *e)
+bool QShortcutMap::tryShortcut(QKeyEvent *e)
 {
     Q_D(QShortcutMap);
 
     if (e->key() == Qt::Key_unknown)
         return false;
 
-    bool wasAccepted = e->isAccepted();
-    bool wasSpontaneous = e->spont;
-    if (d->currentState == QKeySequence::NoMatch) {
-        ushort orgType = e->t;
-        e->t = QEvent::ShortcutOverride;
-        e->ignore();
-        QCoreApplication::sendEvent(o, e);
-        e->t = orgType;
-        e->spont = wasSpontaneous;
-        if (e->isAccepted()) {
-            if (!wasAccepted)
-                e->ignore();
-            return false;
-        }
-    }
-
-    QKeySequence::SequenceMatch result = nextState(e);
-    bool stateWasAccepted = e->isAccepted();
-    if (wasAccepted)
-        e->accept();
-    else
-        e->ignore();
-
-    int identicalMatches = d->identicals.count();
+    QKeySequence::SequenceMatch previousState = state();
 
-    switch(result) {
+    switch (nextState(e)) {
     case QKeySequence::NoMatch:
-        return stateWasAccepted;
+        // In the case of going from a partial match to no match we handled the
+        // event, since we already stated that we did for the partial match. But
+        // in the normal case of directly going to no match we say we didn't.
+        return previousState == QKeySequence::PartialMatch;
+    case QKeySequence::PartialMatch:
+        // 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:
         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;
     default:
-        break;
+        Q_UNREACHABLE();
     }
-    // If nextState is QKeySequence::ExactMatch && identicals.count == 0
-    // we've only found disabled shortcuts
-    return identicalMatches > 0 || result == QKeySequence::PartialMatch;
 }
 
 /*! \internal
@@ -396,10 +379,6 @@ QKeySequence::SequenceMatch QShortcutMap::nextState(QKeyEvent *e)
         }
     }
 
-    // Should we eat this key press?
-    if (d->currentState == QKeySequence::PartialMatch
-        || (d->currentState == QKeySequence::ExactMatch && d->identicals.count()))
-        e->accept();
     // Does the new state require us to clean up?
     if (result == QKeySequence::NoMatch)
         clearSequence(d->currentSequences);