From 8513bcd90cc3900f9c32e59e0823f621ea6eea1d Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Mon, 11 Oct 2021 13:03:44 +0200
Subject: Fix caching of parsed border color values in CSS parser

When parsing CSS, a border-color value is parsed as four brushes, as css
allows assigning up to four values, one for each side.

When applying the CSS to the HTML, we accessed it as a color value,
which overwrote the parsed value with a QColor. So while we had a valid
parsed value (and didn't re-parse), the code accessing that value still
expected it to be a list, and thus failed to retrieve the data.

There are several ways to fix that, but the cleanest way without
introducing any performance penalty from repeatedly parsing (and in fact
removing a parse of the string into a color) is to enable colorValue to
interpret an already parsed value that is a list without overwriting the
parsed value again. To avoid similar issues in the future, add assert
that the parsed value has the right type in brushValues.

As a drive-by, speed things up further by making use of qMetaTypeId
being constexpr, which allows for it to be used in a switch statement.

Add a test case.

Fixes: QTBUG-96603
Pick-to: 6.2 5.15
Change-Id: Icdbff874daedc91bff497cd0cd1d99e4c713217c
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
---
 .../gui/text/qtextdocument/tst_qtextdocument.cpp   | 124 +++++++++++++++++++++
 1 file changed, 124 insertions(+)

(limited to 'tests')

diff --git a/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp b/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp
index 1a644d1bdd..74c8bfd260 100644
--- a/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp
+++ b/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp
@@ -124,7 +124,10 @@ private slots:
     void clonePreservesIndentWidth();
     void clonePreservesFormatsWhenEmpty();
     void blockCount();
+
     void defaultStyleSheet();
+    void defaultTableStyle_data();
+    void defaultTableStyle();
 
     void resolvedFontInEmptyFormat();
 
@@ -2648,6 +2651,127 @@ void tst_QTextDocument::defaultStyleSheet()
     QVERIFY(fmt.background().color() != QColor("green"));
 }
 
+void tst_QTextDocument::defaultTableStyle_data()
+{
+    QTest::addColumn<QString>("css");
+    QTest::addColumn<QString>("html");
+    QTest::addColumn<QList<QBrush>>("borderBrushes");
+
+    const QString htmlHeader(R"(
+        <tr>
+            <th>1</th> <th>2</th>
+        </tr>
+    )");
+
+    const QString htmlCells(R"(
+        <tr>
+            <td>A</td> <td>B</td>
+        </tr>
+    )");
+
+    const QString cssEachSide = R"({
+        border-top-color: red;
+        border-bottom-color: blue;
+        border-left-color: green;
+        border-right-color: yellow;
+    })";
+    const QString cssOneColor = R"({ border-color: red; })";
+    const QString cssFourColors = R"({ border-color: red blue green yellow; })";
+
+    QTest::addRow("td, each side") << QString("td ") + cssEachSide
+        << htmlCells
+        << QList<QBrush>{Qt::red, Qt::blue, QColor("green"), Qt::yellow};
+
+    QTest::addRow("th, each side") << QString("th ") + cssEachSide
+        << htmlHeader
+        << QList<QBrush>{Qt::red, Qt::blue, QColor("green"), Qt::yellow};
+
+    QTest::addRow("th+td, each side") << QString("th %1 td %1").arg(cssEachSide)
+        << htmlHeader + htmlCells
+        << QList<QBrush>{Qt::red, Qt::blue, QColor("green"), Qt::yellow};
+
+    QTest::addRow("td, one color") << QString("td ") + cssOneColor
+        << htmlCells
+        << QList<QBrush>{Qt::red, Qt::red, Qt::red, Qt::red};
+
+    QTest::addRow("th, one color") << QString("th ") + cssOneColor
+        << htmlHeader
+        << QList<QBrush>{Qt::red, Qt::red, Qt::red, Qt::red};
+
+    QTest::addRow("th+td, one color") << QString("th %1 td %1").arg(cssOneColor)
+        << htmlHeader + htmlCells
+        << QList<QBrush>{Qt::red, Qt::red, Qt::red, Qt::red};
+
+    // css order is top, right, bottom, left
+    QTest::addRow("td, four colors") << QString("td ") + cssFourColors
+        << htmlCells
+        << QList<QBrush>{Qt::red, QColor("green"), Qt::yellow, Qt::blue};
+
+}
+
+void tst_QTextDocument::defaultTableStyle()
+{
+    QFETCH(QString, css);
+    QFETCH(QString, html);
+    QFETCH(QList<QBrush>, borderBrushes);
+
+    CREATE_DOC_AND_CURSOR();
+    doc.setDefaultStyleSheet(css);
+    doc.setHtml(QString("<html><body><table>%1</table></body></html>").arg(html));
+
+    const QTextFrame *frame = doc.rootFrame();
+    const QTextTable *table = nullptr;
+    for (auto it = frame->begin(); !table && !it.atEnd(); ++it)
+        table = qobject_cast<const QTextTable*>(it.currentFrame());
+    QVERIFY(table);
+
+    const QList<QTextFormat::Property> brushProperties = {
+        QTextFormat::TableCellTopBorderBrush,
+        QTextFormat::TableCellBottomBorderBrush,
+        QTextFormat::TableCellLeftBorderBrush,
+        QTextFormat::TableCellRightBorderBrush
+    };
+
+    for (int row = 0; row < table->rows(); ++row) {
+        for (int column = 0; column < table->columns(); ++column) {
+            auto cellDetails = qScopeGuard([&]{
+                qWarning("Failure was in cell %d/%d", row, column);
+            });
+            const QTextTableCell cell = table->cellAt(row, column);
+            const QTextTableCellFormat cellFormat = cell.format().toTableCellFormat();
+            QList<QBrush> brushes;
+            for (const auto side : brushProperties) {
+                QVariant sideProperty = cellFormat.property(side);
+                QVERIFY(sideProperty.isValid());
+                QVERIFY(sideProperty.typeId() == qMetaTypeId<QBrush>());
+                brushes << sideProperty.value<QBrush>();
+            }
+            auto errorDetails = qScopeGuard([&]{
+                if (brushes.count() != borderBrushes.count()) {
+                    qWarning("Different count: %lld vs %lld", brushes.count(), borderBrushes.count());
+                    return;
+                }
+                for (int i = 0; i < brushes.count(); ++i) {
+                    QString side;
+                    QDebug(&side) << QTextFormat::Property(QTextFormat::TableCellTopBorderBrush + i);
+                    QString actual;
+                    QDebug(&actual) << brushes.at(i);
+                    QString expected;
+                    QDebug(&expected) << borderBrushes.at(i);
+                    if (expected != actual) {
+                        qWarning("\n  %s:\n\tActual:  %s\n\tExpected:%s", qPrintable(side),
+                                qPrintable(actual), qPrintable(expected));
+                    }
+                }
+            });
+            QVERIFY2(borderBrushes == brushes, // QCOMPARE doesn't generate helpful output anyway
+                     qPrintable(QString("for cell %1/%2").arg(row).arg(column)));
+            errorDetails.dismiss();
+            cellDetails.dismiss();
+        }
+    }
+}
+
 void tst_QTextDocument::maximumBlockCount()
 {
     QCOMPARE(doc->maximumBlockCount(), 0);
-- 
cgit v1.2.3