AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix caching of parsed border color values in CSS parser

Browse Source 
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>
This commit is contained in:
Volker Hilsheimer 2021-10-11 13:03:44 +02:00
parent 878b2047b5
commit 8513bcd90c
2 changed files with 135 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/gui/text/qcssparser.cpp
View File

@ -1460,10 +1460,18 @@ QColor Declaration::colorValue(const QPalette &pal) const
return QColor();
if (d->parsed.isValid()) {
if (d->parsed.userType() == QMetaType::QColor)
switch (d->parsed.typeId()) {
case qMetaTypeId<QColor>():
return qvariant_cast<QColor>(d->parsed);
if (d->parsed.userType() == QMetaType::Int)
case qMetaTypeId<int>():
return pal.color((QPalette::ColorRole)(d->parsed.toInt()));
case qMetaTypeId<QList<QVariant>>():
if (d->parsed.toList().size() == 1) {
const auto &value = d->parsed.toList().at(0);
return qvariant_cast<QColor>(value);
}
break;
}
}
ColorData color = parseColorValue(d->values.at(0));
@ -1507,6 +1515,7 @@ void Declaration::brushValues(QBrush *c, const QPalette &pal) const
int i = 0;
if (d->parsed.isValid()) {
needParse = 0;
Q_ASSERT(d->parsed.metaType() == QMetaType::fromType<QList<QVariant>>());
QList<QVariant> v = d->parsed.toList();
for (i = 0; i < qMin(v.count(), 4); i++) {
if (v.at(i).userType() == QMetaType::QBrush) {

124
tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp
View File

@ -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);