Merge lp:~gcollura/ubuntu-ui-toolkit/staging-fix-1341814-and-1400297 into lp:ubuntu-ui-toolkit/staging

[Giulio Collura]

This is a bugfix for bug #1341814 and bug #1400297. Essentially we have to force the removal of the previous 'contents' item by removing its parent. This way we ensure that the contents are correctly hidden, focused and removed, without destroying them.

This is a bugfix for bug #1341814 and bug #1400297. Essentially we have to force the removal of the previous 'contents' item by removing its parent. This way we ensure that the contents are correctly hidden, focused and removed, without destroying them.

[Riccardo Padovani (rpadovani)]

Tested on desktop on vivid, it fixes both bugs.

Thanks Giulio!

review: Approve

[Zoltan Balogh (bzoltan)]

Would you please target this MR to the staging branch? (lp:ubuntu-ui-toolkit/staging)

review: Needs Fixing

[Tim Peeters (tpeeters)]

Thanks a lot for the fix! Can you please rebase this with staging so that the changes from trunk don't show up in the diff?

Also,

29 + __oldContents.parent = null

add a semicolon at the end of that line, to use a consistent coding standard.

[Tim Peeters (tpeeters)]

21 + property Item __oldContents: null

Instead of that, I propose to introduce:

QtObject {
  id: internal
  property Item oldContents: null
}

and then use internal.oldContents instead of __oldContents in onContentsChanged. That way you really prevent developers using PageHeadStyle from accessing __oldContents.

[Tim Peeters (tpeeters)]

Great! All is good now, happroving.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[Tim Peeters (tpeeters)]

The bug is still present in RTM. There is a workaround here:

http://paste.ubuntu.com/10609646/

Since we are switching rtm to vivid soon, it is probably not worth the effort to backport the fix.

=== modified file 'modules/Ubuntu/Components/PageHeadConfiguration.qml'
--- modules/Ubuntu/Components/PageHeadConfiguration.qml	2014-10-31 13:36:50 +0000
+++ modules/Ubuntu/Components/PageHeadConfiguration.qml	2015-01-06 12:49:00 +0000
@@ -96,6 +96,21 @@
      */
     property Item contents: null
 
+    QtObject {
+        id: internal
+        property Item oldContents: null
+    }
+
+    onContentsChanged: {
+        if (internal.oldContents) {
+            // FIX: bug #1341814 and #1400297
+            // We have to force the removal of the previous head.contents
+            // in order to show the new contents
+            internal.oldContents.parent = null;
+        }
+        internal.oldContents = contents;
+    }
+
     // FIXME: The example below can be much simplified using PageHeadState
     //  when bug #1345775 has been fixed.
     /*!