From 732962d604e7469f9a9f02fe0cd3d1fd04caddb8 Mon Sep 17 00:00:00 2001 From: Santhosh Kumar Date: Fri, 22 Mar 2024 15:16:29 +0100 Subject: [PATCH] Support rendering CSS 'border' property for html table We supported CSS 'border-width', 'border-style' and 'border-color' for HTML tables since 8a9bec35fb0c60a0e5990c1a12ffe6f39fdbf2d. Now we also support the 'border' property, which is shorthand to set all four borders' width, style and color. Fixes: QTBUG-123167 Pick-to: 6.6 Change-Id: I5f29b94ab9facf412a9c230d554efb5c69368b6b Reviewed-by: Shawn Rutledge (cherry picked from commit 26e75d452eeb2762fa3ece1c63e94d01587c6260) Reviewed-by: Qt Cherry-pick Bot --- src/gui/text/qcssparser.cpp | 8 ++- src/gui/text/qtextdocumentlayout.cpp | 16 ++++- src/gui/text/qtexthtmlparser.cpp | 30 ++++++-- .../gui/text/qtexttable/tst_qtexttable.cpp | 68 +++++++++++++++++++ 4 files changed, 110 insertions(+), 12 deletions(-) diff --git a/src/gui/text/qcssparser.cpp b/src/gui/text/qcssparser.cpp index 2082263d7c0..c82e251d8ab 100644 --- a/src/gui/text/qcssparser.cpp +++ b/src/gui/text/qcssparser.cpp @@ -979,9 +979,11 @@ void ValueExtractor::borderValue(const Declaration &decl, int *width, QCss::Bord } data.color = parseBrushValue(decl.d->values.at(i), pal); - *color = brushFromData(data.color, pal); - if (data.color.type != BrushData::DependsOnThePalette) - decl.d->parsed = QVariant::fromValue(data); + if (data.color.type != BrushData::Invalid) { + *color = brushFromData(data.color, pal); + if (data.color.type != BrushData::DependsOnThePalette) + decl.d->parsed = QVariant::fromValue(data); + } } static void parseShorthandBackgroundProperty(const QList &values, BrushData *brush, QString *image, Repeat *repeat, Qt::Alignment *alignment, const QPalette &pal) diff --git a/src/gui/text/qtextdocumentlayout.cpp b/src/gui/text/qtextdocumentlayout.cpp index 83abe93dbf9..baff79973f2 100644 --- a/src/gui/text/qtextdocumentlayout.cpp +++ b/src/gui/text/qtextdocumentlayout.cpp @@ -1818,11 +1818,20 @@ void QTextDocumentLayoutPrivate::drawTableCell(const QRectF &cellRect, QPainter if (r >= headerRowCount) topMargin += td->headerHeight.toReal(); - if (!td->borderCollapse && td->border != 0) { + // If cell border configured, don't draw default border for cells. It will be taken care later by + // drawTableCellBorder(). + bool cellBorderConfigured = (cell.format().hasProperty(QTextFormat::TableCellLeftBorder) || + cell.format().hasProperty(QTextFormat::TableCellTopBorder) || + cell.format().hasProperty(QTextFormat::TableCellRightBorder) || + cell.format().hasProperty(QTextFormat::TableCellBottomBorder)); + + if (!td->borderCollapse && td->border != 0 && !cellBorderConfigured) { const QBrush oldBrush = painter->brush(); const QPen oldPen = painter->pen(); - const qreal border = td->border.toReal(); + // If border is configured for the table (and not explicitly for the cell), then + // always draw 1px border around the cell + const qreal border = 1; QRectF borderRect(cellRect.left() - border, cellRect.top() - border, cellRect.width() + border, cellRect.height() + border); @@ -1885,7 +1894,8 @@ void QTextDocumentLayoutPrivate::drawTableCell(const QRectF &cellRect, QPainter } // paint over the background - otherwise we would have to adjust the background paint cellRect for the border values - drawTableCellBorder(cellRect, painter, table, td, cell); + if (cellBorderConfigured) + drawTableCellBorder(cellRect, painter, table, td, cell); const QFixed verticalOffset = td->cellVerticalOffsets.at(c + r * table->columns()); diff --git a/src/gui/text/qtexthtmlparser.cpp b/src/gui/text/qtexthtmlparser.cpp index 1168c6d5cd8..81d29d72be2 100644 --- a/src/gui/text/qtexthtmlparser.cpp +++ b/src/gui/text/qtexthtmlparser.cpp @@ -1181,7 +1181,7 @@ void QTextHtmlParserNode::applyCssDeclarations(const QList &d QCss::ValueExtractor extractor(declarations); extractor.extractBox(margin, padding); - if (id == Html_td || id == Html_th) { + auto getBorderValues = [&extractor](qreal *borderWidth, QBrush *borderBrush, QTextFrameFormat::BorderStyle *borderStyles) { QCss::BorderStyle cssStyles[4]; int cssBorder[4]; QSize cssRadii[4]; // unused @@ -1193,12 +1193,16 @@ void QTextHtmlParserNode::applyCssDeclarations(const QList &d // QCss::BorderWidth parsing below which expects a single value // will not work as expected - which in this case does not matter // because tableBorder is not relevant for cells. - extractor.extractBorder(cssBorder, tableCellBorderBrush, cssStyles, cssRadii); + bool hit = extractor.extractBorder(cssBorder, borderBrush, cssStyles, cssRadii); for (int i = 0; i < 4; ++i) { - tableCellBorderStyle[i] = toQTextFrameFormat(cssStyles[i]); - tableCellBorder[i] = static_cast(cssBorder[i]); + borderStyles[i] = toQTextFrameFormat(cssStyles[i]); + borderWidth[i] = static_cast(cssBorder[i]); } - } + return hit; + }; + + if (id == Html_td || id == Html_th) + getBorderValues(tableCellBorder, tableCellBorderBrush, tableCellBorderStyle); for (int i = 0; i < declarations.size(); ++i) { const QCss::Declaration &decl = declarations.at(i); @@ -1220,6 +1224,19 @@ void QTextHtmlParserNode::applyCssDeclarations(const QList &d tableBorder = borders[0]; } break; + case QCss::Border: { + qreal tblBorder[4]; + QBrush tblBorderBrush[4]; + QTextFrameFormat::BorderStyle tblBorderStyle[4]; + if (getBorderValues(tblBorder, tblBorderBrush, tblBorderStyle)) { + tableBorder = tblBorder[0]; + if (tblBorderBrush[0].color().isValid()) + borderBrush = tblBorderBrush[0]; + if (tblBorderStyle[0] != static_cast(-1)) + borderStyle = tblBorderStyle[0]; + } + } + break; case QCss::BorderCollapse: borderCollapse = decl.borderCollapseValue(); break; @@ -1697,7 +1714,8 @@ void QTextHtmlParser::applyAttributes(const QStringList &attributes) } break; case Html_table: - if (key == "border"_L1) { + // If table border already set through css style, prefer that one otherwise consider this value + if (key == "border"_L1 && !node->tableBorder) { setFloatAttribute(&node->tableBorder, value); } else if (key == "bgcolor"_L1) { QColor c = QColor::fromString(value); diff --git a/tests/auto/gui/text/qtexttable/tst_qtexttable.cpp b/tests/auto/gui/text/qtexttable/tst_qtexttable.cpp index 6fb5858c644..d0e1e1cd745 100644 --- a/tests/auto/gui/text/qtexttable/tst_qtexttable.cpp +++ b/tests/auto/gui/text/qtexttable/tst_qtexttable.cpp @@ -75,6 +75,8 @@ private slots: #endif void checkBorderAttributes_data(); void checkBorderAttributes(); + void checkTableBorderAttributes_data(); + void checkTableBorderAttributes(); #ifndef QT_NO_WIDGETS void columnWidthWithSpans(); @@ -1261,6 +1263,72 @@ void tst_QTextTable::checkBorderAttributes() } } +void tst_QTextTable::checkTableBorderAttributes_data() +{ + QTest::addColumn("html"); + QTest::addColumn("tableBorderWidth"); + QTest::addColumn("tableBorderStyle"); + QTest::addColumn("tableBorderBrush"); + + const QString tableHtmlStart = QStringLiteral("" + "
OneTwo
" + ""); + const QString tableHtmlEnd2 = QStringLiteral("" + "
OneTwo
" + ""); + + QTest::newRow("table-border-attributes-shorthand") + << QString("%1" + "table {" + "border: 2px solid red;" + "}" + "%2").arg(tableHtmlStart).arg(tableHtmlEnd1) + << 2.0 << QTextFrameFormat::BorderStyle_Solid << QBrush(Qt::red); + + QTest::newRow("table-border-attributes-explicit") + << QString("%1" + "table {" + "border-width: 2px;" + "border-color: red;" + "border-style: dashed;" + "}" + "%2").arg(tableHtmlStart).arg(tableHtmlEnd1) + << 2.0 << QTextFrameFormat::BorderStyle_Dashed << QBrush(Qt::red); + + QTest::newRow("table-border-override") + << QString("%1" + "table {" + "border: 2px solid red;" + "}" + "%2").arg(tableHtmlStart).arg(tableHtmlEnd2) + << 2.0 << QTextFrameFormat::BorderStyle_Solid << QBrush(Qt::red); + + QTest::newRow("table-border-default") + << QString("%1" + "%2").arg(tableHtmlStart).arg(tableHtmlEnd2) + << 10.0 << QTextFrameFormat::BorderStyle_Outset << QBrush(Qt::darkGray); +} + +void tst_QTextTable::checkTableBorderAttributes() +{ + QFETCH(QString, html); + QFETCH(qreal, tableBorderWidth); + QFETCH(QTextFrameFormat::BorderStyle, tableBorderStyle); + QFETCH(QBrush, tableBorderBrush); + + QTextDocument doc; + doc.setHtml(html); + QTextCursor cursor(doc.firstBlock()); + cursor.movePosition(QTextCursor::Right); + + QTextTable *currentTable = cursor.currentTable(); + QVERIFY(currentTable); + QCOMPARE(currentTable->format().border(), tableBorderWidth); + QCOMPARE(currentTable->format().borderStyle(), tableBorderStyle); + QCOMPARE(currentTable->format().borderBrush(), tableBorderBrush); +} + #ifndef QT_NO_WIDGETS void tst_QTextTable::columnWidthWithSpans() {