Change QQuickShortcut::setSequences to bind to all sequences

When binding a Shortcut to a standard key sequence like
QKeySequence::FullScreen, it binds only to one key
sequence, even though there might be multiple key sequences
associated.
This patch changes the code to emit a warning in this case
and allows to bind to multiple key sequences using
'sequences: [ <key> ]'.

Fixes: QTBUG-88682
Change-Id: I88998aa8858d8f2c0c86e46bae94afd7ceb15b66
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 6511b17038627ac30cb6622b13c7d46d9877bac5)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

2 files changed, 166 insertions, 40 deletions

diff --git a/src/quick/util/qquickshortcut.cpp b/src/quick/util/qquickshortcut.cpp
index 2905e36d00..fdd846adb6 100644
--- a/src/quick/util/qquickshortcut.cpp
+++ b/src/quick/util/qquickshortcut.cpp
@@ -44,6 +44,7 @@
 #include <QtQuick/qquickrendercontrol.h>
 #include <QtQuick/private/qtquickglobal_p.h>
 #include <QtGui/private/qguiapplication_p.h>
+#include <QtQml/qqmlinfo.h>
 
 /*!
     \qmltype Shortcut
@@ -124,13 +125,35 @@ Q_QUICK_PRIVATE_EXPORT void qt_quick_set_shortcut_context_matcher(ContextMatcher
 
 QT_BEGIN_NAMESPACE
 
-static QKeySequence valueToKeySequence(const QVariant &value)
+static QKeySequence valueToKeySequence(const QVariant &value, const QQuickShortcut *const shortcut)
 {
-    if (value.userType() == QMetaType::Int)
-        return QKeySequence(static_cast<QKeySequence::StandardKey>(value.toInt()));
+    if (value.userType() == QMetaType::Int) {
+        const QVector<QKeySequence> s =
+                QKeySequence::keyBindings(static_cast<QKeySequence::StandardKey>(value.toInt()));
+        if (s.size() > 1) {
+            const QString templateString = QString::fromUtf16(
+                    u"Shortcut: Only binding to one of multiple key bindings associated with %1. "
+                    u"Use 'sequences: [ <key> ]' to bind to all of them.");
+            qmlWarning(shortcut)
+                    << templateString.arg(static_cast<QKeySequence::StandardKey>(value.toInt()));
+        }
+        return s.size() > 0 ? s[0] : QKeySequence {};
+    }
+
     return QKeySequence::fromString(value.toString());
 }
 
+static QList<QKeySequence> valueToKeySequences(const QVariant &value)
+{
+    if (value.userType() == QMetaType::Int) {
+        return QKeySequence::keyBindings(static_cast<QKeySequence::StandardKey>(value.toInt()));
+    } else {
+        QList<QKeySequence> result;
+        result.push_back(QKeySequence::fromString(value.toString()));
+        return result;
+    }
+}
+
 QQuickShortcut::QQuickShortcut(QObject *parent) : QObject(parent),
     m_enabled(true), m_completed(false), m_autorepeat(true), m_context(Qt::WindowShortcut)
 {
@@ -172,7 +195,7 @@ void QQuickShortcut::setSequence(const QVariant &value)
     if (value == m_shortcut.userValue)
         return;
 
-    QKeySequence keySequence = valueToKeySequence(value);
+    QKeySequence keySequence = valueToKeySequence(value, this);
 
     ungrabShortcut(m_shortcut);
     m_shortcut.userValue = value;
@@ -207,28 +230,42 @@ QVariantList QQuickShortcut::sequences() const
 
 void QQuickShortcut::setSequences(const QVariantList &values)
 {
-    QVector<Shortcut> remainder = m_shortcuts.mid(values.count());
-    m_shortcuts.resize(values.count());
-
-    bool changed = !remainder.isEmpty();
-    for (int i = 0; i < values.count(); ++i) {
-        const QVariant &value = values.at(i);
-        Shortcut& shortcut = m_shortcuts[i];
-        if (value == shortcut.userValue)
-            continue;
-
-        QKeySequence keySequence = valueToKeySequence(value);
-
-        ungrabShortcut(shortcut);
-        shortcut.userValue = value;
-        shortcut.keySequence = keySequence;
-        grabShortcut(shortcut, m_context);
+    // convert QVariantList to QVector<QKeySequence>
+    QVector<Shortcut> requestedShortcuts;
+    for (const QVariant &v : values) {
+        const QVector<QKeySequence> list = valueToKeySequences(v);
+        for (const QKeySequence &s : list) {
+            Shortcut sc;
+            sc.userValue = v;
+            sc.keySequence = s;
+            requestedShortcuts.push_back(sc);
+        }
+    }
 
-        changed = true;
+    // if nothing has changed, just return:
+    if (m_shortcuts.size() == requestedShortcuts.size()) {
+        bool changed = false;
+        for (int i = 0; i < requestedShortcuts.count(); ++i) {
+            const Shortcut &requestedShortcut = requestedShortcuts[i];
+            const Shortcut &shortcut = m_shortcuts[i];
+            if (!(requestedShortcut.userValue == shortcut.userValue
+                  && requestedShortcut.keySequence == shortcut.keySequence)) {
+                changed = true;
+                break;
+            }
+        }
+        if (!changed) {
+            return;
+        }
     }
 
-    if (changed)
-        emit sequencesChanged();
+    for (Shortcut &s : m_shortcuts)
+        ungrabShortcut(s);
+    m_shortcuts = requestedShortcuts;
+    for (Shortcut &s : m_shortcuts)
+        grabShortcut(s, m_context);
+
+    emit sequencesChanged();
 }
 
 /*!
diff --git a/tests/auto/quick/qquickshortcut/tst_qquickshortcut.cpp b/tests/auto/quick/qquickshortcut/tst_qquickshortcut.cpp
index 7b7113e2ce..6351b97fb3 100644
--- a/tests/auto/quick/qquickshortcut/tst_qquickshortcut.cpp
+++ b/tests/auto/quick/qquickshortcut/tst_qquickshortcut.cpp
@@ -42,6 +42,8 @@ class tst_QQuickShortcut : public QQmlDataTest
     Q_OBJECT
 
 private slots:
+    void standardShortcuts_data();
+    void standardShortcuts();
     void shortcuts_data();
     void shortcuts();
     void sequence_data();
@@ -81,6 +83,49 @@ static QVariant shortcutMap(const QVariant &key, bool enabled = true, bool autoR
     return shortcutMap(key, Qt::WindowShortcut, enabled, autoRepeat);
 }
 
+void tst_QQuickShortcut::standardShortcuts_data()
+{
+    QTest::addColumn<QKeySequence::StandardKey>("standardKey");
+    QTest::newRow("Close") << QKeySequence::Close;
+    QTest::newRow("Cut") << QKeySequence::Cut;
+    QTest::newRow("NextChild") << QKeySequence::NextChild;
+    QTest::newRow("PreviousChild") << QKeySequence::PreviousChild;
+    QTest::newRow("FindNext") << QKeySequence::FindNext;
+    QTest::newRow("FindPrevious") << QKeySequence::FindPrevious;
+    QTest::newRow("FullScreen") << QKeySequence::FullScreen;
+}
+
+void tst_QQuickShortcut::standardShortcuts()
+{
+    QFETCH(QKeySequence::StandardKey, standardKey);
+
+    QQmlApplicationEngine engine;
+
+    engine.load(testFileUrl("multiple.qml"));
+    QQuickWindow *window = qobject_cast<QQuickWindow *>(engine.rootObjects().value(0));
+    QVERIFY(window);
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window));
+    window->requestActivate();
+    QVERIFY(QTest::qWaitForWindowActive(window));
+
+    QObject *shortcut = window->property("shortcut").value<QObject *>();
+    QVERIFY(shortcut);
+
+    // create list of shortcuts
+    QVariantList shortcuts;
+    shortcuts.push_back(standardKey);
+    shortcut->setProperty("sequences", shortcuts);
+
+    // test all:
+    QList<QKeySequence> allsequences = QKeySequence::keyBindings(standardKey);
+    for (const QKeySequence &s : allsequences) {
+        window->setProperty("activated", false);
+        QTest::keySequence(window, s);
+        QCOMPARE(window->property("activated").toBool(), true);
+    }
+}
+
 void tst_QQuickShortcut::shortcuts_data()
 {
     QTest::addColumn<QVariantList>("shortcuts");
@@ -188,6 +233,7 @@ void tst_QQuickShortcut::shortcuts()
     window->setProperty("shortcuts", shortcuts);
 
     QTest::keyPress(window, key, modifiers);
+    QTest::keyRelease(window, key, modifiers);
     QCOMPARE(window->property("activatedShortcut").toString(), activatedShortcut);
     QCOMPARE(window->property("ambiguousShortcut").toString(), ambiguousShortcut);
 }
@@ -263,14 +309,22 @@ void tst_QQuickShortcut::sequence()
 
     window->setProperty("shortcuts", shortcuts);
 
-    if (key1 != 0)
+    if (key1 != 0) {
         QTest::keyPress(window, key1, modifiers1);
-    if (key2 != 0)
+        QTest::keyRelease(window, key1, modifiers1);
+    }
+    if (key2 != 0) {
         QTest::keyPress(window, key2, modifiers2);
-    if (key3 != 0)
+        QTest::keyRelease(window, key2, modifiers2);
+    }
+    if (key3 != 0) {
         QTest::keyPress(window, key3, modifiers3);
-    if (key4 != 0)
+        QTest::keyRelease(window, key3, modifiers3);
+    }
+    if (key4 != 0) {
         QTest::keyPress(window, key4, modifiers4);
+        QTest::keyRelease(window, key4, modifiers4);
+    }
 
     QCOMPARE(window->property("activatedShortcut").toString(), activatedShortcut);
     QCOMPARE(window->property("ambiguousShortcut").toString(), ambiguousShortcut);
@@ -352,6 +406,7 @@ void tst_QQuickShortcut::context()
     activeWindow->setProperty("shortcuts", activeWindowShortcuts);
 
     QTest::keyPress(activeWindow, key);
+    QTest::keyRelease(activeWindow, key);
 
     QCOMPARE(activeWindow->property("activatedShortcut").toString(), activeWindowActivatedShortcut);
     QCOMPARE(inactiveWindow->property("activatedShortcut").toString(), inactiveWindowActivatedShortcut);
@@ -411,6 +466,8 @@ void tst_QQuickShortcut::matcher()
     QVERIFY(window);
     window->show();
     QVERIFY(QTest::qWaitForWindowExposed(window));
+    window->requestActivate();
+    QVERIFY(QTest::qWaitForWindowActive(window));
 
     window->setProperty("shortcuts", QVariantList() << shortcut);
     QTest::keyClick(window, key);
@@ -452,6 +509,8 @@ void tst_QQuickShortcut::multiple()
     QVERIFY(window);
     window->show();
     QVERIFY(QTest::qWaitForWindowExposed(window));
+    window->requestActivate();
+    QVERIFY(QTest::qWaitForWindowActive(window));
 
     QObject *shortcut = window->property("shortcut").value<QObject *>();
     QVERIFY(shortcut);
@@ -460,8 +519,33 @@ void tst_QQuickShortcut::multiple()
     shortcut->setProperty("sequences", sequences);
 
     QTest::keyPress(window, key, modifiers);
+    QTest::keyRelease(window, key, modifiers);
 
     QCOMPARE(window->property("activated").toBool(), activated);
+
+    // check it still works after rotating the sequences
+    QStringList rotatedSequences;
+    for (int i = 1; i < sequences.size(); ++i)
+        rotatedSequences.push_back(sequences[i]);
+    if (sequences.size())
+        rotatedSequences.push_back(sequences[0]);
+
+    window->setProperty("activated", false);
+    shortcut->setProperty("sequences", rotatedSequences);
+
+    QTest::keyPress(window, key, modifiers);
+    QTest::keyRelease(window, key, modifiers);
+    QCOMPARE(window->property("activated").toBool(), activated);
+
+    // check setting to no shortcuts
+    QStringList emptySequence;
+
+    window->setProperty("activated", false);
+    shortcut->setProperty("sequences", emptySequence);
+
+    QTest::keyPress(window, key, modifiers);
+    QTest::keyRelease(window, key, modifiers);
+    QCOMPARE(window->property("activated").toBool(), false);
 }
 
 void tst_QQuickShortcut::contextChange_data()
@@ -479,12 +563,12 @@ void tst_QQuickShortcut::contextChange()
     QQmlApplicationEngine engine;
 
     engine.load(testFileUrl("multiple.qml"));
-    QQuickWindow *inactivewindow = qobject_cast<QQuickWindow *>(engine.rootObjects().value(0));
-    QVERIFY(inactivewindow);
-    inactivewindow->show();
-    QVERIFY(QTest::qWaitForWindowExposed(inactivewindow));
+    QQuickWindow *inactiveWindow = qobject_cast<QQuickWindow *>(engine.rootObjects().value(0));
+    QVERIFY(inactiveWindow);
+    inactiveWindow->show();
+    QVERIFY(QTest::qWaitForWindowExposed(inactiveWindow));
 
-    QObject *shortcut = inactivewindow->property("shortcut").value<QObject *>();
+    QObject *shortcut = inactiveWindow->property("shortcut").value<QObject *>();
     QVERIFY(shortcut);
 
     shortcut->setProperty("enabled", enabled);
@@ -492,18 +576,22 @@ void tst_QQuickShortcut::contextChange()
     shortcut->setProperty("context", Qt::WindowShortcut);
 
     engine.load(testFileUrl("multiple.qml"));
-    QQuickWindow *activewindow = qobject_cast<QQuickWindow *>(engine.rootObjects().value(1));
-    QVERIFY(activewindow);
-    activewindow->show();
-    QVERIFY(QTest::qWaitForWindowExposed(activewindow));
+    QQuickWindow *activeWindow = qobject_cast<QQuickWindow *>(engine.rootObjects().value(1));
+    QVERIFY(activeWindow);
+    activeWindow->show();
+    QVERIFY(QTest::qWaitForWindowExposed(activeWindow));
+    activeWindow->requestActivate();
+    QVERIFY(QTest::qWaitForWindowActive(activeWindow));
 
-    QTest::keyPress(activewindow, key, modifiers);
-    QCOMPARE(inactivewindow->property("activated").toBool(), false);
+    QTest::keyPress(activeWindow, key, modifiers);
+    QTest::keyRelease(activeWindow, key, modifiers);
+    QCOMPARE(inactiveWindow->property("activated").toBool(), false);
 
     shortcut->setProperty("context", Qt::ApplicationShortcut);
 
-    QTest::keyPress(activewindow, key, modifiers);
-    QCOMPARE(inactivewindow->property("activated").toBool(), activated);
+    QTest::keyPress(activeWindow, key, modifiers);
+    QTest::keyRelease(activeWindow, key, modifiers);
+    QCOMPARE(inactiveWindow->property("activated").toBool(), activated);
 }
 
 #ifdef QT_QUICKWIDGETS_LIB
@@ -619,6 +707,7 @@ void tst_QQuickShortcut::quickWidgetShortcuts()
     QQuickItem* item = quickWidget->rootObject();
     item->setProperty("shortcuts", shortcuts);
     QTest::keyPress(quickWidget->quickWindow(), key, modifiers, 1500);
+    QTest::keyRelease(quickWidget->quickWindow(), key, modifiers, 1600);
     QCOMPARE(item->property("activatedShortcut").toString(), activatedShortcut);
     QCOMPARE(item->property("ambiguousShortcut").toString(), ambiguousShortcut);
 }