Unity 8

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

  1. cancel-alt-instead-of-delay
  2. Merge into trunk
Proposed by Michael Zanetti
Status: Merged
Approved by: Lukáš Tinkl
Approved revision: 2442
Merged at revision: 2479
Proposed branch: lp:~mzanetti/unity8/cancel-alt-instead-of-delay
Merge into: lp:unity8
Diff against target: 120 lines (+15/-44)
2 files modified
qml/Components/PhysicalKeysMapper.qml (+2/-22)
tests/qmltests/Components/tst_PhysicalKeysMapper.qml (+13/-22)
To merge this branch: bzr merge lp:~mzanetti/unity8/cancel-alt-instead-of-delay
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Needs Fixing
Lukáš Tinkl (community) Approve
Review via email: mp+296818@code.launchpad.net

Commit message

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

Description of the change

 * 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

To post a comment you must log in.
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

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
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
66 property bool superTabPressed: false66 property bool superTabPressed: false
6767
68 property var powerButtonPressStart: 068 property var powerButtonPressStart: 0
69
70 // We need to eat ALT presses until we know what they're for (Alt+Tab or going to the app?)
71 // Once we know if an ALT keypress is for the app, we need to re-inject the pressed event for it
72 // but we must only do that once.
73 property bool altPressInjected: false
74 }69 }
7570
76 InputEventGenerator {71 InputEventGenerator {
@@ -78,12 +73,6 @@
78 }73 }
7974
80 function onKeyPressed(event, currentEventTimestamp) {75 function onKeyPressed(event, currentEventTimestamp) {
81 if (d.altPressed && !d.altTabPressed && event.key !== Qt.Key_Tab && event.key !== Qt.Key_Alt && !d.altPressInjected) {
82 // ALT is pressed and another key that is not Tab has been received. Re-inject the alt pressed event
83 d.altPressInjected = true;
84 inputEventGenerator.generateKeyEvent(Qt.Key_Alt, true, Qt.NoModifier, currentEventTimestamp - 1, 56);
85 }
86
87 if (event.key == Qt.Key_PowerDown || event.key == Qt.Key_PowerOff) {76 if (event.key == Qt.Key_PowerDown || event.key == Qt.Key_PowerOff) {
88 if (event.isAutoRepeat) {77 if (event.isAutoRepeat) {
89 if (d.powerButtonPressStart > 078 if (d.powerButtonPressStart > 0
@@ -119,12 +108,7 @@
119 d.volumeUpKeyPressed = true;108 d.volumeUpKeyPressed = true;
120 }109 }
121 } else if (event.key == Qt.Key_Alt || (root.controlInsteadOfAlt && event.key == Qt.Key_Control)) {110 } else if (event.key == Qt.Key_Alt || (root.controlInsteadOfAlt && event.key == Qt.Key_Control)) {
122 if (!d.altPressed || event.isAutoRepeat) {111 d.altPressed = true;
123 // Only eat it if it's the first time we receive alt pressed (or if it's the autorepeat of the first press)
124 d.altPressed = true;
125 event.accepted = true;
126 d.altPressInjected = false;
127 }
128112
129 // Adding MetaModifier here because that's what keyboards do. Pressing Super_L actually gives113 // Adding MetaModifier here because that's what keyboards do. Pressing Super_L actually gives
130 // Super_L + MetaModifier. This helps to make sure we only invoke superPressed if no other114 // Super_L + MetaModifier. This helps to make sure we only invoke superPressed if no other
@@ -135,6 +119,7 @@
135 d.superPressed = true;119 d.superPressed = true;
136 } else if (event.key == Qt.Key_Tab) {120 } else if (event.key == Qt.Key_Tab) {
137 if (d.altPressed && !d.altTabPressed) {121 if (d.altPressed && !d.altTabPressed) {
122 inputEventGenerator.generateKeyEvent(Qt.Key_Alt, false, Qt.NoModifier, currentEventTimestamp, 56);
138 d.altTabPressed = true;123 d.altTabPressed = true;
139 event.accepted = true;124 event.accepted = true;
140 }125 }
@@ -161,11 +146,6 @@
161 if (d.altTabPressed) {146 if (d.altTabPressed) {
162 d.altTabPressed = false;147 d.altTabPressed = false;
163 event.accepted = true;148 event.accepted = true;
164 } else if (d.altPressed && !d.altPressInjected) {
165 // Alt was released but nothing else. Let's inject a pressed event and also forward the release.
166 d.altPressInjected = true;
167 inputEventGenerator.generateKeyEvent(Qt.Key_Alt, true, Qt.AltModifer, currentEventTimestamp, 56);
168 d.altPressInjected = false;
169 }149 }
170 d.altPressed = false;150 d.altPressed = false;
171 } else if (event.key == Qt.Key_Tab) {151 } else if (event.key == Qt.Key_Tab) {
172152
=== 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 @@
134 compare(screenshotSpy.count, 1);134 compare(screenshotSpy.count, 1);
135 }135 }
136136
137 function test_altIsDispatchedOnRelease() {137 function test_altIsCancelledOnAltTab() {
138 // Press alt, make sure it does *not* end up in inputCatcher (aka, the focused app)138 // Press alt, make sure it does end up in inputCatcher (aka, the focused app)
139 inputCatcher.pressedKeys = [];
139 keyPress(Qt.Key_Alt, Qt.NoModifier)140 keyPress(Qt.Key_Alt, Qt.NoModifier)
140 compare(inputCatcher.pressedKeys.length, 0);
141 // Now release alt. It should cause the previous Alt press to be dispatched along with the release
142 keyRelease(Qt.Key_Alt, Qt.NoModifier);
143 compare(inputCatcher.pressedKeys.length, 1);141 compare(inputCatcher.pressedKeys.length, 1);
144 compare(inputCatcher.pressedKeys[0], Qt.Key_Alt);142 compare(inputCatcher.pressedKeys[0], Qt.Key_Alt);
143 // Now also press tab. As this should trigger the spread, alt needs to be released for the app
144 keyPress(Qt.Key_Tab, Qt.NoModifier)
145 compare(inputCatcher.releasedKeys.length, 1);145 compare(inputCatcher.releasedKeys.length, 1);
146 compare(inputCatcher.releasedKeys[0], Qt.Key_Alt);146 compare(inputCatcher.releasedKeys[0], Qt.Key_Alt);
147 }147
148148 // Now release the keys, none of that should get to the app any more
149 function test_altIsNotDispatchedOnAltTab() {
150 // Press alt, make sure it does *not* end up in inputCatcher (aka, the focused app)
151 inputCatcher.pressedKeys = [];
152 keyPress(Qt.Key_Alt, Qt.NoModifier)
153 compare(inputCatcher.pressedKeys.length, 0);
154 // Now also press tab. As this should trigger the spread, neither of them should end up in the app
155 keyPress(Qt.Key_Tab, Qt.NoModifier)
156 compare(inputCatcher.pressedKeys.length, 0);
157
158 // Also the release events should not be dispatched to the app
159 keyRelease(Qt.Key_Tab, Qt.NoModifier);149 keyRelease(Qt.Key_Tab, Qt.NoModifier);
160 keyRelease(Qt.Key_Alt, Qt.NoModifier);150 keyRelease(Qt.Key_Alt, Qt.NoModifier);
161 compare(inputCatcher.pressedKeys.length, 0);151 compare(inputCatcher.pressedKeys.length, 1);
162 compare(inputCatcher.releasedKeys.length, 0);152 compare(inputCatcher.releasedKeys.length, 1);
163 }153 }
164154
165 function test_altComboIsDispatched() {155 function test_altComboIsDispatched() {
166 inputCatcher.pressedKeys = [];156 inputCatcher.pressedKeys = [];
167 // Press alt, make sure it does *not* yet end up in inputCatcher (aka, the focused app)157 // Press alt, make sure it does end up in inputCatcher (aka, the focused app)
168 keyPress(Qt.Key_Alt, Qt.NoModifier)158 keyPress(Qt.Key_Alt, Qt.NoModifier)
169 compare(inputCatcher.pressedKeys.length, 0);159 compare(inputCatcher.pressedKeys.length, 1);
170 // Now press F in order to opening the File menu (Alt+F), now the app should get the full combo160 compare(inputCatcher.pressedKeys[0], Qt.Key_Alt);
161 // Now press F in order to opening the File menu (Alt+F), make sure it is dispatched too
171 keyPress(Qt.Key_F, Qt.NoModifier)162 keyPress(Qt.Key_F, Qt.NoModifier)
172 compare(inputCatcher.pressedKeys.length, 2);163 compare(inputCatcher.pressedKeys.length, 2);
173 compare(inputCatcher.pressedKeys[0], Qt.Key_Alt);164 compare(inputCatcher.pressedKeys[0], Qt.Key_Alt);

Subscribers

People subscribed via source and target branches