StackView: fix an issue where the current item was hidden

When a StackView has an item A as the current item, calling
replace(B) and then replace(A) would result in A being hidden
when all transitions were finished.

When an item is finishing its transition, we can check to see
if that item exists in the stack (i.e. was pushed while it was
transitioning), and if so, don't hide it.

The patch is based on the one from Anthony Groyer.

Fixes: QTBUG-57267
Change-Id: I441559c54a35c577261074bc7f0c923aeb3ca330
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

2 files changed, 77 insertions, 4 deletions

diff --git a/src/quicktemplates2/qquickstackview_p.cpp b/src/quicktemplates2/qquickstackview_p.cpp
index 7cb943a3..a280e31d 100644
--- a/src/quicktemplates2/qquickstackview_p.cpp
+++ b/src/quicktemplates2/qquickstackview_p.cpp
@@ -267,7 +267,11 @@ void QQuickStackViewPrivate::viewItemTransitionFinished(QQuickItemViewTransition
         element->setStatus(QQuickStackView::Active);
     } else if (element->status == QQuickStackView::Deactivating) {
         element->setStatus(QQuickStackView::Inactive);
-        element->setVisible(false);
+        QQuickStackElement *existingElement = element->item ? findElement(element->item) : nullptr;
+        // If a different element with the same item is found,
+        // do not call setVisible(false) since it needs to be visible.
+        if (!existingElement || element == existingElement)
+            element->setVisible(false);
         if (element->removal || element->isPendingRemoval())
             removed += element;
     }
@@ -275,11 +279,21 @@ void QQuickStackViewPrivate::viewItemTransitionFinished(QQuickItemViewTransition
     if (transitioner && transitioner->runningJobs.isEmpty()) {
         // ~QQuickStackElement() emits QQuickStackViewAttached::removed(), which may be used
         // to modify the stack. Set the status first and make a copy of the destroyable stack
-        // elements to exclude any modifications that may happen during the loop. (QTBUG-62153)
+        // elements to exclude any modifications that may happen during qDeleteAll(). (QTBUG-62153)
         setBusy(false);
-        QList<QQuickStackElement*> elements = removed;
+        QList<QQuickStackElement*> removedElements = removed;
         removed.clear();
-        qDeleteAll(elements);
+
+        for (QQuickStackElement *removedElement : qAsConst(removedElements)) {
+            // If an element with the same item is found in the active stack list,
+            // forget about the item so that we don't hide it.
+            if (removedElement->item && findElement(removedElement->item)) {
+                QQuickItemPrivate::get(removedElement->item)->removeItemChangeListener(removedElement, QQuickItemPrivate::Destroyed);
+                removedElement->item = nullptr;
+            }
+        }
+
+        qDeleteAll(removedElements);
     }
 
     removing.remove(element);
diff --git a/tests/auto/controls/data/tst_stackview.qml b/tests/auto/controls/data/tst_stackview.qml
index c6515f77..c15ce8ea 100644
--- a/tests/auto/controls/data/tst_stackview.qml
+++ b/tests/auto/controls/data/tst_stackview.qml
@@ -1345,4 +1345,63 @@ TestCase {
         ignoreWarning(new RegExp(".*cannot replace while already in the process of removing elements"))
         control.clear(StackView.Immediate)
     }
+
+    Component {
+        id: rectangleComponent
+        Rectangle {}
+    }
+
+    Component {
+        id: qtbug57267_StackViewComponent
+
+        StackView {
+            id: stackView
+
+            popEnter: Transition {
+                XAnimator { from: (stackView.mirrored ? -1 : 1) * -stackView.width; to: 0; duration: 400; easing.type: Easing.Linear }
+            }
+            popExit: Transition {
+                XAnimator { from: 0; to: (stackView.mirrored ? -1 : 1) * stackView.width; duration: 400; easing.type: Easing.Linear }
+            }
+            pushEnter: Transition {
+                XAnimator { from: (stackView.mirrored ? -1 : 1) * stackView.width; to: 0; duration: 400; easing.type: Easing.Linear }
+            }
+            pushExit: Transition {
+                XAnimator { from: 0; to: (stackView.mirrored ? -1 : 1) * -stackView.width; duration: 400; easing.type: Easing.Linear }
+            }
+            replaceEnter: Transition {
+                XAnimator { from: (stackView.mirrored ? -1 : 1) * stackView.width; to: 0; duration: 400; easing.type: Easing.Linear }
+            }
+            replaceExit: Transition {
+                XAnimator { from: 0; to: (stackView.mirrored ? -1 : 1) * -stackView.width; duration: 400; easing.type: Easing.Linear }
+            }
+        }
+    }
+
+    function test_qtbug57267() {
+        let redRect = createTemporaryObject(rectangleComponent, testCase, { color: "red" })
+        verify(redRect)
+        let blueRect = createTemporaryObject(rectangleComponent, testCase, { color: "blue" })
+        verify(blueRect)
+        let control = createTemporaryObject(qtbug57267_StackViewComponent, testCase,
+            { "anchors.fill": testCase, initialItem: redRect })
+        verify(control)
+
+        control.replace(blueRect)
+        compare(control.currentItem, blueRect)
+        compare(control.depth, 1)
+
+        // Wait until the animation has started and then interrupt it by pushing the redRect.
+        tryCompare(control, "busy", true)
+        control.replace(redRect)
+        // The blue rect shouldn't be visible since we replaced it and therefore interrupted its animation.
+        tryCompare(blueRect, "visible", false)
+        // We did the replace very early on, so the transition for the redRect should still be happening.
+        compare(control.busy, true)
+        compare(redRect.visible, true)
+
+        // After finishing the transition, the red rect should still be visible.
+        tryCompare(control, "busy", false)
+        compare(redRect.visible, true)
+    }
 }