QKeySequence: tolerate spaces when parsing a key sequence string

Fixes: QTBUG-130063
Pick-to: 6.9 6.8
Change-Id: I0f0aff0ca0824cd1c9297cd18a1f5cf9a2a44951
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

Author: dfaure-kdab

src/gui/kernel/qkeysequence.cpp

@@ -1102,10 +1102,16 @@ QKeyCombination QKeySequencePrivate::decodeString(QString accel, QKeySequence::S
         return Qt::Key_unknown;
 #endif
 
+    int singlePlus = -1;
     qsizetype i = 0;
     qsizetype lastI = 0;
     while ((i = sl.indexOf(u'+', i + 1)) != -1) {
-        const QStringView sub = QStringView{sl}.mid(lastI, i - lastI + 1);
+        QStringView sub = QStringView{ sl }.mid(lastI, i - lastI + 1);
+        while (sub.size() > 1 && sub.at(0) == QLatin1Char(' ')) {
+            sub = sub.mid(1);
+            ++lastI;
+        }
+
         // If we get here the shortcuts contains at least one '+'. We break up
         // along the following strategy:
         //      Meta+Ctrl++   ( "Meta+", "Ctrl+", "+" )
@@ -1117,33 +1123,49 @@ QKeyCombination QKeySequencePrivate::decodeString(QString accel, QKeySequence::S
         // Only '+' can have length 1.
         if (sub.size() == 1) {
             // Make sure we only encounter a single '+' at the end of the accel
-            if (accel.lastIndexOf(u'+') != accel.size()-1)
+            if (singlePlus >= 0)
                 return Qt::Key_unknown;
+            singlePlus = lastI;
         } else {
-            // Identify the modifier
-            bool validModifier = false;
-            for (int j = 0; j < modifs.size(); ++j) {
-                const QModifKeyName &mkf = modifs.at(j);
-                if (sub == mkf.name) {
-                    ret |= mkf.qt_key;
-                    validModifier = true;
-                    break; // Shortcut, since if we find an other it would/should just be a dup
+
+            const auto identifyModifier = [&](QStringView sub) {
+                for (int j = 0; j < modifs.size(); ++j) {
+                    const QModifKeyName &mkf = modifs.at(j);
+                    if (sub == mkf.name) {
+                        ret |= mkf.qt_key;
+                        return true; // Shortcut, since if we find another it would/should just be a dup
+                    }
                 }
-            }
+                return false;
+            };
 
+            bool validModifier = identifyModifier(sub);
+
+            if (!validModifier) {
+                // Try harder with slower code that trims spaces
+                const QString cleanedSub = sub.toString().remove(QLatin1Char(' '));
+                validModifier = identifyModifier(cleanedSub);
+            }
             if (!validModifier)
                 return Qt::Key_unknown;
         }
         lastI = i + 1;
     }
 
-    qsizetype p = accel.lastIndexOf(u'+', accel.size() - 2); // -2 so that Ctrl++ works
+    qsizetype p = accel.lastIndexOf(u'+', singlePlus > 0 ? singlePlus - 1 : accel.size() - 1);
     QStringView accelRef(accel);
     if (p > 0)
         accelRef = accelRef.mid(p + 1);
 
+    while (accelRef.size() > 1 && accelRef.at(0) == QLatin1Char(' '))
+        accelRef = accelRef.mid(1);
+    while (accelRef.size() > 1 && accelRef.endsWith(QLatin1Char(' ')))
+        accelRef.chop(1);
+
     int fnum = 0;
-    if (accelRef.size() == 1) {
+    if (accelRef.isEmpty())
+        return Qt::Key_unknown;
+    else if (accelRef.size() == 1) {
 #if defined(Q_OS_APPLE)
         int qtKey = qtkeyForAppleSymbol(accelRef.at(0));
         if (qtKey != -1) {

tests/auto/gui/kernel/qkeysequence/tst_qkeysequence.cpp

@@ -543,6 +543,20 @@ void tst_QKeySequence::parseString_data()
     QTest::newRow("Meta+A") << "Meta+a" <<  QKeySequence(Qt::META | Qt::Key_A);
     QTest::newRow("mEtA+A") << "mEtA+a" <<  QKeySequence(Qt::META | Qt::Key_A);
     QTest::newRow("Ctrl++") << "Ctrl++" << QKeySequence(Qt::CTRL | Qt::Key_Plus);
+    QTest::newRow("+") << "+" << QKeySequence(Qt::Key_Plus);
+
+    // Tolerance for spaces
+    QTest::newRow("Ctrl_+_Del") << "Ctrl + Del" << QKeySequence(Qt::CTRL | Qt::Key_Delete);
+    QTest::newRow("Ctrl+Del_") << "Ctrl+Del " << QKeySequence(Qt::CTRL | Qt::Key_Delete);
+    QTest::newRow("Ctrl_+_Del_") << "Ctrl + Del " << QKeySequence(Qt::CTRL | Qt::Key_Delete);
+    QTest::newRow("space") << " " << QKeySequence(Qt::Key_Space);
+    QTest::newRow("Ctrl+space") << "Ctrl+ " << QKeySequence(Qt::CTRL | Qt::Key_Space);
+    QTest::newRow("Ctrl_++") << "Ctrl ++" << QKeySequence(Qt::CTRL | Qt::Key_Plus);
+    QTest::newRow("Ctrl_+_+") << "Ctrl + +" << QKeySequence(Qt::CTRL | Qt::Key_Plus);
+    QTest::newRow("Ctrl_+_+_") << "Ctrl + + " << QKeySequence(Qt::CTRL | Qt::Key_Plus);
+    QTest::newRow("+_") << "+ " << QKeySequence(Qt::Key_Plus);
+    QTest::newRow("_+") << " +" << QKeySequence(Qt::Key_Plus);
+    QTest::newRow("_+_") << " + " << QKeySequence(Qt::Key_Plus);
 
     // Invalid modifiers
     QTest::newRow("Win+A") << "Win+a" <<  QKeySequence(Qt::Key_unknown);
@@ -557,7 +571,9 @@ void tst_QKeySequence::parseString_data()
     QTest::newRow("4+3=2") << "4+3=2" <<  QKeySequence(Qt::Key_unknown);
     QTest::newRow("Alabama") << "Alabama" << QKeySequence(Qt::Key_unknown);
     QTest::newRow("Simon+G") << "Simon+G" << QKeySequence(Qt::Key_unknown);
-    QTest::newRow("Shift+++2") << "Shift+++2" <<  QKeySequence(Qt::Key_unknown);
+    QTest::newRow("Shift+++2") << "Shift+++2" << QKeySequence(Qt::Key_unknown);
+    QTest::newRow("Ctrl+D_el") << "Ctrl+D el" << QKeySequence(Qt::Key_unknown);
+    QTest::newRow("Ct_rl+D_el") << "Ct rl+D el" << QKeySequence(Qt::Key_unknown);
 
     // Wrong order
     QTest::newRow("A+Meta") << "a+Meta" <<  QKeySequence(Qt::Key_unknown);
@@ -570,6 +586,7 @@ void tst_QKeySequence::parseString_data()
     //QTest::newRow("Shift") << "Shift" << QKeySequence(Qt::SHIFT);
 
     // Incomplete
+    QTest::newRow("Ctrl+") << "Ctrl+" << QKeySequence(Qt::Key_unknown);
     QTest::newRow("Meta+Shift+") << "Meta+Shift+" << QKeySequence(Qt::Key_unknown);
 }