From cb029a36279a1b7d47ec4c58c7916c9b63615b57 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 25 Sep 2018 18:02:54 +0200 Subject: ScrollView: Defer calls to doLayout() doLayout() updates the dimensions of the scroll view, and therefore can trigger the scroll bars to be shown or hidden. That, in turn, can trigger another call to doLayout(), resulting in binding loops. Avoid those by deferring the call. The scrollview tests are broken without this change because the binding loop prevents the scrollbars from being shown or hidden in some cases. The deferring necessitates a tryVerify(), but fixes the actual tests. Change-Id: I0dac0ba380240e025237f69c4d357d764a535c3d Fixes: QTBUG-50605 Reviewed-by: Mitch Curtis --- src/controls/Private/ScrollViewHelper.qml | 18 ++++++++---------- tests/auto/controls/data/tst_scrollview.qml | 26 +++++++++++++++++--------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/controls/Private/ScrollViewHelper.qml b/src/controls/Private/ScrollViewHelper.qml index 810de91d..53050108 100644 --- a/src/controls/Private/ScrollViewHelper.qml +++ b/src/controls/Private/ScrollViewHelper.qml @@ -68,31 +68,29 @@ Item { anchors.fill: parent - property bool recursionGuard: false - - function doLayout() { - if (!recursionGuard) { - recursionGuard = true + Timer { + id: layoutTimer + interval: 0; + onTriggered: { blockUpdates = true; scrollHelper.contentWidth = flickableItem !== null ? flickableItem.contentWidth : 0 scrollHelper.contentHeight = flickableItem !== null ? flickableItem.contentHeight : 0 scrollHelper.availableWidth = viewport.width scrollHelper.availableHeight = viewport.height blockUpdates = false; - recursionGuard = false } } Connections { target: viewport - onWidthChanged: doLayout() - onHeightChanged: doLayout() + onWidthChanged: layoutTimer.running = true + onHeightChanged: layoutTimer.running = true } Connections { target: flickableItem - onContentWidthChanged: doLayout() - onContentHeightChanged: doLayout() + onContentWidthChanged: layoutTimer.running = true + onContentHeightChanged: layoutTimer.running = true onContentXChanged: { hscrollbar.flash() vscrollbar.flash() diff --git a/tests/auto/controls/data/tst_scrollview.qml b/tests/auto/controls/data/tst_scrollview.qml index 42398115..d3bfac4b 100644 --- a/tests/auto/controls/data/tst_scrollview.qml +++ b/tests/auto/controls/data/tst_scrollview.qml @@ -212,21 +212,27 @@ TestCase { bigItem.height = 100 bigItem.width = 100 - verify(!scrollView.__horizontalScrollBar.visible, "Scrollbar showing when contents already fit") - verify(!scrollView.__verticalScrollBar.visible, "Scrollbar showing when contents already fit") + tryVerify(function() { return !scrollView.__horizontalScrollBar.visible }, 50, + "Scrollbar showing when contents already fit") + tryVerify(function() { return !scrollView.__verticalScrollBar.visible }, 50, + "Scrollbar showing when contents already fit") bigItem.height = 1000 bigItem.width = 1000 - verify(scrollView.__horizontalScrollBar.visible, "Scrollbar not showing when contents are too big") - verify(scrollView.__verticalScrollBar.visible, "Scrollbar not showing when contents are too big") + tryVerify(function() { return scrollView.__horizontalScrollBar.visible }, 50, + "Scrollbar not showing when contents are too big") + tryVerify(function() { return scrollView.__verticalScrollBar.visible }, 50, + "Scrollbar not showing when contents are too big") //always off bigItem.height = 1000 scrollView.verticalScrollBarPolicy = Qt.ScrollBarAlwaysOff - verify(!scrollView.__verticalScrollBar.visible, "Scrollbar showing when disabled") + tryVerify(function() { return !scrollView.__verticalScrollBar.visible }, 50, + "Scrollbar showing when disabled") bigItem.height = 100 - verify(!scrollView.__verticalScrollBar.visible, "Scrollbar showing when disabled") + tryVerify(function() { return !scrollView.__verticalScrollBar.visible }, 50, + "Scrollbar showing when disabled") //always on scrollView.verticalScrollBarPolicy = Qt.ScrollBarAlwaysOn @@ -258,12 +264,14 @@ TestCase { verify(scrollView !== null, "view created is null") verify(scrollView.flickableItem.contentY === 0) + tryVerify(function() { return scrollView.__verticalScrollBar.visible }); + mouseClick(scrollView, scrollView.width -2, scrollView.height/2, Qt.LeftButton) - verify(Math.round(scrollView.flickableItem.contentY) === 100) + tryVerify(function() { return Math.round(scrollView.flickableItem.contentY) === 100 }); - verify(scrollView.flickableItem.contentX === 0) + tryVerify(function() { return scrollView.flickableItem.contentX === 0 }) mouseClick(scrollView, scrollView.width/2, scrollView.height - 2, Qt.LeftButton) - verify(Math.round(scrollView.flickableItem.contentX) === 100) + tryVerify(function() { return Math.round(scrollView.flickableItem.contentX) === 100 }) } function test_viewport() { -- cgit v1.2.1