Hide old scroll bars

Reuse the hideOldItem added in 80f1186338
to unparent and hide them.

Fixes: QTBUG-89126
Pick-to: 6.1 5.15
Change-Id: I641e46571b8ac42e0e5080b6737f305ff59afd51
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

src/quicktemplates2/qquickscrollbar.cpp

@@ -972,6 +972,16 @@ void QQuickScrollBarAttachedPrivate::cleanupHorizontal()
 {
     Q_ASSERT(flickable && horizontal);
 
+    QQuickControlPrivate::hideOldItem(horizontal);
+    // ScrollBar.qml has a binding to visible and ScrollView.qml has a binding to parent.
+    // If we just set visible to false and parent to null, these bindings will overwrite
+    // them upon component completion as part of the binding evaluation.
+    // That's why we remove the binding completely.
+    const QQmlProperty visibleProperty(horizontal, QStringLiteral("visible"));
+    const QQmlProperty parentProperty(horizontal, QStringLiteral("parent"));
+    QQmlPropertyPrivate::removeBinding(visibleProperty);
+    QQmlPropertyPrivate::removeBinding(parentProperty);
+
     disconnect(flickable, &QQuickFlickable::movingHorizontallyChanged, this, &QQuickScrollBarAttachedPrivate::activateHorizontal);
 
     // TODO: export QQuickFlickableVisibleArea
@@ -984,6 +994,12 @@ void QQuickScrollBarAttachedPrivate::cleanupVertical()
 {
     Q_ASSERT(flickable && vertical);
 
+    QQuickControlPrivate::hideOldItem(vertical);
+    const QQmlProperty visibleProperty(vertical, QStringLiteral("visible"));
+    const QQmlProperty parentProperty(vertical, QStringLiteral("parent"));
+    QQmlPropertyPrivate::removeBinding(visibleProperty);
+    QQmlPropertyPrivate::removeBinding(parentProperty);
+
     disconnect(flickable, &QQuickFlickable::movingVerticallyChanged, this, &QQuickScrollBarAttachedPrivate::activateVertical);
 
     // TODO: export QQuickFlickableVisibleArea

tests/auto/controls/data/tst_scrollbar.qml

@@ -196,6 +196,36 @@ TestCase {
         compare(horizontal.width, oldWidth)
     }
 
+    function test_attachTwice() {
+        let container = createTemporaryObject(flickable, testCase)
+        verify(container)
+        waitForRendering(container)
+
+        container.ScrollBar.vertical = scrollBar.createObject(container, { objectName: "oldVerticalScrollBar" })
+        verify(container.ScrollBar.vertical)
+        let oldVerticalScrollBar = findChild(container, "oldVerticalScrollBar")
+        verify(oldVerticalScrollBar)
+        verify(oldVerticalScrollBar.visible)
+
+        container.ScrollBar.horizontal = scrollBar.createObject(container, { objectName: "oldHorizontalScrollBar" })
+        verify(container.ScrollBar.horizontal)
+        let oldHorizontalScrollBar = findChild(container, "oldHorizontalScrollBar")
+        verify(oldHorizontalScrollBar)
+        verify(oldHorizontalScrollBar.visible)
+
+        container.ScrollBar.vertical = scrollBar.createObject(container, { objectName: "newVerticalScrollBar" })
+        let newVerticalScrollBar = findChild(container, "newVerticalScrollBar")
+        verify(newVerticalScrollBar)
+        verify(newVerticalScrollBar.visible)
+        verify(!oldVerticalScrollBar.visible)
+
+        container.ScrollBar.horizontal = scrollBar.createObject(container, { objectName: "newHorizontalScrollBar" })
+        let newHorizontalScrollBar = findChild(container, "newHorizontalScrollBar")
+        verify(newHorizontalScrollBar)
+        verify(newHorizontalScrollBar.visible)
+        verify(!oldHorizontalScrollBar.visible)
+    }
+
     function test_mouse_data() {
         return [
             { tag: "horizontal", properties: { visible: true, orientation: Qt.Horizontal, width: testCase.width } },

tests/auto/controls/data/tst_scrollview.qml

@@ -70,6 +70,11 @@ TestCase {
         ScrollView { }
     }
 
+    Component {
+        id: scrollBarComponent
+        ScrollBar {}
+    }
+
     Component {
         id: scrollableLabel
         ScrollView {
@@ -526,4 +531,34 @@ TestCase {
         verify(verticalScrollBar)
         mouseDrag(horizontalScrollBar, horizontalScrollBar.width / 2, horizontalScrollBar.height / 2, 50, 0)
     }
+
+    function test_customScrollBars() {
+        let control = createTemporaryObject(scrollView, testCase)
+        verify(control)
+        control.ScrollBar.vertical.objectName = "oldVerticalScrollBar"
+        control.ScrollBar.horizontal.objectName = "oldHorizontalScrollBar"
+
+        let oldVerticalScrollBar = control.ScrollBar.vertical
+        verify(oldVerticalScrollBar)
+        compare(oldVerticalScrollBar.objectName, "oldVerticalScrollBar")
+
+        let oldHorizontalScrollBar = control.ScrollBar.horizontal
+        verify(oldHorizontalScrollBar)
+        compare(oldHorizontalScrollBar.objectName, "oldHorizontalScrollBar")
+
+        // Create the new scroll bars imperatively so that we can easily access the old ones.
+        control.ScrollBar.vertical = scrollBarComponent.createObject(control, { objectName: "newVerticalScrollBar" })
+        verify(control.ScrollBar.vertical)
+        let newVerticalScrollBar = findChild(control, "newVerticalScrollBar")
+        verify(newVerticalScrollBar)
+        verify(newVerticalScrollBar.visible)
+        verify(!oldVerticalScrollBar.visible)
+
+        control.ScrollBar.horizontal = scrollBarComponent.createObject(control, { objectName: "newHorizontalScrollBar" })
+        verify(control.ScrollBar.horizontal)
+        let newHorizontalScrollBar = findChild(control, "newHorizontalScrollBar")
+        verify(newHorizontalScrollBar)
+        verify(newHorizontalScrollBar.visible)
+        verify(!oldHorizontalScrollBar.visible)
+    }
 }