ToolTip: only begin timeout when opened

The old sequence of events was:

1. setVisible(true) (enter transition begins)
2. startTimeout()
3. opened() (enter transition ends)
4. time out elapses (exit transition begins)
5. closed() (exit transition ends)

In most cases, a timeout would be longer than an enter transition,
so the only (barely) noticeable issue would be a slightly quicker
timeout than what was set. However, if the timeout was shorter than the
enter transition, then the sequence would be:

1. setVisible(true) (enter transition begins)
2. startTimeout()
3. time out elapses (exit transition begins)
4. closed() (exit transition ends)

This can result in the enter transition being interrupted and
the tooltip never being properly visible.

Avoid this by only starting the timeout when opened() has been
emitted. This also brings the behavior in line with what would
be expected when reading the documentation.

[ChangeLog][Important Behavior Changes] ToolTip's timeout now begins
only after opened() has been emitted. This bug fix results in tooltips
with enter transitions being visible for the entire duration of the
timeout property. This means that they are visible slightly longer than
they were before, so it may be worthwhile to visually check tooltips in
your application and adjust timeouts if necessary.

Fixes: QTBUG-82257
Change-Id: Iebad64a8cca1f328f9ce85aa78ea0f12918dadf1
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

4 files changed, 30 insertions, 11 deletions

diff --git a/src/quicktemplates2/qquickpopup.cpp b/src/quicktemplates2/qquickpopup.cpp
index d5a0ccf4..6eb993fa 100644
--- a/src/quicktemplates2/qquickpopup.cpp
+++ b/src/quicktemplates2/qquickpopup.cpp
@@ -473,7 +473,7 @@ void QQuickPopupPrivate::finalizeEnterTransition()
     transitionState = NoTransition;
     getPositioner()->reposition();
     emit q->openedChanged();
-    emit q->opened();
+    opened();
 }
 
 void QQuickPopupPrivate::finalizeExitTransition()
@@ -514,6 +514,12 @@ void QQuickPopupPrivate::finalizeExitTransition()
     emit q->closed();
 }
 
+void QQuickPopupPrivate::opened()
+{
+    Q_Q(QQuickPopup);
+    emit q->opened();
+}
+
 QMarginsF QQuickPopupPrivate::getMargins() const
 {
     Q_Q(const QQuickPopup);
diff --git a/src/quicktemplates2/qquickpopup_p_p.h b/src/quicktemplates2/qquickpopup_p_p.h
index 8cac4f08..ae9d36a6 100644
--- a/src/quicktemplates2/qquickpopup_p_p.h
+++ b/src/quicktemplates2/qquickpopup_p_p.h
@@ -136,6 +136,8 @@ public:
     virtual void finalizeEnterTransition();
     virtual void finalizeExitTransition();
 
+    virtual void opened();
+
     QMarginsF getMargins() const;
 
     void setTopMargin(qreal value, bool reset = false);
diff --git a/src/quicktemplates2/qquicktooltip.cpp b/src/quicktemplates2/qquicktooltip.cpp
index 36b6ef5a..0fc3dbf4 100644
--- a/src/quicktemplates2/qquicktooltip.cpp
+++ b/src/quicktemplates2/qquicktooltip.cpp
@@ -130,6 +130,8 @@ public:
     void startTimeout();
     void stopTimeout();
 
+    void opened() override;
+
     QPalette defaultPalette() const override { return QQuickTheme::palette(QQuickTheme::ToolTip); }
 
     int delay = 0;
@@ -163,6 +165,12 @@ void QQuickToolTipPrivate::stopTimeout()
     timeoutTimer.stop();
 }
 
+void QQuickToolTipPrivate::opened()
+{
+    QQuickPopupPrivate::opened();
+    startTimeout();
+}
+
 QQuickToolTip::QQuickToolTip(QQuickItem *parent)
     : QQuickPopup(*(new QQuickToolTipPrivate), parent)
 {
@@ -244,7 +252,7 @@ void QQuickToolTip::setTimeout(int timeout)
 
     if (timeout <= 0)
         d->stopTimeout();
-    else if (isVisible())
+    else if (isOpened())
         d->startTimeout();
 
     emit timeoutChanged();
@@ -260,12 +268,6 @@ void QQuickToolTip::setVisible(bool visible)
                 d->startDelay();
                 return;
             }
-        } else {
-            // We are being made visible, even though we already were.
-            // We've probably been re-opened before our exit transition could finish.
-            // In that case, we need to manually start the timeout, as that is usually
-            // done in itemChange(), which won't be called in this situation.
-            d->startTimeout();
         }
     } else {
         d->stopDelay();
@@ -318,9 +320,7 @@ void QQuickToolTip::itemChange(QQuickItem::ItemChange change, const QQuickItem::
     Q_D(QQuickToolTip);
     QQuickPopup::itemChange(change, data);
     if (change == QQuickItem::ItemVisibleHasChanged) {
-        if (data.boolValue)
-            d->startTimeout();
-        else
+        if (!data.boolValue)
             d->stopTimeout();
 
         QQuickToolTipAttached *attached = qobject_cast<QQuickToolTipAttached *>(qmlAttachedPropertiesObject<QQuickToolTip>(d->parentItem, false));
diff --git a/tests/auto/controls/data/tst_tooltip.qml b/tests/auto/controls/data/tst_tooltip.qml
index 6ca17263..27ba6264 100644
--- a/tests/auto/controls/data/tst_tooltip.qml
+++ b/tests/auto/controls/data/tst_tooltip.qml
@@ -445,4 +445,15 @@ TestCase {
         compare(item.ToolTip.toolTip.contentItem.wrapMode, Text.Wrap)
         verify(item.ToolTip.toolTip.contentItem.width < item.ToolTip.toolTip.contentItem.implicitWidth)
     }
+
+    function test_timeoutAfterOpened() {
+        let control = createTemporaryObject(toolTipWithExitTransition, testCase, { timeout: 1, exit: null })
+        verify(control)
+
+        let openedSpy = createTemporaryObject(signalSpy, testCase, { target: control, signalName: "opened" })
+        verify(openedSpy.valid)
+
+        control.show("Test")
+        tryCompare(openedSpy, "count", 1)
+    }
 }