QKeySequence: add tests, fix handling of '+' as key or separator.

Change-Id: I3c471dff9b46025aab762d36bb3a3adc64ced561
Reviewed-by: Morten Johan Sørvig <morten.sorvig@nokia.com>
Reviewed-by: Lars Knoll <lars.knoll@nokia.com>

2 files changed, 47 insertions, 9 deletions

diff --git a/src/gui/kernel/qkeysequence.cpp b/src/gui/kernel/qkeysequence.cpp
index 34457a9cee..83796bb550 100644
--- a/src/gui/kernel/qkeysequence.cpp
+++ b/src/gui/kernel/qkeysequence.cpp
@@ -1255,10 +1255,21 @@ int QKeySequencePrivate::decodeString(const QString &str, QKeySequence::Sequence
     int lastI = 0;
     while ((i = sl.indexOf(QLatin1Char('+'), i + 1)) != -1) {
         const QString sub = sl.mid(lastI, i - lastI + 1);
-        // Just shortcut the check here if we only have one character.
-        // Rational: A modifier will contain the name AND +, so longer than 1, a length of 1 is just
-        // the remaining part of the shortcut (ei. The 'C' in "Ctrl+C"), so no need to check that.
-        if (sub.length() > 1) {
+        // If we get here the shortcuts contains at least one '+'. We break up
+        // along the following strategy:
+        //      Meta+Ctrl++   ( "Meta+", "Ctrl+", "+" )
+        //      Super+Shift+A ( "Super+", "Shift+" )
+        //      4+3+2=1       ( "4+", "3+" )
+        // In other words, everything we try to handle HAS to be a modifier
+        // except for a single '+' at the end of the string.
+
+        // Only '+' can have length 1.
+        if (sub.length() == 1) {
+            // Make sure we only encounter a single '+' at the end of the accel
+            if (accel.lastIndexOf(QLatin1Char('+')) != accel.length()-1)
+                return Qt::Key_unknown;
+        } else {
+            // Identify the modifier
             bool validModifier = false;
             for (int j = 0; j < modifs.size(); ++j) {
                 const QModifKeyName &mkf = modifs.at(j);
@@ -1268,9 +1279,7 @@ int QKeySequencePrivate::decodeString(const QString &str, QKeySequence::Sequence
                     break; // Shortcut, since if we find an other it would/should just be a dup
                 }
             }
-            // We couldn't match the string with a modifier. This is only
-            // possible if this part is the key. The key is never followed by a
-            // '+'. And if the key is '+' the if() above would have skipped it.
+
             if (!validModifier)
                 return Qt::Key_unknown;
         }
diff --git a/tests/auto/gui/kernel/qkeysequence/tst_qkeysequence.cpp b/tests/auto/gui/kernel/qkeysequence/tst_qkeysequence.cpp
index 858ffd9aa4..31610cdbcb 100644
--- a/tests/auto/gui/kernel/qkeysequence/tst_qkeysequence.cpp
+++ b/tests/auto/gui/kernel/qkeysequence/tst_qkeysequence.cpp
@@ -529,17 +529,46 @@ void tst_QKeySequence::parseString_data()
     QTest::addColumn<QString>("strSequence");
     QTest::addColumn<QKeySequence>("keycode");
 
+    // Valid
     QTest::newRow("A") << "A" << QKeySequence(Qt::Key_A);
     QTest::newRow("a") << "a" << QKeySequence(Qt::Key_A);
     QTest::newRow("Ctrl+Left") << "Ctrl+Left" << QKeySequence(Qt::CTRL + Qt::Key_Left);
-    QTest::newRow("Ctrl++") << "Ctrl++" << QKeySequence(Qt::CTRL + Qt::Key_Plus);
+    QTest::newRow("CTRL+LEFT") << "CTRL+LEFT" << QKeySequence(Qt::CTRL + Qt::Key_Left);
     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);
+
+    // Invalid modifiers
     QTest::newRow("Win+A") << "Win+a" <<  QKeySequence(Qt::Key_unknown);
-    QTest::newRow("4+3=2") << "4+3=2" <<  QKeySequence(Qt::Key_unknown);
     QTest::newRow("Super+Meta+A") << "Super+Meta+A" << QKeySequence(Qt::Key_unknown);
+
+    // Invalid Keys
     QTest::newRow("Meta+Trolls") << "Meta+Trolls" << QKeySequence(Qt::Key_unknown);
+    QTest::newRow("Meta+Period") << "Meta+Period" << QKeySequence(Qt::Key_unknown);
+    QTest::newRow("Meta+Ypsilon") << "Meta+Period" << QKeySequence(Qt::Key_unknown);
+
+    // Garbage
+    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);
+
+    // Wrong order
+    QTest::newRow("A+Meta") << "a+Meta" <<  QKeySequence(Qt::Key_unknown);
+    QTest::newRow("Meta+++Shift") << "Meta+++Shift" <<  QKeySequence(Qt::Key_unknown);
+    QTest::newRow("Meta+a+Shift") << "Meta+a+Shift" <<  QKeySequence(Qt::Key_unknown);
+
+    // Only Modifiers - currently not supported
+    //QTest::newRow("Meta+Shift") << "Meta+Shift" << QKeySequence(Qt::META + Qt::SHIFT);
+    //QTest::newRow("Ctrl") << "Ctrl" << QKeySequence(Qt::CTRL);
+    //QTest::newRow("Shift") << "Shift" << QKeySequence(Qt::SHIFT);
+
+    // Only Keys
+    QTest::newRow("a") << "a" << QKeySequence(Qt::Key_A);
+    QTest::newRow("A") << "A" << QKeySequence(Qt::Key_A);
+
+    // Incomplete
+    QTest::newRow("Meta+Shift+") << "Meta+Shift+" << QKeySequence(Qt::Key_unknown);
 }
 
 void tst_QKeySequence::parseString()