From d7ca800a87a2291c94c6580f0cfe068bb2280caf Mon Sep 17 00:00:00 2001 From: Frederik Gladhorn Date: Thu, 12 Jun 2014 23:42:59 +0200 Subject: [PATCH] Clean up ShortcutOverride handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Jørgen Lind --- src/gui/kernel/qguiapplication.cpp | 12 ------ src/gui/kernel/qshortcutmap.cpp | 4 ++ src/gui/kernel/qwindowsysteminterface.cpp | 36 +++++++++++++++- src/gui/kernel/qwindowsysteminterface.h | 5 ++- src/plugins/platforms/cocoa/qnsview.mm | 2 +- src/testlib/qtestkeyboard.h | 4 ++ src/widgets/kernel/qapplication.cpp | 42 ++++--------------- .../qgraphicsview/tst_qgraphicsview.cpp | 3 +- .../kernel/qshortcut/tst_qshortcut.cpp | 34 ++++++++++++++- 9 files changed, 91 insertions(+), 51 deletions(-) diff --git a/src/gui/kernel/qguiapplication.cpp b/src/gui/kernel/qguiapplication.cpp index 8c71fb23de..2e050572c1 100644 --- a/src/gui/kernel/qguiapplication.cpp +++ b/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(object); - QObject *focus = w ? w->focusObject() : 0; - if (!focus) - focus = object; - if (QGuiApplicationPrivate::instance()->shortcutMap.tryShortcutEvent(focus, static_cast(event))) - return true; - } -#endif - if (object->isWindowType()) QGuiApplicationPrivate::sendQWindowEventToQPlatformWindow(static_cast(object), event); return QCoreApplication::notify(object, event); diff --git a/src/gui/kernel/qshortcutmap.cpp b/src/gui/kernel/qshortcutmap.cpp index cad707ab70..b3e71c77fa 100644 --- a/src/gui/kernel/qshortcutmap.cpp +++ b/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) diff --git a/src/gui/kernel/qwindowsysteminterface.cpp b/src/gui/kernel/qwindowsysteminterface.cpp index 722a695481..073f351a8e 100644 --- a/src/gui/kernel/qwindowsysteminterface.cpp +++ b/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; diff --git a/src/gui/kernel/qwindowsysteminterface.h b/src/gui/kernel/qwindowsysteminterface.h index 30c236b51f..7bb3938fe6 100644 --- a/src/gui/kernel/qwindowsysteminterface.h +++ b/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); diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm index ca98f6cec3..1f58a0457f 100644 --- a/src/plugins/platforms/cocoa/qnsview.mm +++ b/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; diff --git a/src/testlib/qtestkeyboard.h b/src/testlib/qtestkeyboard.h index 57c3a27295..e533293476 100644 --- a/src/testlib/qtestkeyboard.h +++ b/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"); } diff --git a/src/widgets/kernel/qapplication.cpp b/src/widgets/kernel/qapplication.cpp index c913d83213..6df1d735a1 100644 --- a/src/widgets/kernel/qapplication.cpp +++ b/src/widgets/kernel/qapplication.cpp @@ -3126,39 +3126,15 @@ bool QApplication::notify(QObject *receiver, QEvent *e) } switch (e->type()) { - case QEvent::KeyPress: - { - bool isWidget = receiver->isWidgetType(); - bool isWindow = receiver->isWindowType(); - bool isGraphicsWidget = false; -#ifndef QT_NO_GRAPHICSVIEW - isGraphicsWidget = !isWidget && !isWindow && qobject_cast(receiver); -#endif - if (!isWidget && !isGraphicsWidget && !isWindow) { - return d->notify_helper(receiver, e); - } - - QKeyEvent* key = static_cast(e); -#ifndef QT_NO_SHORTCUT - // Try looking for a Shortcut before sending key events - QObject *shortcutReceiver = receiver; - if (!isWidget && isWindow) { - QWindow *w = qobject_cast(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); - - } + case QEvent::KeyPress: { + int key = static_cast(e)->key(); + qt_in_tab_key_event = (key == Qt::Key_Backtab + || key == Qt::Key_Tab + || key == Qt::Key_Left + || key == Qt::Key_Up + || key == Qt::Key_Right + || key == Qt::Key_Down); + } default: break; } diff --git a/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp b/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp index afd8be71e8..8171277f36 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp +++ b/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); - QApplication::sendEvent(view.viewport(), &keyPress); + QTest::keyPress(view.viewport(), Qt::Key_Space); QCOMPARE(item->events.size(), 9); QCOMPARE(item->events.at(item->events.size() - 2), QEvent::ShortcutOverride); QCOMPARE(item->events.last(), QEvent::KeyPress); diff --git a/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp b/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp index 1d6f577192..0691801553 100644 --- a/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp +++ b/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp @@ -42,6 +42,7 @@ #include #include #include +#include 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); } }