ComboBox: don't block the escape/back key (with fixed test)

Accept Key_Escape or Key_Back only if it actually close the popup. If
the popup is not visible, the key is not handled, and the event should
therefore propagate (and potentially close the app on Android).

Note: def92f7 had a broken test (didn't take the popup exit transition
into account) and thus, blocked the CI and got rejected in 110b718.

Task-number: QTBUG-67684
Change-Id: I46b4731b3006721f3737cf909fbd97331ac6d8ed
Reviewed-by: Liang Qi <liang.qi@qt.io>
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

2 files changed, 85 insertions, 3 deletions

diff --git a/src/quicktemplates2/qquickcombobox.cpp b/src/quicktemplates2/qquickcombobox.cpp
index d3226baf..4d5efece 100644
--- a/src/quicktemplates2/qquickcombobox.cpp
+++ b/src/quicktemplates2/qquickcombobox.cpp
@@ -1624,9 +1624,11 @@ void QQuickComboBox::keyReleaseEvent(QKeyEvent *event)
         break;
     case Qt::Key_Escape:
     case Qt::Key_Back:
-        d->hidePopup(false);
-        setPressed(false);
-        event->accept();
+        if (d->isPopupVisible()) {
+            d->hidePopup(false);
+            setPressed(false);
+            event->accept();
+        }
         break;
     default:
         break;
diff --git a/tests/auto/controls/data/tst_combobox.qml b/tests/auto/controls/data/tst_combobox.qml
index 801712be..8e8f9c2f 100644
--- a/tests/auto/controls/data/tst_combobox.qml
+++ b/tests/auto/controls/data/tst_combobox.qml
@@ -1600,4 +1600,84 @@ TestCase {
         tryCompare(control.popup, "visible", true)
         compare(control.popup.height, control.popup.topPadding + control.popup.bottomPadding)
     }
+
+    Component {
+        id: keysMonitor
+        Item {
+            property int pressedKeys: 0
+            property int releasedKeys: 0
+            property int lastPressedKey: 0
+            property int lastReleasedKey: 0
+            property alias comboBox: comboBox
+
+            width: 200
+            height: 200
+
+            Keys.onPressed: { ++pressedKeys; lastPressedKey = event.key }
+            Keys.onReleased: { ++releasedKeys; lastReleasedKey = event.key }
+
+            ComboBox {
+                id: comboBox
+            }
+        }
+    }
+
+    function test_keyClose_data() {
+        return [
+            { tag: "Escape", key: Qt.Key_Escape },
+            { tag: "Back", key: Qt.Key_Back }
+        ]
+    }
+
+    function test_keyClose(data) {
+        var container = createTemporaryObject(keysMonitor, testCase)
+        verify(container)
+
+        var control = comboBox.createObject(container)
+        verify(control)
+
+        control.forceActiveFocus()
+        verify(control.activeFocus)
+
+        var pressedKeys = 0
+        var releasedKeys = 0
+
+        // popup not visible -> propagates
+        keyPress(data.key)
+        compare(container.pressedKeys, ++pressedKeys)
+        compare(container.lastPressedKey, data.key)
+
+        keyRelease(data.key)
+        compare(container.releasedKeys, ++releasedKeys)
+        compare(container.lastReleasedKey, data.key)
+
+        verify(control.activeFocus)
+
+        // popup visible -> handled -> does not propagate
+        control.popup.open()
+        tryCompare(control.popup, "opened", true)
+
+        keyPress(data.key)
+        compare(container.pressedKeys, pressedKeys)
+
+        keyRelease(data.key)
+        // Popup receives the key release event if it has an exit transition, but
+        // not if it has been immediately closed on press, without a transition.
+        // ### TODO: Should Popup somehow always block the key release event?
+        if (!control.popup.exit)
+            ++releasedKeys
+        compare(container.releasedKeys, releasedKeys)
+
+        tryCompare(control.popup, "visible", false)
+        verify(control.activeFocus)
+
+        // popup not visible -> propagates
+        keyPress(data.key)
+        compare(container.pressedKeys, ++pressedKeys)
+        compare(container.lastPressedKey, data.key)
+
+        keyRelease(data.key)
+        compare(container.releasedKeys, ++releasedKeys)
+        compare(container.lastReleasedKey, data.key)
+    }
 }