Fix calculation of text margin if line edit contains side widgets

The previous implementation leads to infinite chain of showing/hidden
line edit under circumstances described in QTBUG-54676. We basically got
the situation when size hint were calculated differently depending on
the line edit visibility state. In this case toolbar layout have to
show/hide extension button and line edit a lot of times and can never
leave this "loop" (please note, that the chain is much more complicated
in reality):
Resize toolbar -> Set layout geometry -> Size is OK to display line edit
-> Set layout geometry -> Hide extension button -> Set layout geometry
(wrong size is calculated here, so "run out of space") -> Hide line edit
-> Set layout geometry -> Show extension button -> Set layout geometry -
> Size is OK to display line edit ... And we're in the "loop"

Clear button is hidden if there is no text in a line edit.
In the previous implementation, the button was always visible, only
opacity was changing in order to "hide" the button. It resulted to
incorrect size hints (regular and minimum).
In the current implementation the button is really hidden/shown, and
size hints calculated correctly.
Also updated unit test for line edit.

Remove code duplication in functions for calculation text margin

Fixes: QTBUG-54676
Change-Id: I4549c9ea98e10b750ba855a07037f6392276358b
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

src/widgets/widgets/qlineedit.cpp

@@ -488,7 +488,10 @@ void QLineEdit::setClearButtonEnabled(bool enable)
         QAction *clearAction = new QAction(d->clearButtonIcon(), QString(), this);
         clearAction->setEnabled(!isReadOnly());
         clearAction->setObjectName(QLatin1String(clearButtonActionNameC));
-        d->addAction(clearAction, 0, QLineEdit::TrailingPosition, QLineEditPrivate::SideWidgetClearButton | QLineEditPrivate::SideWidgetFadeInWithText);
+
+        int flags = QLineEditPrivate::SideWidgetClearButton | QLineEditPrivate::SideWidgetFadeInWithText;
+        auto widgetAction = d->addAction(clearAction, nullptr, QLineEdit::TrailingPosition, flags);
+        widgetAction->setVisible(!text().isEmpty());
     } else {
         QAction *clearAction = findChild<QAction *>(QLatin1String(clearButtonActionNameC));
         Q_ASSERT(clearAction);

src/widgets/widgets/qlineedit_p.cpp

@@ -392,9 +392,47 @@ void QLineEditIconButton::setOpacity(qreal value)
 }
 
 #if QT_CONFIG(animation)
+bool QLineEditIconButton::shouldHideWithText() const
+{
+    return m_hideWithText;
+}
+
+void QLineEditIconButton::setHideWithText(bool hide)
+{
+    m_hideWithText = hide;
+}
+
+void QLineEditIconButton::onAnimationFinished()
+{
+    if (shouldHideWithText() && isVisible() && !m_wasHidden) {
+        hide();
+
+        // Invalidate previous geometry to take into account new size of side widgets
+        if (auto le = lineEditPrivate())
+            le->updateGeometry_helper(true);
+    }
+}
+
+void QLineEditIconButton::animateShow(bool visible)
+{
+    m_wasHidden = visible;
+
+    if (shouldHideWithText() && !isVisible()) {
+        show();
+
+        // Invalidate previous geometry to take into account new size of side widgets
+        if (auto le = lineEditPrivate())
+            le->updateGeometry_helper(true);
+    }
+
+    startOpacityAnimation(visible ? 1.0 : 0.0);
+}
+
 void QLineEditIconButton::startOpacityAnimation(qreal endValue)
 {
     QPropertyAnimation *animation = new QPropertyAnimation(this, QByteArrayLiteral("opacity"));
+    connect(animation, &QPropertyAnimation::finished, this, &QLineEditIconButton::onAnimationFinished);
+
     animation->setDuration(160);
     animation->setEndValue(endValue);
     animation->start(QAbstractAnimation::DeleteWhenStopped);
@@ -409,6 +447,16 @@ void QLineEditIconButton::updateCursor()
 }
 #endif // QT_CONFIG(toolbutton)
 
+#if QT_CONFIG(animation) && QT_CONFIG(toolbutton)
+static void displayWidgets(const QLineEditPrivate::SideWidgetEntryList &widgets, bool display)
+{
+    for (const auto &e : widgets) {
+        if (e.flags & QLineEditPrivate::SideWidgetFadeInWithText)
+            static_cast<QLineEditIconButton *>(e.widget)->animateShow(display);
+    }
+}
+#endif
+
 void QLineEditPrivate::_q_textChanged(const QString &text)
 {
     if (hasSideWidgets()) {
@@ -416,15 +464,9 @@ void QLineEditPrivate::_q_textChanged(const QString &text)
         if (!newTextSize || !lastTextSize) {
             lastTextSize = newTextSize;
 #if QT_CONFIG(animation) && QT_CONFIG(toolbutton)
-            const bool fadeIn = newTextSize > 0;
-            for (const SideWidgetEntry &e : leadingSideWidgets) {
-                if (e.flags & SideWidgetFadeInWithText)
-                   static_cast<QLineEditIconButton *>(e.widget)->animateShow(fadeIn);
-            }
-            for (const SideWidgetEntry &e : trailingSideWidgets) {
-                if (e.flags & SideWidgetFadeInWithText)
-                   static_cast<QLineEditIconButton *>(e.widget)->animateShow(fadeIn);
-            }
+            const bool display = newTextSize > 0;
+            displayWidgets(leadingSideWidgets, display);
+            displayWidgets(trailingSideWidgets, display);
 #endif
         }
     }
@@ -541,8 +583,15 @@ QWidget *QLineEditPrivate::addAction(QAction *newAction, QAction *before, QLineE
         QLineEditIconButton *toolButton = new QLineEditIconButton(q);
         toolButton->setIcon(newAction->icon());
         toolButton->setOpacity(lastTextSize > 0 || !(flags & SideWidgetFadeInWithText) ? 1 : 0);
-        if (flags & SideWidgetClearButton)
+        if (flags & SideWidgetClearButton) {
             QObject::connect(toolButton, SIGNAL(clicked()), q, SLOT(_q_clearButtonClicked()));
+
+#if QT_CONFIG(animation)
+            // The clear button is handled only by this widget. The button should be really
+            // shown/hidden in order to calculate size hints correctly.
+            toolButton->setHideWithText(true);
+#endif
+        }
         toolButton->setDefaultAction(newAction);
         w = toolButton;
 #else
@@ -606,33 +655,26 @@ void QLineEditPrivate::removeAction(QAction *action)
 #endif // QT_CONFIG(action)
 }
 
-static bool isSideWidgetVisible(const QLineEditPrivate::SideWidgetEntry &e)
+static int effectiveTextMargin(int defaultMargin, const QLineEditPrivate::SideWidgetEntryList &widgets,
+                               const QLineEditPrivate::SideWidgetParameters &parameters)
 {
-   return e.widget->isVisible();
+    if (widgets.empty())
+        return defaultMargin;
+
+    return defaultMargin + (parameters.margin + parameters.widgetWidth) *
+           int(std::count_if(widgets.begin(), widgets.end(),
+                             [](const QLineEditPrivate::SideWidgetEntry &e) {
+                                 return e.widget->isVisibleTo(e.widget->parentWidget()); }));
 }
 
 int QLineEditPrivate::effectiveLeftTextMargin() const
 {
-    int result = leftTextMargin;
-    if (!leftSideWidgetList().empty()) {
-        const SideWidgetParameters p = sideWidgetParameters();
-        result += (p.margin + p.widgetWidth)
-            * int(std::count_if(leftSideWidgetList().begin(), leftSideWidgetList().end(),
-                                isSideWidgetVisible));
-    }
-    return result;
+    return effectiveTextMargin(leftTextMargin, leftSideWidgetList(), sideWidgetParameters());
 }
 
 int QLineEditPrivate::effectiveRightTextMargin() const
 {
-    int result = rightTextMargin;
-    if (!rightSideWidgetList().empty()) {
-        const SideWidgetParameters p = sideWidgetParameters();
-        result += (p.margin + p.widgetWidth)
-            * int(std::count_if(rightSideWidgetList().begin(), rightSideWidgetList().end(),
-                                isSideWidgetVisible));
-    }
-    return result;
+    return effectiveTextMargin(rightTextMargin, rightSideWidgetList(), sideWidgetParameters());
 }
 
 

src/widgets/widgets/qlineedit_p.h

@@ -90,7 +90,10 @@ public:
     qreal opacity() const { return m_opacity; }
     void setOpacity(qreal value);
 #if QT_CONFIG(animation)
-    void animateShow(bool visible) { startOpacityAnimation(visible ? 1.0 : 0.0); }
+    void animateShow(bool visible);
+
+    bool shouldHideWithText() const;
+    void setHideWithText(bool hide);
 #endif
 
 protected:
@@ -100,6 +103,10 @@ protected:
 private slots:
     void updateCursor();
 
+#if QT_CONFIG(animation)
+    void onAnimationFinished();
+#endif
+
 private:
 #if QT_CONFIG(animation)
     void startOpacityAnimation(qreal endValue);
@@ -107,6 +114,12 @@ private:
     QLineEditPrivate *lineEditPrivate() const;
 
     qreal m_opacity;
+
+#if QT_CONFIG(animation)
+    bool m_hideWithText = false;
+    bool m_wasHidden = false;
+#endif
+
 };
 #endif // QT_CONFIG(toolbutton)
 

tests/auto/widgets/widgets/qlineedit/tst_qlineedit.cpp

@@ -4451,10 +4451,11 @@ void tst_QLineEdit::clearButtonVisibleAfterSettingText_QTBUG_45518()
     QTRY_VERIFY(clearButton->opacity() > 0);
     QTRY_COMPARE(clearButton->cursor().shape(), Qt::ArrowCursor);
 
-    QTest::mouseClick(clearButton, Qt::LeftButton, 0, clearButton->rect().center());
+    QTest::mouseClick(clearButton, Qt::LeftButton, nullptr, clearButton->rect().center());
     QTRY_COMPARE(edit.text(), QString());
 
     QTRY_COMPARE(clearButton->opacity(), qreal(0));
+    QVERIFY(clearButton->isHidden());
     QTRY_COMPARE(clearButton->cursor().shape(), clearButton->parentWidget()->cursor().shape());
 
     edit.setClearButtonEnabled(false);

tests/auto/widgets/widgets/qtoolbar/tst_qtoolbar.cpp

@@ -42,6 +42,7 @@
 #include <qlineedit.h>
 #include <qkeysequence.h>
 #include <qmenu.h>
+#include <qlabel.h>
 #include <private/qtoolbarextension_p.h>
 
 QT_FORWARD_DECLARE_CLASS(QAction)
@@ -80,6 +81,8 @@ private slots:
 
     void task191727_layout();
     void task197996_visibility();
+
+    void extraCpuConsumption(); // QTBUG-54676
 };
 
 
@@ -1098,5 +1101,81 @@ void tst_QToolBar::task197996_visibility()
     QTRY_VERIFY(toolBar->widgetForAction(pAction)->isVisible());
 }
 
+class ShowHideEventCounter : public QObject
+{
+public:
+    using QObject::QObject;
+
+    bool eventFilter(QObject *watched, QEvent *event) override
+    {
+        if (qobject_cast<QLineEdit*>(watched) && !event->spontaneous()) {
+            if (event->type() == QEvent::Show)
+                ++m_showEventsCount;
+
+            if (event->type() == QEvent::Hide)
+                ++m_hideEventsCount;
+        }
+
+        return QObject::eventFilter(watched, event);
+    }
+
+    uint showEventsCount() const { return m_showEventsCount; }
+    uint hideEventsCount() const { return m_hideEventsCount; }
+
+private:
+    uint m_showEventsCount = 0;
+    uint m_hideEventsCount = 0;
+};
+
+void tst_QToolBar::extraCpuConsumption()
+{
+    QMainWindow mainWindow;
+
+    auto tb = new QToolBar(&mainWindow);
+    tb->setMovable(false);
+
+    auto extensions = tb->findChildren<QToolBarExtension *>();
+    QVERIFY(!extensions.isEmpty());
+
+    auto extensionButton = extensions.at(0);
+    QVERIFY(extensionButton);
+
+    tb->addWidget(new QLabel("Lorem ipsum dolor sit amet"));
+
+    auto le = new QLineEdit;
+    le->setClearButtonEnabled(true);
+    le->setText("Lorem ipsum");
+    tb->addWidget(le);
+
+    mainWindow.addToolBar(tb);
+    mainWindow.show();
+    QVERIFY(QTest::qWaitForWindowActive(&mainWindow));
+
+    auto eventCounter = new ShowHideEventCounter(&mainWindow);
+    le->installEventFilter(eventCounter);
+
+    auto defaultSize = mainWindow.size();
+
+    // Line edit should be hidden now and extension button should be displayed
+    for (double p = 0.7; extensionButton->isHidden() || qFuzzyCompare(p, 0.); p -= 0.01) {
+        mainWindow.resize(int(defaultSize.width() * p), defaultSize.height());
+    }
+    QVERIFY(!extensionButton->isHidden());
+
+    // Line edit should be visible, but smaller
+    for (double p = 0.75; !extensionButton->isHidden() || qFuzzyCompare(p, 1.); p += 0.01) {
+        mainWindow.resize(int(defaultSize.width() * p), defaultSize.height());
+    }
+    QVERIFY(extensionButton->isHidden());
+
+    // Dispatch all pending events
+    qApp->sendPostedEvents();
+    qApp->processEvents();
+
+    QCOMPARE(eventCounter->showEventsCount(), eventCounter->hideEventsCount());
+    QCOMPARE(eventCounter->showEventsCount(), uint(1));
+    QCOMPARE(eventCounter->hideEventsCount(), uint(1));
+}
+
 QTEST_MAIN(tst_QToolBar)
 #include "tst_qtoolbar.moc"