Unity 8

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

  1. lvwph_bad_header_position_1240118
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Nick Dedekind
Approved revision: 485
Merged at revision: 527
Proposed branch: lp:~aacid/unity8/lvwph_bad_header_position_1240118
Merge into: lp:unity8
Diff against target: 70 lines (+22/-1)
3 files modified
plugins/ListViewWithPageHeader/listviewwithpageheader.cpp (+4/-1)
plugins/ListViewWithPageHeader/listviewwithpageheader.h (+1/-0)
tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp (+17/-0)
To merge this branch: bzr merge lp:~aacid/unity8/lvwph_bad_header_position_1240118
Reviewer Review Type Date Requested Status
Nick Dedekind (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Michal Hruby (community) Approve
Review via email: mp+193200@code.launchpad.net

Commit message

LVWPH: Fix header going bad when setContentHeight ends up moving the viewport

How to reproduce the bug easily without the patch:
 * In the Dash Home, search for london
 * Scroll to the bottom
 * Start moving to the apps scope very slowly
 * At around 3/4 of the move you'll see the header in the home scope went to a bad position
 * Go back to the Dash Home

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:482
http://jenkins.qa.ubuntu.com/job/unity8-ci/1521/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/187
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/181/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/48
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/45
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/45
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/45/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/45
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/176
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/187
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/187/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/181
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/181/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2804/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2855
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/648
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/647

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1521/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Michal Hruby (mhr3) wrote :

Fixes the incorrect positioning for me, although the code looks a bit fragile :)

review: Approve
lp:~aacid/unity8/lvwph_bad_header_position_1240118 updated
483. By Albert Astals Cid

Merge lp:unity8

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:483
http://jenkins.qa.ubuntu.com/job/unity8-ci/1544/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/331
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/319
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/88
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/68
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/68
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/68/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/68
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/312
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/331
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/331/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/319
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/319/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2988
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/911

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1544/rebuild

review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/lvwph_bad_header_position_1240118 updated
484. By Albert Astals Cid

booo

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:484
http://jenkins.qa.ubuntu.com/job/unity8-ci/1545/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/334
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/322
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/89
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/69
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/69
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/69/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/69
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/315
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/334
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/334/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/322
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/322/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2991
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/914

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1545/rebuild

review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/lvwph_bad_header_position_1240118 updated
485. By Albert Astals Cid

Enable the check later?

I'm pretty convinced it's unstability has to do with the non great qml engine loop in 5.0

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:485
http://jenkins.qa.ubuntu.com/job/unity8-ci/1547/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/344
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/332
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/93
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/71
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/71
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/71/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/71
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/323
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/344
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/344/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/332
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/332/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3001
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/924

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1547/rebuild

review: Approve (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Ok, don't really understand the code, but it fixes the issue.
Test successfully tests for regressions.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/ListViewWithPageHeader/listviewwithpageheader.cpp'
2--- plugins/ListViewWithPageHeader/listviewwithpageheader.cpp 2013-10-03 11:48:24 +0000
3+++ plugins/ListViewWithPageHeader/listviewwithpageheader.cpp 2013-11-04 12:34:19 +0000
4@@ -155,6 +155,7 @@
5 , m_topSectionItem(nullptr)
6 , m_forceNoClip(false)
7 , m_inLayout(false)
8+ , m_inContentHeightKeepHeaderShown(false)
9 {
10 m_clipItem = new QQuickItem(contentItem());
11 // m_clipItem = new QQuickRectangle(contentItem());
12@@ -494,7 +495,7 @@
13 if (!scrolledUp && contentY() == -m_minYExtent) {
14 m_headerItemShownHeight = 0;
15 m_headerItem->setY(contentY());
16- } else if ((scrolledUp && notRebounding && notShownByItsOwn && !maximizeVisibleAreaRunning) || (m_headerItemShownHeight > 0)) {
17+ } else if ((scrolledUp && notRebounding && notShownByItsOwn && !maximizeVisibleAreaRunning) || (m_headerItemShownHeight > 0) || m_inContentHeightKeepHeaderShown) {
18 if (maximizeVisibleAreaRunning && diff > 0) // If we are maximizing and the header was shown, make sure we hide it
19 m_headerItemShownHeight -= diff;
20 else
21@@ -1267,7 +1268,9 @@
22
23 m_contentHeightDirty = false;
24 adjustMinYExtent();
25+ m_inContentHeightKeepHeaderShown = m_headerItem && m_headerItem->y() == contentY();
26 setContentHeight(contentHeight);
27+ m_inContentHeightKeepHeaderShown = false;
28 }
29 }
30
31
32=== modified file 'plugins/ListViewWithPageHeader/listviewwithpageheader.h'
33--- plugins/ListViewWithPageHeader/listviewwithpageheader.h 2013-09-02 13:22:33 +0000
34+++ plugins/ListViewWithPageHeader/listviewwithpageheader.h 2013-11-04 12:34:19 +0000
35@@ -202,6 +202,7 @@
36
37 bool m_forceNoClip;
38 bool m_inLayout;
39+ bool m_inContentHeightKeepHeaderShown;
40
41 // Qt 5.0 doesn't like releasing the items just after itemCreated
42 // so we delay the releasing until the next updatePolish
43
44=== modified file 'tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp'
45--- tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp 2013-10-02 13:20:56 +0000
46+++ tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp 2013-11-04 12:34:19 +0000
47@@ -1824,6 +1824,23 @@
48 lvwph->positionAtBeginning();
49 }
50
51+ void testHeaderPositionBug1240118()
52+ {
53+ scrollToBottom();
54+ lvwph->showHeader();
55+ QTRY_VERIFY(!lvwph->m_contentYAnimation->isRunning());
56+ QTest::qWait(100); // Make sure stuff is stable
57+ QMetaObject::invokeMethod(model, "insertItem", Q_ARG(QVariant, 0), Q_ARG(QVariant, 100));
58+ model->setProperty(3, "size", 10);
59+ model->setProperty(4, "size", 10);
60+ model->setProperty(5, "size", 10);
61+ model->setProperty(6, "size", 10);
62+#if (QT_VERSION > QT_VERSION_CHECK(5, 0, 3))
63+ QTRY_COMPARE(lvwph->m_minYExtent, 210.);
64+#endif
65+ QTRY_COMPARE(lvwph->m_headerItem->y(), -lvwph->m_minYExtent);
66+ }
67+
68 private:
69 QQuickView *view;
70 ListViewWithPageHeader *lvwph;

Subscribers

People subscribed via source and target branches