Fix issue where updated text in layout is not correctly rewrapped

When the text element is inside a Qt Quick layout and we set the implicit width,
this can trigger the Layout to resize the width further down in the stack. At
this point, a recursion guard we have in place to protect against binding
loops will cause us to ignore the size update. The end result is that the text
width has changed, but the contents have not been updated to reflect this.

This change detects this case by checking if the width of the text changed
while the recursion guard was in place, and if it was, we call updateSize()
again to make sure everything is up-to-date.

I added a second admittedly ugly recursion guard for this out of paranoia.
I haven't seen any case where this might cause an infinite recursion, but
it seemed better to be on the safe side.

[ChangeLog][Text] Fixed an issue where updating text inside a layout would
not cause it to correctly adapt to its new width.

Task-number: QTBUG-53279
Change-Id: I1e421a7073428fa490a24be36208a2077f5836dd
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

4 files changed, 76 insertions, 17 deletions

diff --git a/src/quick/items/qquicktext.cpp b/src/quick/items/qquicktext.cpp
index 9eaf9f8d6d..dd66930ef7 100644
--- a/src/quick/items/qquicktext.cpp
+++ b/src/quick/items/qquicktext.cpp
@@ -88,6 +88,7 @@ QQuickTextPrivate::QQuickTextPrivate()
     , truncated(false), hAlignImplicit(true), rightToLeftText(false)
     , layoutTextElided(false), textHasChanged(true), needToUpdateLayout(false), formatModifiesFontSize(false)
     , polishSize(false)
+    , updateSizeRecursionGuard(false)
 {
     implicitAntialiasing = true;
 }
@@ -442,6 +443,7 @@ void QQuickTextPrivate::updateSize()
 
         //### need to confirm cost of always setting these for richText
         internalWidthUpdate = true;
+        qreal oldWidth = q->width();
         qreal iWidth = -1;
         if (!q->widthValid())
             iWidth = size.width();
@@ -449,24 +451,35 @@ void QQuickTextPrivate::updateSize()
             q->setImplicitSize(iWidth + hPadding, size.height() + vPadding);
         internalWidthUpdate = false;
 
-        if (iWidth == -1)
-            q->setImplicitHeight(size.height() + vPadding);
-
-        QTextBlock firstBlock = extra->doc->firstBlock();
-        while (firstBlock.layout()->lineCount() == 0)
-            firstBlock = firstBlock.next();
-
-        QTextBlock lastBlock = extra->doc->lastBlock();
-        while (lastBlock.layout()->lineCount() == 0)
-            lastBlock = lastBlock.previous();
-
-        if (firstBlock.lineCount() > 0 && lastBlock.lineCount() > 0) {
-            QTextLine firstLine = firstBlock.layout()->lineAt(0);
-            QTextLine lastLine = lastBlock.layout()->lineAt(lastBlock.layout()->lineCount() - 1);
-            advance = QSizeF(lastLine.horizontalAdvance(),
-                             (lastLine.y() + lastBlock.layout()->position().y()) - (firstLine.y() + firstBlock.layout()->position().y()));
+        // If the implicit width update caused a recursive change of the width,
+        // we will have skipped integral parts of the layout due to the
+        // internalWidthUpdate recursion guard. To make sure everything is up
+        // to date, we need to run a second pass over the layout when updateSize()
+        // is done.
+        if (!qFuzzyCompare(q->width(), oldWidth) && !updateSizeRecursionGuard) {
+            updateSizeRecursionGuard = true;
+            updateSize();
+            updateSizeRecursionGuard = false;
         } else {
-            advance = QSizeF();
+            if (iWidth == -1)
+                q->setImplicitHeight(size.height() + vPadding);
+
+            QTextBlock firstBlock = extra->doc->firstBlock();
+            while (firstBlock.layout()->lineCount() == 0)
+                firstBlock = firstBlock.next();
+
+            QTextBlock lastBlock = extra->doc->lastBlock();
+            while (lastBlock.layout()->lineCount() == 0)
+                lastBlock = lastBlock.previous();
+
+            if (firstBlock.lineCount() > 0 && lastBlock.lineCount() > 0) {
+                QTextLine firstLine = firstBlock.layout()->lineAt(0);
+                QTextLine lastLine = lastBlock.layout()->lineAt(lastBlock.layout()->lineCount() - 1);
+                advance = QSizeF(lastLine.horizontalAdvance(),
+                                 (lastLine.y() + lastBlock.layout()->position().y()) - (firstLine.y() + firstBlock.layout()->position().y()));
+            } else {
+                advance = QSizeF();
+            }
         }
     }
 
diff --git a/src/quick/items/qquicktext_p_p.h b/src/quick/items/qquicktext_p_p.h
index b0b1492d57..fd26d966c8 100644
--- a/src/quick/items/qquicktext_p_p.h
+++ b/src/quick/items/qquicktext_p_p.h
@@ -174,6 +174,7 @@ public:
     bool needToUpdateLayout:1;
     bool formatModifiesFontSize:1;
     bool polishSize:1; // Workaround for problem with polish called after updateSize (QTBUG-42636)
+    bool updateSizeRecursionGuard:1;
 
     static const QChar elideChar;
 
diff --git a/tests/auto/quick/qquicktext/data/implicitSizeChangeRewrap.qml b/tests/auto/quick/qquicktext/data/implicitSizeChangeRewrap.qml
new file mode 100644
index 0000000000..fb8626a75a
--- /dev/null
+++ b/tests/auto/quick/qquicktext/data/implicitSizeChangeRewrap.qml
@@ -0,0 +1,27 @@
+import QtQuick 2.0
+import QtQuick.Layouts 1.0
+
+Item
+{
+  id : _rootItem
+  width : 200
+  height : 1000
+        ColumnLayout
+        {
+            id : _textContainer
+            anchors.centerIn: parent
+            Layout.maximumWidth: (_rootItem.width - 40) // to have some space left / right
+            Text
+            {
+                id : text
+                objectName: "text"
+                font.italic: true
+                textFormat: Text.RichText
+                horizontalAlignment :   Text.AlignHCenter
+                verticalAlignment :     Text.AlignVCenter
+                wrapMode: Text.Wrap
+                Layout.maximumWidth: (_rootItem.width - 60)
+                Component.onCompleted: text.text = "This is a too long text for the interface with a stupid path also too long -> /home/long/long/long/to/force/it/to/need/to/wrap This is a too long text for the interface with a stupid path also too long -> /home/long/long/long/to/force/it/to/need/to/wrap This is a too long text for the interface with a stupid path also too long -> /home/long/long/long/to/force/it/to/need/to/wrap This is a too long text for the interface with a stupid path also too long -> /home/long/long/long/to/force/it/to/need/to/wrap"
+            }
+    }
+}
diff --git a/tests/auto/quick/qquicktext/tst_qquicktext.cpp b/tests/auto/quick/qquicktext/tst_qquicktext.cpp
index 89d2590c03..5f346a6e0a 100644
--- a/tests/auto/quick/qquicktext/tst_qquicktext.cpp
+++ b/tests/auto/quick/qquicktext/tst_qquicktext.cpp
@@ -105,6 +105,7 @@ private slots:
 
     void implicitSize_data();
     void implicitSize();
+    void implicitSizeChangeRewrap();
     void dependentImplicitSizes();
     void contentSize();
     void implicitSizeBinding_data();
@@ -4369,6 +4370,23 @@ void tst_qquicktext::fontInfo()
     QVERIFY(copy->font().pixelSize() < 1000);
 }
 
+void tst_qquicktext::implicitSizeChangeRewrap()
+{
+    QScopedPointer<QQuickView> window(new QQuickView);
+    window->setSource(testFileUrl("implicitSizeChangeRewrap.qml"));
+    QTRY_COMPARE(window->status(), QQuickView::Ready);
+
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window.data()));
+
+    QObject *root = window->rootObject();
+
+    QQuickText *text = root->findChild<QQuickText *>("text");
+    QVERIFY(text != nullptr);
+
+    QVERIFY(text->contentWidth() < window->width());
+}
+
 QTEST_MAIN(tst_qquicktext)
 
 #include "tst_qquicktext.moc"