Fix QQuickToolTip's delay and visibility

The imperative open() and close() methods used to be the only ways to
open and close popups, but the visible-property was later made writable
to allow declarative visibility bindings. Since then, it is no longer
sufficient for QQuickToolTip to overshadow open() and close() in QML,
because setting the visible-property would bypass these overshadowed
method.

There was a bit of duplicate code between setVisible(), open(), and
close(). This change moves the logic to one place by changing open()
and close() to call setVisible(). Furthermore, setVisible() has been
made virtual to make it possible for QQuickToolTip to apply its delay
properly. QQuickToolTip needs to control the delay and timeout timers
before the effective visibility is applied on the popup.

Task-number: QTBUG-55572
Change-Id: I5a109157f9ec5d0db145e710426665a9a8d7e870
Reviewed-by: J-P Nurmi <jpnurmi@qt.io>

5 files changed, 43 insertions, 41 deletions

diff --git a/src/quicktemplates2/qquickpopup.cpp b/src/quicktemplates2/qquickpopup.cpp
index e0a1f359..6a1e1c4f 100644
--- a/src/quicktemplates2/qquickpopup.cpp
+++ b/src/quicktemplates2/qquickpopup.cpp
@@ -833,12 +833,7 @@ QQuickPopup::~QQuickPopup()
 */
 void QQuickPopup::open()
 {
-    Q_D(QQuickPopup);
-    if (d->visible && d->transitionState != QQuickPopupPrivate::ExitTransition)
-        return;
-
-    if (d->complete)
-        d->transitionManager.transitionEnter();
+    setVisible(true);
 }
 
 /*!
@@ -850,12 +845,7 @@ void QQuickPopup::open()
 */
 void QQuickPopup::close()
 {
-    Q_D(QQuickPopup);
-    if (!d->visible)
-        return;
-
-    if (d->complete)
-        d->transitionManager.transitionExit();
+    setVisible(false);
 }
 
 /*!
@@ -1775,12 +1765,13 @@ void QQuickPopup::setVisible(bool visible)
     if (d->visible == visible && d->transitionState != QQuickPopupPrivate::ExitTransition)
         return;
 
-    d->visible = visible;
     if (d->complete) {
         if (visible)
             d->transitionManager.transitionEnter();
         else
             d->transitionManager.transitionExit();
+    } else {
+        d->visible = visible;
     }
 }
 
diff --git a/src/quicktemplates2/qquickpopup_p.h b/src/quicktemplates2/qquickpopup_p.h
index 9ef5c64b..a024c68f 100644
--- a/src/quicktemplates2/qquickpopup_p.h
+++ b/src/quicktemplates2/qquickpopup_p.h
@@ -233,7 +233,7 @@ public:
     void resetDim();
 
     bool isVisible() const;
-    void setVisible(bool visible);
+    virtual void setVisible(bool visible);
 
     qreal opacity() const;
     void setOpacity(qreal opacity);
diff --git a/src/quicktemplates2/qquicktooltip.cpp b/src/quicktemplates2/qquicktooltip.cpp
index b793ada5..a1601ad6 100644
--- a/src/quicktemplates2/qquicktooltip.cpp
+++ b/src/quicktemplates2/qquicktooltip.cpp
@@ -244,6 +244,20 @@ void QQuickToolTip::setTimeout(int timeout)
     emit timeoutChanged();
 }
 
+void QQuickToolTip::setVisible(bool visible)
+{
+    Q_D(QQuickToolTip);
+    if (visible) {
+        if (!d->visible && d->delay > 0) {
+            d->startDelay();
+            return;
+        }
+    } else {
+        d->stopDelay();
+    }
+    QQuickPopup::setVisible(visible);
+}
+
 QQuickToolTipAttached *QQuickToolTip::qmlAttachedProperties(QObject *object)
 {
     QQuickItem *item = qobject_cast<QQuickItem *>(object);
@@ -253,22 +267,6 @@ QQuickToolTipAttached *QQuickToolTip::qmlAttachedProperties(QObject *object)
     return new QQuickToolTipAttached(object);
 }
 
-void QQuickToolTip::open()
-{
-    Q_D(QQuickToolTip);
-    if (!d->visible && d->delay > 0)
-        d->startDelay();
-    else
-        QQuickPopup::open();
-}
-
-void QQuickToolTip::close()
-{
-    Q_D(QQuickToolTip);
-    d->stopDelay();
-    QQuickPopup::close();
-}
-
 QFont QQuickToolTip::defaultFont() const
 {
     return QQuickControlPrivate::themeFont(QPlatformTheme::TipLabelFont);
@@ -295,10 +293,10 @@ void QQuickToolTip::timerEvent(QTimerEvent *event)
     Q_D(QQuickToolTip);
     if (event->timerId() == d->timeoutTimer.timerId()) {
         d->stopTimeout();
-        close();
+        QQuickPopup::setVisible(false);
     } else if (event->timerId() == d->delayTimer.timerId()) {
         d->stopDelay();
-        QQuickPopup::open();
+        QQuickPopup::setVisible(true);
     }
 }
 
diff --git a/src/quicktemplates2/qquicktooltip_p.h b/src/quicktemplates2/qquicktooltip_p.h
index e39862bf..730e38d9 100644
--- a/src/quicktemplates2/qquicktooltip_p.h
+++ b/src/quicktemplates2/qquicktooltip_p.h
@@ -75,11 +75,9 @@ public:
     int timeout() const;
     void setTimeout(int timeout);
 
-    static QQuickToolTipAttached *qmlAttachedProperties(QObject *object);
+    void setVisible(bool visible) override;
 
-public Q_SLOTS:
-    void open();
-    void close();
+    static QQuickToolTipAttached *qmlAttachedProperties(QObject *object);
 
 Q_SIGNALS:
     void textChanged();
diff --git a/tests/auto/controls/data/tst_tooltip.qml b/tests/auto/controls/data/tst_tooltip.qml
index 82ad01f1..c78a7770 100644
--- a/tests/auto/controls/data/tst_tooltip.qml
+++ b/tests/auto/controls/data/tst_tooltip.qml
@@ -156,8 +156,10 @@ TestCase {
 
     function test_delay_data() {
         return [
-            {tag: "0", delay: 0},
-            {tag: "100", delay: 100},
+            {tag: "imperative:0", delay: 0, imperative: true},
+            {tag: "imperative:100", delay: 100, imperative: true},
+            {tag: "declarative:0", delay: 0, imperative: false},
+            {tag: "declarative:100", delay: 100, imperative: false}
         ]
     }
 
@@ -165,18 +167,31 @@ TestCase {
         var control = toolTip.createObject(testCase, {delay: data.delay})
 
         compare(control.visible, false)
-        control.open()
+        if (data.imperative)
+            control.open()
+        else
+            control.visible = true
         compare(control.visible, data.delay <= 0)
         tryCompare(control, "visible", true)
 
         control.destroy()
     }
 
-    function test_timeout() {
+    function test_timeout_data() {
+        return [
+            {tag: "imperative", imperative: true},
+            {tag: "declarative", imperative: false}
+        ]
+    }
+
+    function test_timeout(data) {
         var control = toolTip.createObject(testCase, {timeout: 100})
 
         compare(control.visible, false)
-        control.open()
+        if (data.imperative)
+            control.open()
+        else
+            control.visible = true
         compare(control.visible, true)
         tryCompare(control, "visible", false)