Unity 8

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

  1. journal_misc_fixes
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 641
Merged at revision: 646
Proposed branch: lp:~aacid/unity8/journal_misc_fixes
Merge into: lp:unity8
Diff against target: 164 lines (+71/-11)
5 files modified
plugins/DashViews/abstractjournal.cpp (+1/-1)
plugins/DashViews/horizontaljournal.cpp (+1/-0)
plugins/DashViews/verticaljournal.cpp (+2/-1)
tests/plugins/DashViews/horizontaljournaltest.cpp (+35/-9)
tests/plugins/DashViews/verticaljournaltest.cpp (+32/-0)
To merge this branch: bzr merge lp:~aacid/unity8/journal_misc_fixes
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Michał Karnicki (community) Approve
Review via email: mp+201785@code.launchpad.net

Commit message

Misc journal fixes

Don't init *modelIndex to INT_MAX
  Makes no sense since we're not doing any qMin and the calling function also accepts any index >= 0 as valid
  so in some cases it may end up wanting to create an index that doesn't exist

Don't refill if height() < 0, that gives bad ranges for from/to and the code gets confused

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

FAILED: Continuous integration, rev:640
http://jenkins.qa.ubuntu.com/job/unity8-ci/2070/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2365
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2238
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/922
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/592
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/594
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/594/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/592
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2075
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2367
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2367/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2238
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2238/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4695
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3121

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2070/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Karnicki (karni) wrote :

+1

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

PASSED: Continuous integration, rev:640
http://jenkins.qa.ubuntu.com/job/unity8-ci/2085/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2393
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2264
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/942
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/607
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/609
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/609/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/607
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2099
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2395
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2395/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2264
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2264/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4715
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3144

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2085/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

Can we test this?

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

PASSED: Continuous integration, rev:641
http://jenkins.qa.ubuntu.com/job/unity8-ci/2097/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2426
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2282
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/955
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/619
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/621
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/621/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/619
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2118
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2428
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2428/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2282
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2282/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4728
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3164

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2097/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michał Karnicki (karni) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/949/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3912
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2446
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2296
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/962
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/335
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/335
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/335/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/335
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2136
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2448
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2448/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2296
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2296/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4742
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3180

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/950/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3918
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2461
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2304
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/963
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/336
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/336
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/336/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/336
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2150
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2463
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2463/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2304
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2304/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4750
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3195

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/DashViews/abstractjournal.cpp'
--- plugins/DashViews/abstractjournal.cpp 2014-01-07 14:51:15 +0000
+++ plugins/DashViews/abstractjournal.cpp 2014-01-17 10:09:05 +0000
@@ -194,7 +194,7 @@
194194
195void AbstractJournal::refill()195void AbstractJournal::refill()
196{196{
197 if (!isComponentComplete()) {197 if (!isComponentComplete() || height() < 0) {
198 return;198 return;
199 }199 }
200200
201201
=== modified file 'plugins/DashViews/horizontaljournal.cpp'
--- plugins/DashViews/horizontaljournal.cpp 2014-01-09 17:14:47 +0000
+++ plugins/DashViews/horizontaljournal.cpp 2014-01-17 10:09:05 +0000
@@ -173,6 +173,7 @@
173 m_visibleItems.clear();173 m_visibleItems.clear();
174 m_lastInRowIndexPosition.clear();174 m_lastInRowIndexPosition.clear();
175 m_firstVisibleIndex = -1;175 m_firstVisibleIndex = -1;
176 setImplicitHeightDirty();
176}177}
177178
178void HorizontalJournal::calculateImplicitHeight()179void HorizontalJournal::calculateImplicitHeight()
179180
=== modified file 'plugins/DashViews/verticaljournal.cpp'
--- plugins/DashViews/verticaljournal.cpp 2014-01-03 09:18:24 +0000
+++ plugins/DashViews/verticaljournal.cpp 2014-01-17 10:09:05 +0000
@@ -79,7 +79,7 @@
7979
80void VerticalJournal::findTopModelIndexToAdd(int *modelIndex, qreal *yPos)80void VerticalJournal::findTopModelIndexToAdd(int *modelIndex, qreal *yPos)
81{81{
82 *modelIndex = INT_MAX;82 *modelIndex = 0;
83 *yPos = std::numeric_limits<qreal>::lowest();83 *yPos = std::numeric_limits<qreal>::lowest();
84 int columnToAddTo = -1;84 int columnToAddTo = -1;
8585
@@ -179,6 +179,7 @@
179 column.clear();179 column.clear();
180 }180 }
181 m_indexColumnMap.clear();181 m_indexColumnMap.clear();
182 setImplicitHeightDirty();
182}183}
183184
184void VerticalJournal::calculateImplicitHeight()185void VerticalJournal::calculateImplicitHeight()
185186
=== modified file 'tests/plugins/DashViews/horizontaljournaltest.cpp'
--- tests/plugins/DashViews/horizontaljournaltest.cpp 2014-01-03 08:37:09 +0000
+++ tests/plugins/DashViews/horizontaljournaltest.cpp 2014-01-17 10:09:05 +0000
@@ -126,9 +126,9 @@
126 view->engine()->addImportPath(BUILT_PLUGINS_DIR);126 view->engine()->addImportPath(BUILT_PLUGINS_DIR);
127127
128 model = new WidthModel();128 model = new WidthModel();
129 QStringList heightList;129 QStringList widthList;
130 heightList << "100" << "50" << "125" << "10" << "40" << "70" << "200" << "110" << "160" << "20" << "20" << "65" << "80" << "200" << "300" << "130" << "400" << "300" << "500" << "10";130 widthList << "100" << "50" << "125" << "10" << "40" << "70" << "200" << "110" << "160" << "20" << "20" << "65" << "80" << "200" << "300" << "130" << "400" << "300" << "500" << "10";
131 model->setStringList(heightList);131 model->setStringList(widthList);
132132
133 view->setSource(QUrl::fromLocalFile(DASHVIEWSTEST_FOLDER "/horizontaljournaltest.qml"));133 view->setSource(QUrl::fromLocalFile(DASHVIEWSTEST_FOLDER "/horizontaljournaltest.qml"));
134134
@@ -336,9 +336,9 @@
336336
337 void testModelReset()337 void testModelReset()
338 {338 {
339 QStringList heightList;339 QStringList widthList;
340 heightList << "100" << "50" << "125" << "25" << "50" << "50";340 widthList << "100" << "50" << "125" << "25" << "50" << "50";
341 model->setStringList(heightList);341 model->setStringList(widthList);
342342
343 QTRY_COMPARE(hj->m_firstVisibleIndex, 0);343 QTRY_COMPARE(hj->m_firstVisibleIndex, 0);
344 QTRY_COMPARE(hj->m_visibleItems.count(), 6);344 QTRY_COMPARE(hj->m_visibleItems.count(), 6);
@@ -386,9 +386,9 @@
386 void testModelAppendRemoveLast()386 void testModelAppendRemoveLast()
387 {387 {
388 WidthModel *model2 = new WidthModel();388 WidthModel *model2 = new WidthModel();
389 QStringList heightList;389 QStringList widthList;
390 heightList << "100" << "50" << "125" << "25" << "50" << "50";390 widthList << "100" << "50" << "125" << "25" << "50" << "50";
391 model2->setStringList(heightList);391 model2->setStringList(widthList);
392 hj->setModel(model2);392 hj->setModel(model2);
393 delete model;393 delete model;
394 model = model2;394 model = model2;
@@ -454,6 +454,32 @@
454 QCOMPARE(hj->implicitHeight(), 310.);454 QCOMPARE(hj->implicitHeight(), 310.);
455 }455 }
456456
457 void testNegativeHeight()
458 {
459 QQuickItemPrivate::get(hj)->anchors()->resetFill();
460 hj->setHeight(-8);
461
462 QStringList widthList;
463 widthList << "100" << "50" << "125" << "25" << "50" << "50";
464 model->setStringList(widthList);
465
466 QTRY_COMPARE(hj->m_visibleItems.count(), 0);
467 QTRY_COMPARE(hj->implicitHeight(), 0.);
468 }
469
470 void testNegativeDelegateCreationRange()
471 {
472 hj->setDelegateCreationBegin(0);
473 hj->setDelegateCreationEnd(-100);
474
475 QStringList widthList;
476 widthList << "100" << "50" << "50" << "30";
477 model->setStringList(widthList);
478
479 QTRY_COMPARE(hj->m_visibleItems.count(), 0);
480 QTRY_COMPARE(hj->implicitHeight(), 0.);
481 }
482
457private:483private:
458 QQuickView *view;484 QQuickView *view;
459 HorizontalJournal *hj;485 HorizontalJournal *hj;
460486
=== modified file 'tests/plugins/DashViews/verticaljournaltest.cpp'
--- tests/plugins/DashViews/verticaljournaltest.cpp 2014-01-03 08:37:09 +0000
+++ tests/plugins/DashViews/verticaljournaltest.cpp 2014-01-17 10:09:05 +0000
@@ -459,6 +459,38 @@
459 verifyItem(vj->m_columnVisibleItems[1][2], 7, 160, 120, true);459 verifyItem(vj->m_columnVisibleItems[1][2], 7, 160, 120, true);
460 }460 }
461461
462 void testNegativeHeight()
463 {
464 QQuickItemPrivate::get(vj)->anchors()->resetFill();
465 vj->setHeight(-8);
466
467 QStringList heightList;
468 heightList << "100" << "50" << "50" << "30";
469 model->setStringList(heightList);
470
471 QTRY_COMPARE(vj->m_columnVisibleItems.count(), 3);
472 QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 0);
473 QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 0);
474 QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 0);
475 QTRY_COMPARE(vj->implicitHeight(), 0.);
476 }
477
478 void testNegativeDelegateCreationRange()
479 {
480 vj->setDelegateCreationBegin(0);
481 vj->setDelegateCreationEnd(-100);
482
483 QStringList heightList;
484 heightList << "100" << "50" << "50" << "30";
485 model->setStringList(heightList);
486
487 QTRY_COMPARE(vj->m_columnVisibleItems.count(), 3);
488 QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 0);
489 QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 0);
490 QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 0);
491 QTRY_COMPARE(vj->implicitHeight(), 0.);
492 }
493
462private:494private:
463 QQuickView *view;495 QQuickView *view;
464 VerticalJournal *vj;496 VerticalJournal *vj;

Subscribers

People subscribed via source and target branches