Clean up ShortcutOverride handling

Instead of sending the event from random places, send it from
QWindowSystemInterface. This allows to send override events on OS X to
menus before doing other key processing and reduces the number of
ShortcutOverride events on all platforms to exactly one per key press
event.

Additional test by Friedemann Kleint.

Task-number: QTBUG-38986
Change-Id: I6981bb776aba586ebc7c3daa5fd7a0d84c25bc3e
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@digia.com>
Reviewed-by: Jørgen Lind <jorgen.lind@digia.com>

src/gui/kernel/qguiapplication.cpp

@@ -1538,18 +1538,6 @@ int QGuiApplication::exec()
 */
 bool QGuiApplication::notify(QObject *object, QEvent *event)
 {
-#ifndef QT_NO_SHORTCUT
-    if (event->type() == QEvent::KeyPress) {
-        // Try looking for a Shortcut before sending key events
-        QWindow *w = qobject_cast<QWindow *>(object);
-        QObject *focus = w ? w->focusObject() : 0;
-        if (!focus)
-            focus = object;
-        if (QGuiApplicationPrivate::instance()->shortcutMap.tryShortcutEvent(focus, static_cast<QKeyEvent *>(event)))
-            return true;
-    }
-#endif
-
     if (object->isWindowType())
         QGuiApplicationPrivate::sendQWindowEventToQPlatformWindow(static_cast<QWindow *>(object), event);
     return QCoreApplication::notify(object, event);

src/gui/kernel/qshortcutmap.cpp

@@ -310,6 +310,10 @@ QKeySequence::SequenceMatch QShortcutMap::state()
     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.
+
+    \note that this function should only be called from QWindowSystemInterface,
+    otherwise it will result in duplicate events.
+
     \sa nextState, dispatchEvent
 */
 bool QShortcutMap::tryShortcutEvent(QObject *o, QKeyEvent *e)

src/gui/kernel/qwindowsysteminterface.cpp

@@ -220,6 +220,28 @@ bool QWindowSystemInterface::tryHandleShortcutEvent(QWindow *w, ulong timestamp,
 #endif
 }
 
+// used by QTestLib to directly send shortcuts to objects
+bool QWindowSystemInterface::tryHandleShortcutEventToObject(QObject *o, ulong timestamp, int k, Qt::KeyboardModifiers mods,
+                                   const QString &text, bool autorep, ushort count)
+{
+#ifndef QT_NO_SHORTCUT
+    QGuiApplicationPrivate::modifier_buttons = mods;
+
+    QKeyEvent qevent(QEvent::ShortcutOverride, k, mods, text, autorep, count);
+    qevent.setTimestamp(timestamp);
+    return QGuiApplicationPrivate::instance()->shortcutMap.tryShortcutEvent(o, &qevent);
+#else
+    Q_UNUSED(w)
+    Q_UNUSED(timestamp)
+    Q_UNUSED(k)
+    Q_UNUSED(mods)
+    Q_UNUSED(text)
+    Q_UNUSED(autorep)
+    Q_UNUSED(count)
+    return false;
+#endif
+}
+
 bool QWindowSystemInterface::tryHandleExtendedShortcutEvent(QWindow *w, int k, Qt::KeyboardModifiers mods,
                                                                        quint32 nativeScanCode, quint32 nativeVirtualKey, quint32 nativeModifiers,
                                                                        const QString &text, bool autorep, ushort count)
@@ -265,6 +287,9 @@ void QWindowSystemInterface::handleKeyEvent(QWindow *w, QEvent::Type t, int k, Q
 
 void QWindowSystemInterface::handleKeyEvent(QWindow *tlw, ulong timestamp, QEvent::Type t, int k, Qt::KeyboardModifiers mods, const QString & text, bool autorep, ushort count)
 {
+    if (t == QEvent::KeyPress && QWindowSystemInterface::tryHandleShortcutEvent(tlw, timestamp, k, mods, text))
+            return;
+
     QWindowSystemInterfacePrivate::KeyEvent * e =
             new QWindowSystemInterfacePrivate::KeyEvent(tlw, timestamp, t, k, mods, text, autorep, count);
     QWindowSystemInterfacePrivate::handleWindowSystemEvent(e);
@@ -286,8 +311,12 @@ void QWindowSystemInterface::handleExtendedKeyEvent(QWindow *tlw, ulong timestam
                                                     quint32 nativeScanCode, quint32 nativeVirtualKey,
                                                     quint32 nativeModifiers,
                                                     const QString& text, bool autorep,
-                                                    ushort count)
+                                                    ushort count, bool tryShortcutOverride)
 {
+    // on OS X we try the shortcut override even earlier and thus shouldn't handle it here
+    if (tryShortcutOverride && type == QEvent::KeyPress && QWindowSystemInterface::tryHandleShortcutEvent(tlw, timestamp, key, modifiers, text))
+            return;
+
     QWindowSystemInterfacePrivate::KeyEvent * e =
             new QWindowSystemInterfacePrivate::KeyEvent(tlw, timestamp, type, key, modifiers,
                 nativeScanCode, nativeVirtualKey, nativeModifiers, text, autorep, count);
@@ -760,6 +789,11 @@ Q_GUI_EXPORT void qt_handleKeyEvent(QWindow *w, QEvent::Type t, int k, Qt::Keybo
     QWindowSystemInterface::handleKeyEvent(w, t, k, mods, text, autorep, count);
 }
 
+Q_GUI_EXPORT bool qt_sendShortcutOverrideEvent(QObject *o, ulong timestamp, int k, Qt::KeyboardModifiers mods, const QString &text = QString(), bool autorep = false, ushort count = 1)
+{
+    return QWindowSystemInterface::tryHandleShortcutEventToObject(o, timestamp, k, mods, text, autorep, count);
+}
+
 static QWindowSystemInterface::TouchPoint touchPoint(const QTouchEvent::TouchPoint& pt)
 {
     QWindowSystemInterface::TouchPoint p;

src/gui/kernel/qwindowsysteminterface.h

@@ -83,6 +83,9 @@ public:
     static bool tryHandleShortcutEvent(QWindow *w, ulong timestamp, int k, Qt::KeyboardModifiers mods,
                                                   const QString & text = QString(), bool autorep = false, ushort count = 1);
 
+    static bool tryHandleShortcutEventToObject(QObject *o, ulong timestamp, int k, Qt::KeyboardModifiers mods,
+                                       const QString & text = QString(), bool autorep = false, ushort count = 1);
+
     static bool tryHandleExtendedShortcutEvent(QWindow *w, int k, Qt::KeyboardModifiers mods,
                                                           quint32 nativeScanCode, quint32 nativeVirtualKey, quint32 nativeModifiers,
                                                           const QString & text = QString(), bool autorep = false, ushort count = 1);
@@ -102,7 +105,7 @@ public:
                                        quint32 nativeScanCode, quint32 nativeVirtualKey,
                                        quint32 nativeModifiers,
                                        const QString& text = QString(), bool autorep = false,
-                                       ushort count = 1);
+                                       ushort count = 1, bool tryShortcutOverride = true);
     static void handleWheelEvent(QWindow *w, const QPointF & local, const QPointF & global, QPoint pixelDelta, QPoint angleDelta, Qt::KeyboardModifiers mods = Qt::NoModifier, Qt::ScrollPhase phase = Qt::ScrollUpdate);
     static void handleWheelEvent(QWindow *w, ulong timestamp, const QPointF & local, const QPointF & global, QPoint pixelDelta, QPoint angleDelta, Qt::KeyboardModifiers mods = Qt::NoModifier, Qt::ScrollPhase phase = Qt::ScrollUpdate);
 

src/plugins/platforms/cocoa/qnsview.mm

@@ -1460,7 +1460,7 @@ static QTabletEvent::TabletDevice wacomTabletDevice(NSEvent *theEvent)
 
     if (m_sendKeyEvent && m_composingText.isEmpty())
         QWindowSystemInterface::handleExtendedKeyEvent(focusWindow, timestamp, QEvent::Type(eventType), keyCode, modifiers,
-                                                       nativeScanCode, nativeVirtualKey, nativeModifiers, text, [nsevent isARepeat]);
+                                                       nativeScanCode, nativeVirtualKey, nativeModifiers, text, [nsevent isARepeat], false);
 
     m_sendKeyEvent = false;
     m_resendKeyEvent = false;

src/testlib/qtestkeyboard.h

@@ -57,6 +57,7 @@
 QT_BEGIN_NAMESPACE
 
 Q_GUI_EXPORT void qt_handleKeyEvent(QWindow *w, QEvent::Type t, int k, Qt::KeyboardModifiers mods, const QString & text = QString(), bool autorep = false, ushort count = 1);
+Q_GUI_EXPORT bool qt_sendShortcutOverrideEvent(QObject *o, ulong timestamp, int k, Qt::KeyboardModifiers mods, const QString &text = QString(), bool autorep = false, ushort count = 1);
 
 namespace QTest
 {
@@ -170,6 +171,9 @@ namespace QTest
 
         QKeyEvent a(press ? QEvent::KeyPress : QEvent::KeyRelease, code, modifier, text, repeat);
         QSpontaneKeyEvent::setSpontaneous(&a);
+
+        if (press && qt_sendShortcutOverrideEvent(widget, a.timestamp(), code, modifier, text, repeat))
+            return;
         if (!qApp->notify(widget, &a))
             QTest::qWarn("Keyboard event not accepted by receiving widget");
     }

src/widgets/kernel/qapplication.cpp

@@ -3126,38 +3126,14 @@ bool QApplication::notify(QObject *receiver, QEvent *e)
     }
 
     switch (e->type()) {
-        case QEvent::KeyPress:
+        case QEvent::KeyPress: {
-            {
+                int key = static_cast<QKeyEvent*>(e)->key();
-                bool isWidget = receiver->isWidgetType();
+                qt_in_tab_key_event = (key == Qt::Key_Backtab
-                bool isWindow = receiver->isWindowType();
+                        || key == Qt::Key_Tab
-                bool isGraphicsWidget = false;
+                        || key == Qt::Key_Left
-#ifndef QT_NO_GRAPHICSVIEW
+                        || key == Qt::Key_Up
-                isGraphicsWidget = !isWidget && !isWindow && qobject_cast<QGraphicsWidget *>(receiver);
+                        || key == Qt::Key_Right
-#endif
+                        || key == Qt::Key_Down);
-                if (!isWidget && !isGraphicsWidget && !isWindow) {
-                    return d->notify_helper(receiver, e);
-                }
-
-                QKeyEvent* key = static_cast<QKeyEvent*>(e);
-#ifndef QT_NO_SHORTCUT
-                // Try looking for a Shortcut before sending key events
-                QObject *shortcutReceiver = receiver;
-                if (!isWidget && isWindow) {
-                    QWindow *w = qobject_cast<QWindow *>(receiver);
-                    QObject *focus = w ? w->focusObject() : 0;
-                    if (focus)
-                        shortcutReceiver = focus;
-                }
-                if (qApp->d_func()->shortcutMap.tryShortcutEvent(shortcutReceiver, key))
-                    return true;
-#endif
-                qt_in_tab_key_event = (key->key() == Qt::Key_Backtab
-                        || key->key() == Qt::Key_Tab
-                        || key->key() == Qt::Key_Left
-                        || key->key() == Qt::Key_Up
-                        || key->key() == Qt::Key_Right
-                        || key->key() == Qt::Key_Down);
-
         }
         default:
             break;

tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp

@@ -2105,8 +2105,7 @@ void tst_QGraphicsView::sendEvent()
     QCOMPARE(item->events.at(item->events.size() - 2), QEvent::GraphicsSceneMouseRelease);
     QCOMPARE(item->events.at(item->events.size() - 1), QEvent::UngrabMouse);
 
-    QKeyEvent keyPress(QEvent::KeyPress, Qt::Key_Space, 0);
+    QTest::keyPress(view.viewport(), Qt::Key_Space);
-    QApplication::sendEvent(view.viewport(), &keyPress);
     QCOMPARE(item->events.size(), 9);
     QCOMPARE(item->events.at(item->events.size() - 2), QEvent::ShortcutOverride);
     QCOMPARE(item->events.last(), QEvent::KeyPress);

tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp

@@ -42,6 +42,7 @@
 #include <qdebug.h>
 #include <qstring.h>
 #include <qshortcut.h>
+#include <qscreen.h>
 
 class AccelForm;
 QT_BEGIN_NAMESPACE
@@ -119,6 +120,7 @@ private slots:
     void keypressConsumption();
     void unicodeCompare();
     void context();
+    void duplicatedShortcutOverride();
 
 protected:
     static Qt::KeyboardModifiers toButtons( int key );
@@ -1082,6 +1084,36 @@ void tst_QShortcut::context()
     clearAllShortcuts();
 }
 
+// QTBUG-38986, do not generate duplicated QEvent::ShortcutOverride in event processing.
+class OverrideCountingWidget : public QWidget
+{
+public:
+    OverrideCountingWidget(QWidget *parent = 0) : QWidget(parent), overrideCount(0) {}
+
+    int overrideCount;
+
+    bool event(QEvent *e) Q_DECL_OVERRIDE
+    {
+        if (e->type() == QEvent::ShortcutOverride)
+            overrideCount++;
+        return QWidget::event(e);
+    }
+};
+
+void tst_QShortcut::duplicatedShortcutOverride()
+{
+    OverrideCountingWidget w;
+    w.setWindowTitle(Q_FUNC_INFO);
+    w.resize(200, 200);
+    w.move(QGuiApplication::primaryScreen()->availableGeometry().center() - QPoint(100, 100));
+    w.show();
+    QApplication::setActiveWindow(&w);
+    QVERIFY(QTest::qWaitForWindowActive(&w));
+    QTest::keyPress(w.windowHandle(), Qt::Key_A);
+    QCoreApplication::processEvents();
+    QCOMPARE(w.overrideCount, 1);
+}
+
 // ------------------------------------------------------------------
 // Element Testing helper functions ---------------------------------
 // ------------------------------------------------------------------
@@ -1226,7 +1258,7 @@ void tst_QShortcut::testElement()
         setupShortcut(testWidget, txt, k1, k2, k3, k4);
     } else {
         sendKeyEvents(k1, c1, k2, c2, k3, c3, k4, c4);
-        QCOMPARE(currentResult, result);
+        QCOMPARE((int)currentResult, (int)result);
     }
 }