Merge lp:~lukas-kde/unity8/eatDecorationMouseEvents into lp:unity8

[Lukáš Tinkl]

Do not let mouse and wheel events pass thru the titlebar

Do not let mouse and wheel events pass thru the titlebar

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

No

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

Yes

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

N/A

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

N/A

[Daniel d'Andrada (dandrader)]

"""
- hoverEnabled: true
"""

So you never needed hover events in that MouseArea?

[Lukáš Tinkl (lukas-kde)]

> """
> - hoverEnabled: true
> """
>
> So you never needed hover events in that MouseArea?

Correct, they are needed in WindowControlButtons but not here

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

PASSED: Continuous integration, rev:2442
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1376/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1841
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/925
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/925
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/925
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1867
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1805
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1805
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1805
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1796
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1796/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1796
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1796/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1796
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1796/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1796
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1796/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1796
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1796/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1796
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1796/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1796
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1796/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1796
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1796/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1796
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1796/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

You're now able to move the window around by dragging with any mouse button pressed.

Please write a regression test for that

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

What about having the test in a tst_WindowDecoration instead of in tst_DesktopStage? No need to involve the whole desktop stage.

[Daniel d'Andrada (dandrader)]

And please try to make tests a such a way that they can be executed manually in "make tryFoo". For this specific case, see "make tryPanel" for an example. It's background goes read when a mouse press reaches it.

[Daniel d'Andrada (dandrader)]

> It's background goes read when a mouse press reaches it.

s/It's/Its
s/read/red

[lp:~lukas-kde/unity8/eatDecorationMouseEvents]

2443. By Lukáš Tinkl on 2016-06-09

don't accept drags with button != left; fix the tests

[Lukáš Tinkl (lukas-kde)]

> And please try to make tests a such a way that they can be executed manually
> in "make tryFoo". For this specific case, see "make tryPanel" for an example.
> It's background goes read when a mouse press reaches it.

The functionality can be tried out in either make tryShell or make tryDesktopStage.

[Lukáš Tinkl (lukas-kde)]

> You're now able to move the window around by dragging with any mouse button
> pressed.
>
> Please write a regression test for that

Thanks for spotting, fixed! Also corrected the test so that it checks for the "pressed" signal instead of "clicked" (which was the source of this "regression").

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

FAILED: Continuous integration, rev:2443
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1455/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1933
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/993
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/993
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/993
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1959
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1894
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1894
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1894
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1885
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1885/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1885
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1885/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1885
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1885/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1885
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1885/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1885
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1885/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1885
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1885/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1885
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1885/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1885
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1885/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1885
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1885/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Daniel d'Andrada (dandrader)]

Imagine the replacement of all stages with a single, new, one that mzanetti is working on. With this branch, there's yet another test he has to juggle around and mind about. But it's unnecessary as the test is not about stages code. It's a DecoratedWindow internal detail he shouldn't bother about when refactoring stages code.

It would make our qmltests code base more modular if this was moved to a tst_DecoratedWindow or a tst_WindowDecoration (ie, having it as, or closer to being, a unit test). Unless you think it's interesting to also exercise surrounding code along with it (ie, as a integration test). But following this rationale we could just as well move it to tst_OrientedShell...

In short: I believe this test looks more like (as is best as) a unit test then a integration test

[Daniel d'Andrada (dandrader)]

- Move mouse over the window decoration
   * mouse cursor shape is an arrow)
- Press left mouse button over the window decoration
- Move mouse (window is dragged along)
   * mouse cursor shape is a closed hand
- Lift left mouse button

expected outcome:
Mouse cursor shape is an arrow once again

actual outcome:
Mouse cursor shape remains a closed hand. Moving it over a window border fixes it.

review: Needs Fixing

[lp:~lukas-kde/unity8/eatDecorationMouseEvents]

2444. By Lukáš Tinkl on 2016-06-15

consolidate the mouse deco test

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

FAILED: Continuous integration, rev:2444
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1486/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/1982/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2008/console

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

review: Needs Fixing (continuous-integration)

[lp:~lukas-kde/unity8/eatDecorationMouseEvents]

2445. By Lukáš Tinkl on 2016-06-15

add a test for window (decoration) dragging using different mouse buttons

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

FAILED: Continuous integration, rev:2445
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1487/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/1983/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2009/console

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

review: Needs Fixing (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

> - Move mouse over the window decoration
> * mouse cursor shape is an arrow)
> - Press left mouse button over the window decoration
> - Move mouse (window is dragged along)
> * mouse cursor shape is a closed hand
> - Lift left mouse button
>
> expected outcome:
> Mouse cursor shape is an arrow once again
>
> actual outcome:
> Mouse cursor shape remains a closed hand. Moving it over a window border fixes
> it.

Fixed, and also added another test for checking for moving/dragging the window with different mouse buttons.

[lp:~lukas-kde/unity8/eatDecorationMouseEvents]

2446. By Lukáš Tinkl on 2016-06-15

merge lp:~lukas-kde/unity8/preventScrollThruShellElements

2447. By Lukáš Tinkl on 2016-06-20

merge trunk, resolve conflicts

Unmerged revisions

2441. By Launchpad Translations on behalf of unity-team on 2016-06-02

Launchpad automatic translations update.

=== modified file 'qml/Stages/WindowDecoration.qml'
--- qml/Stages/WindowDecoration.qml	2016-05-02 11:35:53 +0000
+++ qml/Stages/WindowDecoration.qml	2016-06-15 16:58:24 +0000
@@ -27,7 +27,7 @@
     property Item target
     property alias title: titleLabel.text
     property bool active: false
-    hoverEnabled: true
+    acceptedButtons: Qt.AllButtons // prevent leaking unhandled mouse events
 
     signal closeClicked()
     signal minimizeClicked()
@@ -35,7 +35,11 @@
     signal maximizeHorizontallyClicked()
     signal maximizeVerticallyClicked()
 
-    onDoubleClicked: root.maximizeClicked()
+    onDoubleClicked: {
+        if (mouse.button == Qt.LeftButton) {
+            root.maximizeClicked();
+        }
+    }
 
     QtObject {
         id: priv
@@ -45,7 +49,7 @@
     }
 
     onPressedChanged: {
-        if (pressed) {
+        if (pressed && pressedButtons == Qt.LeftButton) {
             var pos = mapToItem(root.target, mouseX, mouseY);
             priv.distanceX = pos.x;
             priv.distanceY = pos.y;
@@ -65,6 +69,9 @@
         }
     }
 
+    // do not let unhandled wheel event pass thru the decoration
+    onWheel: wheel.accepted = true;
+
     Rectangle {
         anchors.fill: parent
         anchors.bottomMargin: -radius

=== modified file 'tests/qmltests/Stages/tst_DesktopStage.qml'
--- tests/qmltests/Stages/tst_DesktopStage.qml	2016-05-06 08:16:07 +0000
+++ tests/qmltests/Stages/tst_DesktopStage.qml	2016-06-15 16:58:24 +0000
@@ -89,6 +89,13 @@
                 topLevelSurfaceList: topSurfaceList
             }
         }
+
+        MouseArea {
+            id: clickThroughTester
+            anchors.fill: desktopStageLoader.item
+            acceptedButtons: Qt.AllButtons
+            hoverEnabled: true
+        }
     }
 
     Rectangle {
@@ -140,6 +147,11 @@
         }
     }
 
+    SignalSpy {
+        id: mouseEaterSpy
+        target: clickThroughTester
+    }
+
     UnityTestCase {
         id: testCase
         name: "DesktopStage"
@@ -179,6 +191,7 @@
             desktopStageLoader.active = true;
             tryCompare(desktopStageLoader, "status", Loader.Ready);
 
+            mouseEaterSpy.clear();
         }
 
         function waitUntilAppSurfaceShowsUp(surfaceId) {
@@ -654,5 +667,56 @@
             var closeButton = findChild(dashAppDelegate, "closeWindowButton");
             tryCompare(closeButton, "visible", false);
         }
+
+        function test_eatWindowDecorationMouseEvents_data() {
+            return [
+                {tag: "left mouse click", signalName: "clicked", button: Qt.LeftButton },
+                {tag: "right mouse click", signalName: "clicked", button: Qt.RightButton },
+                {tag: "middle mouse click", signalName: "clicked", button: Qt.MiddleButton },
+                {tag: "mouse wheel", signalName: "wheel", button: Qt.MiddleButton },
+                {tag: "double click (RMB)", signalName: "doubleClicked", button: Qt.RightButton },
+            ]
+        }
+
+        function test_eatWindowDecorationMouseEvents(data) {
+            var dialerAppDelegate = startApplication("dialer-app");
+            verify(dialerAppDelegate);
+            var decoration = findChild(dialerAppDelegate, "appWindowDecoration");
+            verify(decoration);
+
+            mouseEaterSpy.signalName = data.signalName;
+            if (data.signalName === "wheel") {
+                mouseWheel(decoration, decoration.width/2, decoration.height/2, 20, 20);
+            } else if (data.signalName === "clicked") {
+                mouseClick(decoration, decoration.width/2, decoration.height/2, data.button);
+            } else {
+                mouseDoubleClick(decoration, decoration.width/2, decoration.height/2, data.button);
+                tryCompare(dialerAppDelegate, "maximized", false);
+            }
+
+            tryCompare(mouseEaterSpy, "count", 0);
+        }
+
+        function test_canMoveWindowWithLeftMouseButtonOnly_data() {
+            return [
+                {tag: "left mouse button", button: Qt.LeftButton },
+                {tag: "right mouse button", button: Qt.RightButton },
+                {tag: "middle mouse button", button: Qt.MiddleButton }
+            ]
+        }
+
+        function test_canMoveWindowWithLeftMouseButtonOnly(data) {
+            var appDelegate = startApplication("dialer-app");
+            verify(appDelegate);
+
+            var posBefore = Qt.point(appDelegate.x, appDelegate.y);
+
+            mousePress(appDelegate, appDelegate.width / 2, units.gu(1), data.button);
+            mouseMove(appDelegate, appDelegate.width / 2, -units.gu(100), undefined /* delay */, data.button);
+
+            var posAfter = Qt.point(appDelegate.x, appDelegate.y);
+
+            tryCompareFunction(function(){return posBefore == posAfter;}, data.button !== Qt.LeftButton ? true : false);
+        }
     }
 }