Merge lp:~aacid/unity8/stray_menus into lp:unity8

[Albert Astals Cid]

Make sure we destroy the popups when the item goes away

Another way of doing this would be changing the parent in the createObject calls
but if we change that we have to change the x,y logic and since that's quite
delicate i decided to go the easy route to not cause regressions

 * Are there any related MPs required for this MP to build/function as expected?
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

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

PASSED: Continuous integration, rev:2868
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3362/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4431
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2656
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2656
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4459
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4292
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4292/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4292
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4292/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4292
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4292/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4292
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4292/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4292
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4292/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4292
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4292/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

Fancy adding a test? :)

review: Needs Information

[lp:~aacid/unity8/stray_menus]

2869. By Albert Astals Cid on 2017-03-14

Add test

[Albert Astals Cid (aacid)]

> Fancy adding a test? :)

Added

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

PASSED: Continuous integration, rev:2869
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3372/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4445
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2661
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2661
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4473
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4303
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4303/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4303
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4303/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4303
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4303/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4303
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4303/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4303
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4303/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4303
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4303/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

Just a note, this is conflicting with other menu branches in silo 2555, might not rebasing

[lp:~aacid/unity8/stray_menus]

2870. By Albert Astals Cid on 2017-03-24

Merge

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

PASSED: Continuous integration, rev:2870
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3519/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4653
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2813
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2813
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4681
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4506
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4506/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4506
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4506/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4506
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4506/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4506
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4506/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4506
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4506/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4506
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4506/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

* 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.

Yes

review: Approve

=== modified file 'qml/ApplicationMenus/MenuBar.qml'
--- qml/ApplicationMenus/MenuBar.qml	2017-03-21 10:56:10 +0000
+++ qml/ApplicationMenus/MenuBar.qml	2017-03-24 08:30:38 +0000
@@ -224,6 +224,13 @@
                         }
                     }
                 }
+
+                Component.onDestruction: {
+                    if (__popup) {
+                        __popup.destroy();
+                        __popup = null;
+                    }
+                }
             } // Item ( delegate )
         } // Repeater
     } // Row

=== modified file 'qml/ApplicationMenus/MenuPopup.qml'
--- qml/ApplicationMenus/MenuPopup.qml	2017-03-21 10:56:10 +0000
+++ qml/ApplicationMenus/MenuPopup.qml	2017-03-24 08:30:38 +0000
@@ -297,6 +297,8 @@
 
                         width: MathUtils.clamp(implicitWidth, d.__minimumWidth, d.__maximumWidth)
 
+                        property Item popup: null
+
                         action.onTriggered: {
                             submenuHoverTimer.stop();
 
@@ -351,6 +353,13 @@
                                 }
                             }
                         }
+
+                        Component.onDestruction: {
+                            if (popup) {
+                                popup.destroy();
+                                popup = null;
+                            }
+                        }
                     }
                 }
 
@@ -373,7 +382,7 @@
                             id: loader
                             objectName: root.objectName + "-item" + __ownIndex
 
-                            property Item popup: null
+                            readonly property var popup: item ? item.popup : null
                             property var __menuData: model
                             property int __ownIndex: index
                             property bool __isSeparator: model.isSeparator

=== modified file 'tests/qmltests/ApplicationMenus/tst_MenuBar.qml'
--- tests/qmltests/ApplicationMenus/tst_MenuBar.qml	2017-03-21 10:56:10 +0000
+++ tests/qmltests/ApplicationMenus/tst_MenuBar.qml	2017-03-24 08:30:38 +0000
@@ -339,6 +339,31 @@
             }, true);
         }
 
+        function test_stray_submenus() {
+            menuBackend.modelData = appMenuData.generateTestData(3,4,1,0,"menu");
+            var priv = findInvisibleChild(menuBar, "d");
+
+            var menuItem = findChild(menuBar, "menuBar-item0");
+            menuItem.show();
+            compare(priv.currentItem, menuItem, "CurrentItem should be set to item 0");
+            compare(priv.currentItem.popupVisible, true, "Popup should be visible");
+
+            var actionItem = findChild(menuBar, "menuBar-item0-menu-item0-actionItem");
+            mouseClick(actionItem);
+
+            actionItem = findChild(menuBar, "menuBar-item0-menu-item0-menu-item0-actionItem");
+            mouseClick(actionItem);
+
+            actionItem = findChild(menuBar, "menuBar-item0-menu-item0-menu-item0-menu-item0-actionItem");
+
+            // There's one popup
+            tryCompareFunction(function() { return findChildsByType(root, "MenuPopup").length; }, 3);
+
+            menuBackend.modelData = null;
+
+            tryCompareFunction(function() { return findChildsByType(root, "MenuPopup").length; }, 0);
+        }
+
         function test_firstDisabled() {
             var data = appMenuData.generateTestData(10,5,2,3);
             data[0].submenu[1].submenu[0].rowData.sensitive = false;