From 8090adf268ca616f306c7093275a4e8a25a4c8c9 Mon Sep 17 00:00:00 2001 From: Andi Gallo Date: Sun, 4 Jun 2023 14:21:20 +0000 Subject: [PATCH] LibWeb: Add partial implementation of border conflict resolution Fix handling of border style specified per column as well. --- .../border-conflict-resolution-with-col.txt | 70 ++++++++ .../border-conflict-resolution-with-col.html | 42 +++++ .../Libraries/LibWeb/HTML/AttributeNames.h | 1 + .../Libraries/LibWeb/Layout/LayoutState.cpp | 3 + .../Libraries/LibWeb/Layout/LayoutState.h | 5 + .../LibWeb/Layout/TableFormattingContext.cpp | 151 ++++++++++++++++-- .../LibWeb/Layout/TableFormattingContext.h | 20 +++ .../LibWeb/Painting/PaintableBox.cpp | 2 +- .../Libraries/LibWeb/Painting/PaintableBox.h | 4 + 9 files changed, 282 insertions(+), 16 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/table/border-conflict-resolution-with-col.txt create mode 100644 Tests/LibWeb/Layout/input/table/border-conflict-resolution-with-col.html diff --git a/Tests/LibWeb/Layout/expected/table/border-conflict-resolution-with-col.txt b/Tests/LibWeb/Layout/expected/table/border-conflict-resolution-with-col.txt new file mode 100644 index 0000000000..45cf47b56e --- /dev/null +++ b/Tests/LibWeb/Layout/expected/table/border-conflict-resolution-with-col.txt @@ -0,0 +1,70 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x181.875 [BFC] children: not-inline + BlockContainer <(anonymous)> at (0,0) content-size 800x0 children: inline + TextNode <#text> + BlockContainer at (8,8) content-size 784x165.875 children: not-inline + BlockContainer <(anonymous)> at (8,8) content-size 784x0 children: inline + TextNode <#text> + TableWrapper <(anonymous)> at (8,8) content-size 64.265625x165.875 [BFC] children: not-inline + Box at (8,8) content-size 64.265625x165.875 table-box [TFC] children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer (not painted) table-column-group children: not-inline + BlockContainer (not painted) children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + Box at (8,8) content-size 64.265625x165.875 table-row-group children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + Box at (8,8) content-size 64.265625x43.46875 table-row children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer at (8,51.46875) content-size 64.265625x39.46875 table-row children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer at (8,90.9375) content-size 64.265625x39.46875 table-row children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer at (8,130.40625) content-size 64.265625x43.46875 table-row children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer
at (33,23) content-size 14.265625x17.46875 table-cell [BFC] children: inline + line 0 width: 14.265625, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 1, rect: [33,23 14.265625x17.46875] + "A" + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + Box
at (33,62.46875) content-size 14.265625x17.46875 table-cell [BFC] children: inline + line 0 width: 9.34375, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 1, rect: [33,62.46875 9.34375x17.46875] + "B" + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + Box
at (33,101.9375) content-size 14.265625x17.46875 table-cell [BFC] children: inline + line 0 width: 10.3125, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 1, rect: [33,101.9375 10.3125x17.46875] + "C" + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + Box
at (33,141.40625) content-size 14.265625x17.46875 table-cell [BFC] children: inline + line 0 width: 11.140625, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 1, rect: [33,141.40625 11.140625x17.46875] + "D" + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer <(anonymous)> at (8,173.875) content-size 784x0 children: inline + TextNode <#text> diff --git a/Tests/LibWeb/Layout/input/table/border-conflict-resolution-with-col.html b/Tests/LibWeb/Layout/input/table/border-conflict-resolution-with-col.html new file mode 100644 index 0000000000..b5b29f49f9 --- /dev/null +++ b/Tests/LibWeb/Layout/input/table/border-conflict-resolution-with-col.html @@ -0,0 +1,42 @@ + + + + + Table with border conflict resolution with col + + + + + + + + + + + + + + + + + + + + + + +
A
B
C
D
+ + + + + \ No newline at end of file diff --git a/Userland/Libraries/LibWeb/HTML/AttributeNames.h b/Userland/Libraries/LibWeb/HTML/AttributeNames.h index 083375ebd4..daf9bac4ad 100644 --- a/Userland/Libraries/LibWeb/HTML/AttributeNames.h +++ b/Userland/Libraries/LibWeb/HTML/AttributeNames.h @@ -211,6 +211,7 @@ namespace AttributeNames { __ENUMERATE_HTML_ATTRIBUTE(shape) \ __ENUMERATE_HTML_ATTRIBUTE(size) \ __ENUMERATE_HTML_ATTRIBUTE(sizes) \ + __ENUMERATE_HTML_ATTRIBUTE(span) \ __ENUMERATE_HTML_ATTRIBUTE(src) \ __ENUMERATE_HTML_ATTRIBUTE(srcdoc) \ __ENUMERATE_HTML_ATTRIBUTE(srclang) \ diff --git a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp index 8d765ca4ae..0149c863eb 100644 --- a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp +++ b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp @@ -122,6 +122,9 @@ void LayoutState::commit() paintable_box.set_offset(used_values.offset); paintable_box.set_content_size(used_values.content_width(), used_values.content_height()); paintable_box.set_containing_line_box_fragment(used_values.containing_line_box_fragment); + if (used_values.override_borders_data().has_value()) { + paintable_box.set_override_borders_data(used_values.override_borders_data().value()); + } if (is(box)) { for (auto& line_box : used_values.line_boxes) { diff --git a/Userland/Libraries/LibWeb/Layout/LayoutState.h b/Userland/Libraries/LibWeb/Layout/LayoutState.h index ffe55d9d19..47814383f9 100644 --- a/Userland/Libraries/LibWeb/Layout/LayoutState.h +++ b/Userland/Libraries/LibWeb/Layout/LayoutState.h @@ -117,6 +117,9 @@ struct LayoutState { void add_floating_descendant(Box const& box) { m_floating_descendants.set(&box); } auto const& floating_descendants() const { return m_floating_descendants; } + void set_override_borders_data(Painting::BordersData const& override_borders_data) { m_override_borders_data = override_borders_data; }; + auto const& override_borders_data() const { return m_override_borders_data; } + private: AvailableSize available_width_inside() const; AvailableSize available_height_inside() const; @@ -130,6 +133,8 @@ struct LayoutState { bool m_has_definite_height { false }; HashTable> m_floating_descendants; + + Optional m_override_borders_data; }; void commit(); diff --git a/Userland/Libraries/LibWeb/Layout/TableFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/TableFormattingContext.cpp index fcb916a777..cc831de9eb 100644 --- a/Userland/Libraries/LibWeb/Layout/TableFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/TableFormattingContext.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -161,11 +162,10 @@ void TableFormattingContext::compute_table_measures() CSSPixels padding_left = computed_values.padding().left().to_px(cell.box, width_of_containing_block); CSSPixels padding_right = computed_values.padding().right().to_px(cell.box, width_of_containing_block); - auto is_left_most_cell = cell.column_index == 0; - auto is_right_most_cell = cell.column_index == m_columns.size() - 1; - auto should_hide_borders = cell.box->computed_values().border_collapse() == CSS::BorderCollapse::Collapse; - CSSPixels border_left = should_hide_borders && !is_left_most_cell ? 0 : computed_values.border_left().width; - CSSPixels border_right = should_hide_borders && !is_right_most_cell ? 0 : computed_values.border_right().width; + auto const& cell_state = m_state.get(cell.box); + auto is_collapse = cell.box->computed_values().border_collapse() == CSS::BorderCollapse::Collapse; + CSSPixels border_left = is_collapse ? cell_state.border_left : computed_values.border_left().width; + CSSPixels border_right = is_collapse ? cell_state.border_right : computed_values.border_right().width; CSSPixels width = computed_values.width().to_px(cell.box, width_of_containing_block); auto cell_intrinsic_offsets = padding_left + padding_right + border_left + border_right; @@ -499,16 +499,12 @@ void TableFormattingContext::compute_table_height(LayoutMode layout_mode) cell_state.padding_left = cell.box->computed_values().padding().left().to_px(cell.box, width_of_containing_block); cell_state.padding_right = cell.box->computed_values().padding().right().to_px(cell.box, width_of_containing_block); - auto is_top_most_cell = cell.row_index == 0; - auto is_left_most_cell = cell.column_index == 0; - auto is_right_most_cell = cell.column_index == m_columns.size() - 1; - auto is_bottom_most_cell = cell.row_index == m_rows.size() - 1; - auto should_hide_borders = cell.box->computed_values().border_collapse() == CSS::BorderCollapse::Collapse; - - cell_state.border_top = (should_hide_borders && is_top_most_cell) ? 0 : cell.box->computed_values().border_top().width; - cell_state.border_bottom = (should_hide_borders && is_bottom_most_cell) ? 0 : cell.box->computed_values().border_bottom().width; - cell_state.border_left = (should_hide_borders && is_left_most_cell) ? 0 : cell.box->computed_values().border_left().width; - cell_state.border_right = (should_hide_borders && is_right_most_cell) ? 0 : cell.box->computed_values().border_right().width; + if (cell.box->computed_values().border_collapse() == CSS::BorderCollapse::Separate) { + cell_state.border_top = cell.box->computed_values().border_top().width; + cell_state.border_bottom = cell.box->computed_values().border_bottom().width; + cell_state.border_left = cell.box->computed_values().border_left().width; + cell_state.border_right = cell.box->computed_values().border_right().width; + } auto cell_computed_height = cell.box->computed_values().height(); if (cell_computed_height.is_length()) { @@ -744,6 +740,84 @@ void TableFormattingContext::position_cell_boxes() } } +static const CSS::BorderData& winning_border_style(const CSS::BorderData& a, const CSS::BorderData& b) +{ + // Implements steps 1, 2 and 3 of border conflict resolution algorithm. + static HashMap const line_style_score = { + { CSS::LineStyle::Inset, 0 }, + { CSS::LineStyle::Groove, 1 }, + { CSS::LineStyle::Outset, 2 }, + { CSS::LineStyle::Ridge, 3 }, + { CSS::LineStyle::Dotted, 4 }, + { CSS::LineStyle::Dashed, 5 }, + { CSS::LineStyle::Solid, 6 }, + { CSS::LineStyle::Double, 7 }, + }; + if (a.line_style == CSS::LineStyle::Hidden) { + return a; + } + if (b.line_style == CSS::LineStyle::Hidden) { + return b; + } + if (a.line_style == CSS::LineStyle::None) { + return b; + } + if (b.line_style == CSS::LineStyle::None) { + return a; + } + if (a.width > b.width) { + return a; + } else if (a.width < b.width) { + return b; + } + if (*line_style_score.get(a.line_style) > *line_style_score.get(b.line_style)) { + return a; + } else if (*line_style_score.get(a.line_style) < *line_style_score.get(b.line_style)) { + return b; + } + return a; +} + +void TableFormattingContext::border_conflict_resolution() +{ + // Partially implements border conflict resolution, as described in + // https://www.w3.org/TR/CSS22/tables.html#border-conflict-resolution + BorderConflictFinder finder(this); + for (auto& cell : m_cells) { + if (cell.box->computed_values().border_collapse() == CSS::BorderCollapse::Separate) { + continue; + } + // Execute steps 1, 2 and 3 of the algorithm for each edge. + Painting::BordersData override_borders_data; + auto const& cell_style = cell.box->computed_values(); + auto& cell_state = m_state.get_mutable(cell.box); + for (auto const conflicting_element : finder.conflicting_elements(cell, ConflictingEdge::Left)) { + auto const& other_style = conflicting_element->computed_values(); + override_borders_data.left = winning_border_style(cell_style.border_left(), other_style.border_left()); + cell_state.border_left = override_borders_data.left.width; + } + for (auto const conflicting_element : finder.conflicting_elements(cell, ConflictingEdge::Right)) { + auto const& other_style = conflicting_element->computed_values(); + override_borders_data.right = winning_border_style(cell_style.border_right(), other_style.border_right()); + cell_state.border_right = override_borders_data.right.width; + } + for (auto const conflicting_element : finder.conflicting_elements(cell, ConflictingEdge::Top)) { + auto const& other_style = conflicting_element->computed_values(); + override_borders_data.top = winning_border_style(cell_style.border_top(), other_style.border_top()); + cell_state.border_top = override_borders_data.top.width; + } + for (auto const conflicting_element : finder.conflicting_elements(cell, ConflictingEdge::Bottom)) { + auto const& other_style = conflicting_element->computed_values(); + override_borders_data.bottom = winning_border_style(cell_style.border_bottom(), other_style.border_bottom()); + cell_state.border_bottom = override_borders_data.bottom.width; + } + // FIXME: 4. If border styles differ only in color, then a style set on a cell wins over one on a row, which wins over a + // row group, column, column group and, lastly, table. When two elements of the same type conflict, then the one + // further to the left (if the table's 'direction' is 'ltr'; right, if it is 'rtl') and further to the top wins. + cell_state.set_override_borders_data(override_borders_data); + } +} + CSSPixels TableFormattingContext::compute_row_content_height(Cell const& cell) const { auto& row_state = m_state.get(m_rows[cell.row_index].box); @@ -771,6 +845,51 @@ CSSPixels TableFormattingContext::compute_row_content_height(Cell const& cell) c return span_height; } +TableFormattingContext::BorderConflictFinder::BorderConflictFinder(TableFormattingContext const* context) + : m_context(context) +{ + collect_conflicting_col_elements(); +} + +void TableFormattingContext::BorderConflictFinder::collect_conflicting_col_elements() +{ + m_col_elements_by_index.resize(m_context->m_columns.size()); + for (auto* child = m_context->table_box().first_child(); child; child = child->next_sibling()) { + if (!child->display().is_table_column_group()) { + continue; + } + size_t column_index = 0; + for (auto* child_of_column_group = child->first_child(); child_of_column_group; child_of_column_group = child_of_column_group->next_sibling()) { + VERIFY(child_of_column_group->display().is_table_column()); + auto const& col_node = static_cast(*child_of_column_group->dom_node()); + unsigned span = col_node.attribute(HTML::AttributeNames::span).to_uint().value_or(1); + for (size_t i = column_index; i < column_index + span; ++i) { + m_col_elements_by_index[i] = child_of_column_group; + } + column_index += span; + } + } +} + +Vector TableFormattingContext::BorderConflictFinder::conflicting_elements(Cell const& cell, TableFormattingContext::ConflictingEdge edge) const +{ + // FIXME: Conflicting elements can be cells, rows, row groups, columns, column groups, and the table itself, + // but we only consider 'col' elements in a 'colgroup', the table and the cell itself for now. + Vector result = { cell.box }; + auto is_top = cell.row_index == 0; + auto is_left = cell.column_index == 0; + auto is_right = cell.column_index == m_context->m_columns.size() - 1; + auto is_bottom = cell.row_index == m_context->m_rows.size() - 1; + auto conflicts_top_or_bottom = (is_top && edge == ConflictingEdge::Top) || (is_bottom && edge == ConflictingEdge::Bottom); + if (conflicts_top_or_bottom || (is_left && edge == ConflictingEdge::Left) || (is_right && edge == ConflictingEdge::Right)) { + result.append(&m_context->table_box()); + } + if (m_col_elements_by_index[cell.column_index] && (conflicts_top_or_bottom || edge == ConflictingEdge::Left || edge == ConflictingEdge::Right)) { + result.append(m_col_elements_by_index[cell.column_index]); + } + return result; +} + void TableFormattingContext::run(Box const& box, LayoutMode layout_mode, AvailableSpace const& available_space) { m_available_space = available_space; @@ -782,6 +901,8 @@ void TableFormattingContext::run(Box const& box, LayoutMode layout_mode, Availab // Determine the number of rows/columns the table requires. calculate_row_column_grid(box); + border_conflict_resolution(); + // Compute the minimum width of each column. compute_table_measures(); diff --git a/Userland/Libraries/LibWeb/Layout/TableFormattingContext.h b/Userland/Libraries/LibWeb/Layout/TableFormattingContext.h index c58f22d36c..8866fe06b4 100644 --- a/Userland/Libraries/LibWeb/Layout/TableFormattingContext.h +++ b/Userland/Libraries/LibWeb/Layout/TableFormattingContext.h @@ -39,6 +39,7 @@ private: void distribute_height_to_rows(); void position_row_boxes(CSSPixels&); void position_cell_boxes(); + void border_conflict_resolution(); CSSPixels m_table_height { 0 }; CSSPixels m_automatic_content_height { 0 }; @@ -81,6 +82,25 @@ private: CSSPixels compute_row_content_height(Cell const& cell) const; + enum class ConflictingEdge { + Top, + Right, + Bottom, + Left, + }; + + class BorderConflictFinder { + public: + BorderConflictFinder(TableFormattingContext const* context); + Vector conflicting_elements(Cell const&, ConflictingEdge) const; + + private: + void collect_conflicting_col_elements(); + + Vector m_col_elements_by_index; + TableFormattingContext const* m_context; + }; + Vector m_cells; Vector m_columns; Vector m_rows; diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index 57a1cb0800..bbdf0741cc 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -222,7 +222,7 @@ void PaintableBox::paint(PaintContext& context, PaintPhase phase) const void PaintableBox::paint_border(PaintContext& context) const { - auto borders_data = BordersData { + auto borders_data = m_override_borders_data.has_value() ? m_override_borders_data.value() : BordersData { .top = box_model().border.top == 0 ? CSS::BorderData() : computed_values().border_top(), .right = box_model().border.right == 0 ? CSS::BorderData() : computed_values().border_right(), .bottom = box_model().border.bottom == 0 ? CSS::BorderData() : computed_values().border_bottom(), diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.h b/Userland/Libraries/LibWeb/Painting/PaintableBox.h index 21af78b392..8d79c3d956 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.h +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.h @@ -123,6 +123,8 @@ public: bool is_out_of_view(PaintContext&) const; + void set_override_borders_data(BordersData const& override_borders_data) { m_override_borders_data = override_borders_data; }; + protected: explicit PaintableBox(Layout::Box const&); @@ -161,6 +163,8 @@ private: mutable bool m_clipping_overflow { false }; Optional mutable m_overflow_corner_radius_clipper; + + Optional m_override_borders_data; }; class PaintableWithLines final : public PaintableBox {