Merge lp:~aacid/ubuntu-ui-toolkit/fix_popup_focus_restore into lp:ubuntu-ui-toolkit/staging

[Albert Astals Cid]

Fix saving/restoring of previous item focus for popups

You can't save the focus after the popup/dialog has been created,
at that stage the dialog has focus already,
you need to do it before creating it

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2153
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1337/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7321/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1337/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2153
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1371/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7322/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1371/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2153
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/151/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7323/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/151/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2153
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1587/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7324/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1587/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2153
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/849/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1587/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1371/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1337/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/151/console
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7325/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7324
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7322
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7321
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7323

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/849/rebuild

review: Needs Fixing (continuous-integration)

[Cris Dywan (kalikiana)]

XPASS : components::FocusingTests::test_popover_refocus(Dialog component) 'Button focus not restored.' returned TRUE unexpectedly. ()
   Loc: [/tmp/buildd/ubuntu-ui-toolkit-1.3.2151+17.04.20161223/tests/unit/visual/tst_focus.13.qml(470)]
PASS : components::FocusingTests::test_popover_refocus(Popover component)
XFAIL : components::FocusingTests::test_popover_refocus(Dialog item) Incorrect focus restoration. See bug 1569979 and 1569981.
   Loc: [/tmp/buildd/ubuntu-ui-toolkit-1.3.2151+17.04.20161223/tests/unit/visual/tst_focus.13.qml(470)]
PASS : components::FocusingTests::test_popover_refocus(Dialog item)
XFAIL : components::FocusingTests::test_popover_refocus(Popover item) Incorrect focus restoration. See bug 1569981.
   Loc: [/tmp/buildd/ubuntu-ui-toolkit-1.3.2151+17.04.20161223/tests/unit/visual/tst_focus.13.qml(470)]

review: Needs Fixing

[lp:~aacid/ubuntu-ui-toolkit/fix_popup_focus_restore]

2154. By Albert Astals Cid on 2017-01-10

Merge

2155. By Albert Astals Cid on 2017-01-10

Seems i fixed 1569979

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2154
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1375/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7337/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1375/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2154
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1591/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7338/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1591/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2154
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1341/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7342/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1341/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2154
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/155/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7343/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/155/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

FAILED: Continuous integration, rev:2154
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/852/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1591/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1375/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1341/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/155/console
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7344/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7338
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7337
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7342
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7343

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/852/rebuild

review: Needs Fixing (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:2155
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1342/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7345/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1342/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:2155
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/156/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7346/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/156/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:2155
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1592/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7347/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1592/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:2155
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1376/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7348/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1376/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:2155
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/853/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1592
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1376
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1342
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/156
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7349/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7347
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7348
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7345
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7346

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/853/rebuild

review: Approve (continuous-integration)

[Zsombor Egri (zsombi)]

Looks good to me. Thanks for fixing!

review: Approve

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:2155
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1595/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7359/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1595/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:2155
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1379/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7360/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1379/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:2155
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1345/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7361/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1345/rebuild

review: Approve (continuous-integration)

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:2155
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/159/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7362/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/159/rebuild

review: Approve (continuous-integration)

=== modified file 'src/imports/Components/Popups/1.3/PopupBase.qml'
--- src/imports/Components/Popups/1.3/PopupBase.qml	2016-04-13 19:32:20 +0000
+++ src/imports/Components/Popups/1.3/PopupBase.qml	2017-01-10 12:23:48 +0000
@@ -119,6 +119,14 @@
 
     /*!
       \internal
+      The function saves the active focus for later.
+      */
+    function __setPreviousActiveFocusItem(item) {
+        stateWrapper.prevFocus = item;
+    }
+
+    /*!
+      \internal
       Foreground component excluded from InverseMouseArea
       */
     property Item __foreground
@@ -177,30 +185,14 @@
     /*! \internal */
     onParentChanged: stateWrapper.rootItem = QuickUtils.rootItem(popupBase)
     Component.onCompleted: {
-        stateWrapper.saveActiveFocus();
         stateWrapper.rootItem = QuickUtils.rootItem(popupBase);
     }
 
     Item {
         id: stateWrapper
         property Item rootItem: QuickUtils.rootItem(popupBase)
-
-        property bool windowIsValid: typeof window != "undefined"
         property Item prevFocus
 
-        function saveActiveFocus() {
-            // 'window' context property is exposed to QML after component completion
-            // before rendering is complete, therefore a simple 'if (window)' check is
-            // not enough.
-            if (windowIsValid) {
-                prevFocus = window.activeFocusItem;
-                windowIsValidChanged.disconnect(saveActiveFocus);
-            } else {
-                // connect the function so we can save the original focus item
-                windowIsValidChanged.connect(saveActiveFocus);
-            }
-        }
-
         function restoreActiveFocus() {
             if (prevFocus) {
                 if (prevFocus.hasOwnProperty("requestFocus")) {

=== modified file 'src/imports/Components/Popups/1.3/popupUtils.js'
--- src/imports/Components/Popups/1.3/popupUtils.js	2016-09-15 15:53:27 +0000
+++ src/imports/Components/Popups/1.3/popupUtils.js	2017-01-10 12:23:48 +0000
@@ -66,6 +66,8 @@
     }
 
     var popupObject;
+    // If there's an active item, save it so we can restore it later
+    var prevFocusItem = (typeof window !== "undefined") ? window.activeFocusItem : null;
     if (params !== undefined) {
         popupObject = popupComponent.createObject(rootObject, params);
     } else {
@@ -75,8 +77,11 @@
         print(popupComponent.errorString().slice(0, -1));
         print("PopupUtils.open(): Failed to create the popup object.");
         return;
-    } else if (popupObject.hasOwnProperty("caller") && caller)
+    } else if (popupObject.hasOwnProperty("caller") && caller) {
         popupObject.caller = caller;
+    } else if (popupObject.hasOwnProperty("__setPreviousActiveFocusItem")) {
+        popupObject.__setPreviousActiveFocusItem(prevFocusItem);
+    }
 
     // if caller is specified, connect its cleanup to the popup's close
     // so popups will be removed together with the caller.

=== modified file 'tests/unit/visual/tst_focus.13.qml'
--- tests/unit/visual/tst_focus.13.qml	2016-09-19 16:40:49 +0000
+++ tests/unit/visual/tst_focus.13.qml	2017-01-10 12:23:48 +0000
@@ -414,13 +414,13 @@
                             component: dialogComponent,
                             item: null,
                             foreground_name: "dialogForeground",
-                            bug: "1569979"
+                            bug: "" // this is not buggy
                         },
                         {   tag: "Popover component",
                             component: popoverComponent,
                             item: null,
                             foreground_name: "popover_foreground",
-                            bug: "" // this is the only case without a bug
+                            bug: "" // this is not buggy
                         },
                         {   tag: "Dialog item",
                             component: null,

=== modified file 'tests/unit/visual/tst_popups_dialog.13.qml'
--- tests/unit/visual/tst_popups_dialog.13.qml	2016-06-15 13:46:51 +0000
+++ tests/unit/visual/tst_popups_dialog.13.qml	2017-01-10 12:23:48 +0000
@@ -47,6 +47,21 @@
             keyClick(Qt.Key_Escape);
             tryCompare(test, "dialogDestroyed", true, 500, "Dialog not destroyed");
         }
+
+        function test_focus_restore_ondismiss_dialog() {
+            pressMe.forceActiveFocus();
+
+            tryCompare(window, "activeFocusItem", pressMe);
+
+            var dlg = PopupUtils.open(dialog);
+            waitForRendering(dlg);
+
+            tryCompare(window, "activeFocusItem", dlg.button);
+
+            keyClick(Qt.Key_Escape);
+
+            tryCompare(window, "activeFocusItem", pressMe);
+        }
     }
 
     Component {
@@ -54,10 +69,13 @@
         Dialog {
             id: ahojDialog
             title: "Ahoj"
+            property alias button: closeButton
 
             Button {
+                id: closeButton
                 text: "Close"
                 onClicked: PopupUtils.close(ahojDialog)
+                focus: true
             }
         }
     }