Merge lp:~zsombi/ubuntu-ui-toolkit/focusing-improvements into lp:ubuntu-ui-toolkit/staging

[Zsombor Egri]

Focus should not be taken by a component if disabled. Complete child filtering properly.

[Zsombor Egri (zsombi)]

The child filtering wasn't completed in teh original MR :/ the setFiltersChildMouseEvents() call was there without having the childMouseEventsFilter() being implemented. This caused us to have all sorts of workarounds for which QtQuick provides a solution. Mea culpa.

[Cris Dywan (kalikiana)]

This makes quite a bit of sense. I wonder how neither of us noticed before… looks sensible.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1227
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/949/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4306/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/781/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/781/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/781/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5558/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/949/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-autolanding/472/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4307/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-autolanding/416/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-autolanding/416/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-autolanding/416/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5559/console

review: Needs Fixing (continuous-integration)

[lp:~zsombi/ubuntu-ui-toolkit/focusing-improvements]

1228. By Zsombor Egri on 2014-09-02

remove unused d-pointer causing build warning

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1228
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/954/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4316/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/786/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/786/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/786/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5568/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/954/rebuild

review: Needs Fixing (continuous-integration)

[lp:~zsombi/ubuntu-ui-toolkit/focusing-improvements]

1229. By Zsombor Egri on 2014-09-02

documentation fix

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1229
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/960/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4328
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/792
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/792
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/792/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/792
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4128
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5580
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5580/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12483

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/960/rebuild

review: Approve (continuous-integration)

=== modified file 'modules/Ubuntu/Components/AbstractButton.qml'
--- modules/Ubuntu/Components/AbstractButton.qml	2014-08-21 05:24:14 +0000
+++ modules/Ubuntu/Components/AbstractButton.qml	2014-09-02 12:49:18 +0000
@@ -103,7 +103,6 @@
         hoverEnabled: true
 
         onClicked: {
-            button.requestFocus();
             if (button.__acceptEvents) {
                 pressEffect.start()
                 button.clicked()

=== modified file 'modules/Ubuntu/Components/Pickers/Picker.qml'
--- modules/Ubuntu/Components/Pickers/Picker.qml	2014-08-21 05:24:14 +0000
+++ modules/Ubuntu/Components/Pickers/Picker.qml	2014-09-02 12:49:18 +0000
@@ -294,7 +294,6 @@
             width: parent ? parent.width : 0
             clip: true
             focus: true
-            Mouse.onClicked: picker.requestFocus()
 
             model: picker.model
             delegate: picker.delegate

=== modified file 'modules/Ubuntu/Components/plugin/ucstyleditembase.cpp'
--- modules/Ubuntu/Components/plugin/ucstyleditembase.cpp	2014-08-22 16:09:46 +0000
+++ modules/Ubuntu/Components/plugin/ucstyleditembase.cpp	2014-09-02 12:49:18 +0000
@@ -86,7 +86,7 @@
 }
 
 /*!
- * \qmlmethod void StyledItemBase::requestFocus()
+ * \qmlmethod void StyledItemBase::requestFocus(Qt::FocusReason reason)
  * \since Ubuntu.Components 1.1
  * The function is similar to \c forceActiveFocus except that it sets focus only
  * if all the ancestors have activeFocusOnPressed set. Returns true if the request
@@ -96,7 +96,7 @@
 {
     Q_D(UCStyledItemBase);
     bool focusable = d->isParentFocusable();
-    if (focusable) {
+    if (focusable && isEnabled()) {
         QQuickItem::forceActiveFocus(reason);
     }
     return focusable;
@@ -151,57 +151,6 @@
  * }
  * \endqml
  *
- * StyledItem cannot gain focus automaically if there is a MouseArea or other
- * component grabbing mouse events placed over it. In that case the focus must
- * be grabbed by the overlapping area explicitly either by calling the \l requestFocus
- * function or by forwarding mouse events to the covered StyledItem area.
- * \qml
- * import QtQuick 2.2
- * import Ubuntu.Components 1.1
- *
- * Column {
- *     width: units.gu(50)
- *     height: units.gu(100)
- *
- *     StyledItem {
- *         id: scope1
- *         width: parent.width
- *         height: units.gu(30)
- *         activeFocusOnPress: false
- *         Rectangle {
- *             focus: true
- *             anchors {
- *                 fill: parent
- *                 margins: units.gu(1)
- *             }
- *             color: !activeFocus ? "red" : "green"
- *             MouseArea {
- *                 anchors.fill: parent
- *                 onClicked: scope1.requestFocus()
- *             }
- *         }
- *     }
- *     StyledItem {
- *         id: scope2
- *         width: parent.width
- *         height: units.gu(30)
- *         activeFocusOnPress: false
- *         Rectangle {
- *             focus: true
- *             anchors {
- *                 fill: parent
- *                 margins: units.gu(1)
- *             }
- *             color: !activeFocus ? "red" : "green"
- *             MouseArea {
- *                 anchors.fill: parent
- *                 Mouse.forwardTo: [scope2]
- *             }
- *         }
- *     }
- * }
- * \endqml
- *
  * The default value is \c false.
  */
 bool UCStyledItemBase::activefocusOnPress() const
@@ -223,8 +172,27 @@
 void UCStyledItemBase::mousePressEvent(QMouseEvent *event)
 {
     QQuickItem::mousePressEvent(event);
-    Q_D(UCStyledItemBase);
     requestFocus(Qt::MouseFocusReason);
 }
 
+// filter children events as well, so we can catch mouse presses done over child
+// MouseAreas or other mouse grabbers
+bool UCStyledItemBase::childMouseEventFilter(QQuickItem *child, QEvent *event)
+{
+    // only filter pressed events
+    if (event->type() == QEvent::MouseButtonPress) {
+        QMouseEvent *mouse = static_cast<QMouseEvent*>(event);
+        // the event may occur outside of the parent's boundaries if not clipped
+        // therefore must check containment
+        QPointF point = mapFromItem(child, mouse->localPos());
+        if (contains(point)) {
+            QMouseEvent press(event->type(), point, mouse->windowPos(), mouse->screenPos(),
+                              mouse->button(), mouse->buttons(), mouse->modifiers());
+            mousePressEvent(&press);
+        }
+    }
+    // let the event be passed to children
+    return false;
+}
+
 #include "moc_ucstyleditembase.cpp"

=== modified file 'modules/Ubuntu/Components/plugin/ucstyleditembase.h'
--- modules/Ubuntu/Components/plugin/ucstyleditembase.h	2014-08-20 12:56:51 +0000
+++ modules/Ubuntu/Components/plugin/ucstyleditembase.h	2014-09-02 12:49:18 +0000
@@ -44,6 +44,7 @@
     UCStyledItemBase(UCStyledItemBasePrivate &, QQuickItem *parent);
 
     void mousePressEvent(QMouseEvent *event);
+    bool childMouseEventFilter(QQuickItem *child, QEvent *event);
 
 private:
     Q_DECLARE_PRIVATE(UCStyledItemBase)

=== modified file 'tests/unit_x11/tst_components/tst_focus.qml'
--- tests/unit_x11/tst_components/tst_focus.qml	2014-08-18 16:02:33 +0000
+++ tests/unit_x11/tst_components/tst_focus.qml	2014-09-02 12:49:18 +0000
@@ -82,6 +82,12 @@
         Slider {
             id: slider
         }
+        StyledItem {
+            id: disabledButton
+            enabled: false
+            width: units.gu(20)
+            height: units.gu(6)
+        }
         ComboButton {
             id: comboButton
             Rectangle {
@@ -211,6 +217,7 @@
             compare(dropdownButton.focus, true, "Dropdown button hasn't got focused!");
             compare(comboButton.focus, true, "ComboButton hasn't been focused!");
             comboButton.expanded = false;
+            waitForRendering(comboButton);
         }
 
         function test_popover_refocus_data() {
@@ -236,5 +243,10 @@
             popupCloseSpy.wait();
             verify(popoverTest.focus, "Button focus not restored.");
         }
+
+        function test_disabled_component_does_not_focus() {
+            mousePress(disabledButton, centerOf(disabledButton).x, centerOf(disabledButton).y);
+            compare(disabledButton.focus, false, "Disabled component shoudl not focus");
+        }
     }
 }

=== modified file 'tests/unit_x11/tst_components/tst_styleditem.qml'
--- tests/unit_x11/tst_components/tst_styleditem.qml	2014-08-20 12:40:19 +0000
+++ tests/unit_x11/tst_components/tst_styleditem.qml	2014-09-02 12:49:18 +0000
@@ -90,58 +90,6 @@
                 }
             }
         }
-        StyledItem {
-            id: activeScope2
-            objectName: "activeScope"
-            width: units.gu(50)
-            height: units.gu(20)
-            activeFocusOnPress: true
-
-            Rectangle {
-                width: height
-                height: units.gu(10)
-                color: "green"
-                anchors.centerIn: parent
-
-                StyledItem {
-                    objectName: "in_active_scope"
-                    activeFocusOnPress: true
-                    anchors.fill: parent
-                    MouseArea {
-                        focus: true
-                        objectName: "mouseArea"
-                        anchors.fill: parent
-                        onClicked: parent.requestFocus()
-                    }
-                }
-            }
-        }
-        StyledItem {
-            id: activeScope3
-            objectName: "activeScope"
-            width: units.gu(50)
-            height: units.gu(20)
-            activeFocusOnPress: true
-
-            Rectangle {
-                width: height
-                height: units.gu(10)
-                color: "green"
-                anchors.centerIn: parent
-
-                StyledItem {
-                    objectName: "in_active_scope"
-                    activeFocusOnPress: true
-                    anchors.fill: parent
-                    MouseArea {
-                        focus: true
-                        objectName: "mouseArea"
-                        anchors.fill: parent
-                        Mouse.forwardTo: [parent]
-                    }
-                }
-            }
-        }
     }
 
     UbuntuTestCase {
@@ -152,9 +100,7 @@
              return [
                  {tag: "main scope is passive", mainScope: passiveScope, innerScope: "in_passive_scope", focusing: false},
                  {tag: "main scope is active", mainScope: activeScope, innerScope: "in_active_scope", focusing: true},
-                 {tag: "MouseArea as child, suppressing", mainScope: activeScope1, innerScope: "mouseArea", focusing: false},
-                 {tag: "MouseArea as child, gaining", mainScope: activeScope2, innerScope: "mouseArea", focusing: true},
-                 {tag: "MouseArea as child, forwarding", mainScope: activeScope3, innerScope: "mouseArea", focusing: true},
+                 {tag: "MouseArea as child, gaining", mainScope: activeScope1, innerScope: "mouseArea", focusing: true},
              ];
          }
          function test_scope_focusing(data) {