Merge lp:~mzanetti/unity8/cancel-alt-instead-of-delay into lp:unity8

[Michael Zanetti]

send a Alt-release event on alt+tab instead of delaying it completely and invoking it later

 * Are there any related MPs required for this MP to build/function as expected? Please list.

nope

 * Did you perform an exploratory manual test run of your code change and any related functionality?

yip yip

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

nope

 * If you changed the UI, has there been a design review?

nope

[Lukáš Tinkl (lukas-kde)]

Yup, AltGr works \o/

* Did you perform an exploratory manual test run of the code change and any related functionality?

Yes

* Did CI run pass? If not, please explain why.

Not yet, will wait for it to top approve

review: Approve

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2442
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1444/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1921
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/983
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/983
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/983
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1947
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1883
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1883
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1883
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1874/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1444/rebuild

review: Needs Fixing (continuous-integration)

=== modified file 'qml/Components/PhysicalKeysMapper.qml'
--- qml/Components/PhysicalKeysMapper.qml	2016-02-02 15:21:44 +0000
+++ qml/Components/PhysicalKeysMapper.qml	2016-06-08 16:25:15 +0000
@@ -66,11 +66,6 @@
         property bool superTabPressed: false
 
         property var powerButtonPressStart: 0
-
-        // We need to eat ALT presses until we know what they're for (Alt+Tab or going to the app?)
-        // Once we know if an ALT keypress is for the app, we need to re-inject the pressed event for it
-        // but we must only do that once.
-        property bool altPressInjected: false
     }
 
     InputEventGenerator {
@@ -78,12 +73,6 @@
     }
 
     function onKeyPressed(event, currentEventTimestamp) {
-        if (d.altPressed && !d.altTabPressed && event.key !== Qt.Key_Tab && event.key !== Qt.Key_Alt && !d.altPressInjected) {
-            // ALT is pressed and another key that is not Tab has been received. Re-inject the alt pressed event
-            d.altPressInjected = true;
-            inputEventGenerator.generateKeyEvent(Qt.Key_Alt, true, Qt.NoModifier, currentEventTimestamp - 1, 56);
-        }
-
         if (event.key == Qt.Key_PowerDown || event.key == Qt.Key_PowerOff) {
             if (event.isAutoRepeat) {
                 if (d.powerButtonPressStart > 0
@@ -119,12 +108,7 @@
                 d.volumeUpKeyPressed = true;
             }
         } else if (event.key == Qt.Key_Alt || (root.controlInsteadOfAlt && event.key == Qt.Key_Control)) {
-            if (!d.altPressed || event.isAutoRepeat) {
-                // Only eat it if it's the first time we receive alt pressed (or if it's the autorepeat of the first press)
-                d.altPressed = true;
-                event.accepted = true;
-                d.altPressInjected = false;
-            }
+            d.altPressed = true;
 
         // Adding MetaModifier here because that's what keyboards do. Pressing Super_L actually gives
         // Super_L + MetaModifier. This helps to make sure we only invoke superPressed if no other
@@ -135,6 +119,7 @@
             d.superPressed = true;
         } else if (event.key == Qt.Key_Tab) {
             if (d.altPressed && !d.altTabPressed) {
+                inputEventGenerator.generateKeyEvent(Qt.Key_Alt, false, Qt.NoModifier, currentEventTimestamp, 56);
                 d.altTabPressed = true;
                 event.accepted = true;
             }
@@ -161,11 +146,6 @@
             if (d.altTabPressed) {
                 d.altTabPressed = false;
                 event.accepted = true;
-            } else if (d.altPressed && !d.altPressInjected) {
-                // Alt was released but nothing else. Let's inject a pressed event and also forward the release.
-                d.altPressInjected = true;
-                inputEventGenerator.generateKeyEvent(Qt.Key_Alt, true, Qt.AltModifer, currentEventTimestamp, 56);
-                d.altPressInjected = false;
             }
             d.altPressed = false;
         } else if (event.key == Qt.Key_Tab) {

=== modified file 'tests/qmltests/Components/tst_PhysicalKeysMapper.qml'
--- tests/qmltests/Components/tst_PhysicalKeysMapper.qml	2016-03-29 03:47:39 +0000
+++ tests/qmltests/Components/tst_PhysicalKeysMapper.qml	2016-06-08 16:25:15 +0000
@@ -134,40 +134,31 @@
             compare(screenshotSpy.count, 1);
         }
 
-        function test_altIsDispatchedOnRelease() {
-            // Press alt, make sure it does *not* end up in inputCatcher (aka, the focused app)
+        function test_altIsCancelledOnAltTab() {
+            // Press alt, make sure it does end up in inputCatcher (aka, the focused app)
+            inputCatcher.pressedKeys = [];
             keyPress(Qt.Key_Alt, Qt.NoModifier)
-            compare(inputCatcher.pressedKeys.length, 0);
-            // Now release alt. It should cause the previous Alt press to be dispatched along with the release
-            keyRelease(Qt.Key_Alt, Qt.NoModifier);
             compare(inputCatcher.pressedKeys.length, 1);
             compare(inputCatcher.pressedKeys[0], Qt.Key_Alt);
+            // Now also press tab. As this should trigger the spread, alt needs to be released for the app
+            keyPress(Qt.Key_Tab, Qt.NoModifier)
             compare(inputCatcher.releasedKeys.length, 1);
             compare(inputCatcher.releasedKeys[0], Qt.Key_Alt);
-        }
-
-        function test_altIsNotDispatchedOnAltTab() {
-            // Press alt, make sure it does *not* end up in inputCatcher (aka, the focused app)
-            inputCatcher.pressedKeys = [];
-            keyPress(Qt.Key_Alt, Qt.NoModifier)
-            compare(inputCatcher.pressedKeys.length, 0);
-            // Now also press tab. As this should trigger the spread, neither of them should end up in the app
-            keyPress(Qt.Key_Tab, Qt.NoModifier)
-            compare(inputCatcher.pressedKeys.length, 0);
-
-            // Also the release events should not be dispatched to the app
+
+            // Now release the keys, none of that should get to the app any more
             keyRelease(Qt.Key_Tab, Qt.NoModifier);
             keyRelease(Qt.Key_Alt, Qt.NoModifier);
-            compare(inputCatcher.pressedKeys.length, 0);
-            compare(inputCatcher.releasedKeys.length, 0);
+            compare(inputCatcher.pressedKeys.length, 1);
+            compare(inputCatcher.releasedKeys.length, 1);
         }
 
         function test_altComboIsDispatched() {
             inputCatcher.pressedKeys = [];
-            // Press alt, make sure it does *not* yet end up in inputCatcher (aka, the focused app)
+            // Press alt, make sure it does end up in inputCatcher (aka, the focused app)
             keyPress(Qt.Key_Alt, Qt.NoModifier)
-            compare(inputCatcher.pressedKeys.length, 0);
-            // Now press F in order to opening the File menu (Alt+F), now the app should get the full combo
+            compare(inputCatcher.pressedKeys.length, 1);
+            compare(inputCatcher.pressedKeys[0], Qt.Key_Alt);
+            // Now press F in order to opening the File menu (Alt+F), make sure it is dispatched too
             keyPress(Qt.Key_F, Qt.NoModifier)
             compare(inputCatcher.pressedKeys.length, 2);
             compare(inputCatcher.pressedKeys[0], Qt.Key_Alt);