diff options
author | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-10-11 13:03:44 +0200 |
---|---|---|
committer | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-10-12 18:26:26 +0200 |
commit | 8513bcd90cc3900f9c32e59e0823f621ea6eea1d (patch) | |
tree | 28f40070f1079f9e3ed8dd5b885637a1c3ef07c4 /tests | |
parent | 878b2047b52c93e904eb46ef1044819a8b5614ab (diff) |
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>
Diffstat (limited to 'tests')
-rw-r--r-- | tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp | 124 |
1 files changed, 124 insertions, 0 deletions
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); |