From 236795e931f31cbd49219bd0f2ce0ea1932a4468 Mon Sep 17 00:00:00 2001 From: martinfalisse Date: Sat, 17 Sep 2022 18:09:35 +0200 Subject: [PATCH] LibWeb+Base: Re-implement grid track span Implement span correctly when indicated in the grid-column-start, grid-row-start, etc. CSS properties. Previously it had been implemented as if span was something that went alongside the position property, but actually it seems like if you do 'span 3' in the grid-column-start property, for example, this means it literally spans 3 blocks, and the 3 has nothing to do with position. --- Base/res/html/misc/display-grid.html | 40 +-- .../LibWeb/CSS/GridTrackPlacement.cpp | 17 +- .../Libraries/LibWeb/CSS/GridTrackPlacement.h | 36 ++- .../Libraries/LibWeb/CSS/Parser/Parser.cpp | 19 +- .../LibWeb/Layout/GridFormattingContext.cpp | 229 ++++++++++++++---- 5 files changed, 224 insertions(+), 117 deletions(-) diff --git a/Base/res/html/misc/display-grid.html b/Base/res/html/misc/display-grid.html index 16cea2158b..812d6c9b94 100644 --- a/Base/res/html/misc/display-grid.html +++ b/Base/res/html/misc/display-grid.html @@ -40,26 +40,6 @@
1
- -

Should render a full-width 4x4 grid with: -

-
-
1
-
2
-
3
-
4
-
5
-
6
-
-

Should render a 2x2 grid with columns 50px and 50%

4
+ + +

Should render a full-width 4x4 grid with: +

+
+
1
+
2
+
3
+
4
+
5
+
6
+
diff --git a/Userland/Libraries/LibWeb/CSS/GridTrackPlacement.cpp b/Userland/Libraries/LibWeb/CSS/GridTrackPlacement.cpp index c6a1de5f23..2e7d1e5538 100644 --- a/Userland/Libraries/LibWeb/CSS/GridTrackPlacement.cpp +++ b/Userland/Libraries/LibWeb/CSS/GridTrackPlacement.cpp @@ -9,28 +9,23 @@ namespace Web::CSS { -GridTrackPlacement::GridTrackPlacement(int position, bool has_span) - : m_position(position) - , m_has_span(has_span) -{ -} - -GridTrackPlacement::GridTrackPlacement(int position) - : m_position(position) +GridTrackPlacement::GridTrackPlacement(int span_or_position, bool has_span) + : m_type(has_span ? Type::Span : Type::Position) + , m_value(span_or_position) { } GridTrackPlacement::GridTrackPlacement() - : m_is_auto(true) + : m_type(Type::Auto) { } String GridTrackPlacement::to_string() const { StringBuilder builder; - if (m_has_span) + if (is_span()) builder.append("span "sv); - builder.append(String::number(m_position)); + builder.append(String::number(m_value)); return builder.to_string(); } diff --git a/Userland/Libraries/LibWeb/CSS/GridTrackPlacement.h b/Userland/Libraries/LibWeb/CSS/GridTrackPlacement.h index 9c5090fbe0..32483641b7 100644 --- a/Userland/Libraries/LibWeb/CSS/GridTrackPlacement.h +++ b/Userland/Libraries/LibWeb/CSS/GridTrackPlacement.h @@ -12,38 +12,34 @@ namespace Web::CSS { class GridTrackPlacement { public: - GridTrackPlacement(int, bool); - GridTrackPlacement(int); + enum class Type { + Span, + Position, + Auto + }; + + GridTrackPlacement(int, bool = false); GridTrackPlacement(); static GridTrackPlacement make_auto() { return GridTrackPlacement(); }; - void set_position(int position) - { - m_is_auto = false; - m_position = position; - } - int position() const { return m_position; } + bool is_span() const { return m_type == Type::Span; } + bool is_position() const { return m_type == Type::Position; } + bool is_auto() const { return m_type == Type::Auto; } + bool is_auto_positioned() const { return m_type == Type::Auto || m_type == Type::Span; } - void set_has_span(bool has_span) - { - VERIFY(!m_is_auto); - m_has_span = has_span; - } - bool has_span() const { return m_has_span; } - - bool is_auto() const { return m_is_auto; } + int raw_value() const { return m_value; } + Type type() const { return m_type; } String to_string() const; bool operator==(GridTrackPlacement const& other) const { - return m_position == other.position() && m_has_span == other.has_span(); + return m_type == other.type() && m_value == other.raw_value(); } private: - bool m_is_auto { false }; - int m_position { 0 }; - bool m_has_span { false }; + Type m_type; + int m_value { 0 }; }; } diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index 40b2d7180d..4be1a50f67 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -5457,20 +5457,14 @@ RefPtr Parser::parse_grid_track_placement(Vector con return {}; } - auto first_grid_track_placement = CSS::GridTrackPlacement(); auto has_span = false; if (current_token.to_string() == "span"sv) { has_span = true; tokens.skip_whitespace(); current_token = tokens.next_token().token(); } - if (current_token.is(Token::Type::Number) && current_token.number().is_integer()) { - first_grid_track_placement.set_position(static_cast(current_token.number_value())); - first_grid_track_placement.set_has_span(has_span); - } - - if (!tokens.has_next_token()) - return GridTrackPlacementStyleValue::create(first_grid_track_placement); + if (current_token.is(Token::Type::Number) && current_token.number().is_integer() && !tokens.has_next_token()) + return GridTrackPlacementStyleValue::create(CSS::GridTrackPlacement(static_cast(current_token.number_value()), has_span)); return {}; } @@ -5488,18 +5482,15 @@ RefPtr Parser::parse_grid_track_placement_shorthand_value(Vector CSS::GridTrackPlacement { - auto grid_track_placement = CSS::GridTrackPlacement(); auto has_span = false; if (current_token.to_string() == "span"sv) { has_span = true; tokens.skip_whitespace(); current_token = tokens.next_token().token(); } - if (current_token.is(Token::Type::Number) && current_token.number().is_integer()) { - grid_track_placement.set_position(static_cast(current_token.number_value())); - grid_track_placement.set_has_span(has_span); - } - return grid_track_placement; + if (current_token.is(Token::Type::Number) && current_token.number().is_integer()) + return CSS::GridTrackPlacement(static_cast(current_token.number_value()), has_span); + return CSS::GridTrackPlacement(); }; auto first_grid_track_placement = calculate_grid_track_placement(current_token, tokens); diff --git a/Userland/Libraries/LibWeb/Layout/GridFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/GridFormattingContext.cpp index 8bf6a686eb..dbe4f490af 100644 --- a/Userland/Libraries/LibWeb/Layout/GridFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/GridFormattingContext.cpp @@ -107,18 +107,29 @@ void GridFormattingContext::run(Box const& box, LayoutMode, AvailableSpace const // 1. Position anything that's not auto-positioned. for (size_t i = 0; i < boxes_to_place.size(); i++) { auto const& child_box = boxes_to_place[i]; - if (child_box.computed_values().grid_row_start().is_auto() - || child_box.computed_values().grid_row_end().is_auto() - || child_box.computed_values().grid_column_start().is_auto() - || child_box.computed_values().grid_column_end().is_auto()) + if ((child_box.computed_values().grid_row_start().is_auto_positioned() && child_box.computed_values().grid_row_end().is_auto_positioned()) + || (child_box.computed_values().grid_column_start().is_auto_positioned() && child_box.computed_values().grid_column_end().is_auto_positioned())) continue; - int row_start = child_box.computed_values().grid_row_start().position(); - int row_end = child_box.computed_values().grid_row_end().position(); - int column_start = child_box.computed_values().grid_column_start().position(); - int column_end = child_box.computed_values().grid_column_end().position(); - int row_span = 1; - int column_span = 1; + int row_start = child_box.computed_values().grid_row_start().raw_value(); + int row_end = child_box.computed_values().grid_row_end().raw_value(); + int column_start = child_box.computed_values().grid_column_start().raw_value(); + int column_end = child_box.computed_values().grid_column_end().raw_value(); + + // https://drafts.csswg.org/css-grid/#line-placement + // 8.3. Line-based Placement: the grid-row-start, grid-column-start, grid-row-end, and grid-column-end properties + + // https://drafts.csswg.org/css-grid/#grid-placement-slot + // FIXME: + // First attempt to match the grid area’s edge to a named grid area: if there is a grid line whose + // line name is -start (for grid-*-start) / -end (for grid-*-end), + // contributes the first such line to the grid item’s placement. + + // Note: Named grid areas automatically generate implicitly-assigned line names of this form, so + // specifying grid-row-start: foo will choose the start edge of that named grid area (unless another + // line named foo-start was explicitly specified before it). + + // Otherwise, treat this as if the integer 1 had been specified along with the . // https://drafts.csswg.org/css-grid/#grid-placement-int // [ | ] && ? @@ -128,33 +139,68 @@ void GridFormattingContext::run(Box const& box, LayoutMode, AvailableSpace const row_end = static_cast(occupation_grid.size()) + row_end + 2; if (column_end < 0) column_end = static_cast(occupation_grid[0].size()) + column_end + 2; - // FIXME: If a name is given as a , only lines with that name are counted. If not enough + + // If a name is given as a , only lines with that name are counted. If not enough // lines with that name exist, all implicit grid lines are assumed to have that name for the purpose // of finding this position. - // FIXME: An value of zero makes the declaration invalid. + // An value of zero makes the declaration invalid. + + // https://drafts.csswg.org/css-grid/#grid-placement-span-int + // span && [ || ] + // Contributes a grid span to the grid item’s placement such that the corresponding edge of the grid + // item’s grid area is N lines from its opposite edge in the corresponding direction. For example, + // grid-column-end: span 2 indicates the second grid line in the endward direction from the + // grid-column-start line. + int row_span = 1; + int column_span = 1; + if (child_box.computed_values().grid_row_start().is_position() && child_box.computed_values().grid_row_end().is_span()) + row_span = child_box.computed_values().grid_row_end().raw_value(); + if (child_box.computed_values().grid_column_start().is_position() && child_box.computed_values().grid_column_end().is_span()) + column_span = child_box.computed_values().grid_column_end().raw_value(); + if (child_box.computed_values().grid_row_end().is_position() && child_box.computed_values().grid_row_start().is_span()) { + row_span = child_box.computed_values().grid_row_start().raw_value(); + row_start = row_end - row_span; + } + if (child_box.computed_values().grid_column_end().is_position() && child_box.computed_values().grid_column_start().is_span()) { + column_span = child_box.computed_values().grid_column_start().raw_value(); + column_start = column_end - column_span; + } + + // If a name is given as a , only lines with that name are counted. If not enough + // lines with that name exist, all implicit grid lines on the side of the explicit grid + // corresponding to the search direction are assumed to have that name for the purpose of counting + // this span. + + // https://drafts.csswg.org/css-grid/#grid-placement-auto + // auto + // The property contributes nothing to the grid item’s placement, indicating auto-placement or a + // default span of one. (See § 8 Placing Grid Items, above.) // https://drafts.csswg.org/css-grid/#grid-placement-errors // 8.3.1. Grid Placement Conflict Handling // If the placement for a grid item contains two lines, and the start line is further end-ward than // the end line, swap the two lines. If the start line is equal to the end line, remove the end // line. - if (row_start > row_end) { - auto temp = row_end; - row_end = row_start; - row_start = temp; + if (child_box.computed_values().grid_row_start().is_position() && child_box.computed_values().grid_row_end().is_position()) { + if (row_start > row_end) + swap(row_start, row_end); + if (row_start != row_end) + row_span = row_end - row_start; } - if (column_start > column_end) { - auto temp = column_end; - column_end = column_start; - column_start = temp; + if (child_box.computed_values().grid_column_start().is_position() && child_box.computed_values().grid_column_end().is_position()) { + if (column_start > column_end) + swap(column_start, column_end); + if (column_start != column_end) + column_span = column_end - column_start; } - if (row_start != row_end) - row_span = row_end - row_start; - if (column_start != column_end) - column_span = column_end - column_start; - // FIXME: If the placement contains two spans, remove the one contributed by the end grid-placement + + // If the placement contains two spans, remove the one contributed by the end grid-placement // property. + if (child_box.computed_values().grid_row_start().is_span() && child_box.computed_values().grid_row_end().is_span()) + row_span = child_box.computed_values().grid_row_start().raw_value(); + if (child_box.computed_values().grid_column_start().is_span() && child_box.computed_values().grid_column_end().is_span()) + column_span = child_box.computed_values().grid_column_start().raw_value(); // FIXME: If the placement contains only a span for a named line, replace it with a span of 1. @@ -173,13 +219,26 @@ void GridFormattingContext::run(Box const& box, LayoutMode, AvailableSpace const // FIXME: Do "dense" packing for (size_t i = 0; i < boxes_to_place.size(); i++) { auto const& child_box = boxes_to_place[i]; - if (child_box.computed_values().grid_row_start().is_auto() - || child_box.computed_values().grid_row_end().is_auto()) + if (child_box.computed_values().grid_row_start().is_auto_positioned() && child_box.computed_values().grid_row_end().is_auto_positioned()) continue; - int row_start = child_box.computed_values().grid_row_start().position(); - int row_end = child_box.computed_values().grid_row_end().position(); - int row_span = 1; + int row_start = child_box.computed_values().grid_row_start().raw_value(); + int row_end = child_box.computed_values().grid_row_end().raw_value(); + + // https://drafts.csswg.org/css-grid/#line-placement + // 8.3. Line-based Placement: the grid-row-start, grid-column-start, grid-row-end, and grid-column-end properties + + // https://drafts.csswg.org/css-grid/#grid-placement-slot + // FIXME: + // First attempt to match the grid area’s edge to a named grid area: if there is a grid line whose + // line name is -start (for grid-*-start) / -end (for grid-*-end), + // contributes the first such line to the grid item’s placement. + + // Note: Named grid areas automatically generate implicitly-assigned line names of this form, so + // specifying grid-row-start: foo will choose the start edge of that named grid area (unless another + // line named foo-start was explicitly specified before it). + + // Otherwise, treat this as if the integer 1 had been specified along with the . // https://drafts.csswg.org/css-grid/#grid-placement-int // [ | ] && ? @@ -187,26 +246,53 @@ void GridFormattingContext::run(Box const& box, LayoutMode, AvailableSpace const // instead counts in reverse, starting from the end edge of the explicit grid. if (row_end < 0) row_end = static_cast(occupation_grid.size()) + row_end + 2; - // FIXME: If a name is given as a , only lines with that name are counted. If not enough + + // If a name is given as a , only lines with that name are counted. If not enough // lines with that name exist, all implicit grid lines are assumed to have that name for the purpose // of finding this position. - // FIXME: An value of zero makes the declaration invalid. + // An value of zero makes the declaration invalid. + + // https://drafts.csswg.org/css-grid/#grid-placement-span-int + // span && [ || ] + // Contributes a grid span to the grid item’s placement such that the corresponding edge of the grid + // item’s grid area is N lines from its opposite edge in the corresponding direction. For example, + // grid-column-end: span 2 indicates the second grid line in the endward direction from the + // grid-column-start line. + int row_span = 1; + if (child_box.computed_values().grid_row_start().is_position() && child_box.computed_values().grid_row_end().is_span()) + row_span = child_box.computed_values().grid_row_end().raw_value(); + if (child_box.computed_values().grid_row_end().is_position() && child_box.computed_values().grid_row_start().is_span()) { + row_span = child_box.computed_values().grid_row_start().raw_value(); + row_start = row_end - row_span; + } + + // If a name is given as a , only lines with that name are counted. If not enough + // lines with that name exist, all implicit grid lines on the side of the explicit grid + // corresponding to the search direction are assumed to have that name for the purpose of counting + // this span. + + // https://drafts.csswg.org/css-grid/#grid-placement-auto + // auto + // The property contributes nothing to the grid item’s placement, indicating auto-placement or a + // default span of one. (See § 8 Placing Grid Items, above.) // https://drafts.csswg.org/css-grid/#grid-placement-errors // 8.3.1. Grid Placement Conflict Handling // If the placement for a grid item contains two lines, and the start line is further end-ward than // the end line, swap the two lines. If the start line is equal to the end line, remove the end // line. - if (row_start > row_end) { - auto temp = row_end; - row_end = row_start; - row_start = temp; + if (child_box.computed_values().grid_row_start().is_position() && child_box.computed_values().grid_row_end().is_position()) { + if (row_start > row_end) + swap(row_start, row_end); + if (row_start != row_end) + row_span = row_end - row_start; } - if (row_start != row_end) - row_span = row_end - row_start; - // FIXME: If the placement contains two spans, remove the one contributed by the end grid-placement + + // If the placement contains two spans, remove the one contributed by the end grid-placement // property. + if (child_box.computed_values().grid_row_start().is_span() && child_box.computed_values().grid_row_end().is_span()) + row_span = child_box.computed_values().grid_row_start().raw_value(); // FIXME: If the placement contains only a span for a named line, replace it with a span of 1. @@ -263,10 +349,24 @@ void GridFormattingContext::run(Box const& box, LayoutMode, AvailableSpace const // FIXME: no distinction made. See #4.2 // 4.1.1. If the item has a definite column position: - if (!child_box.computed_values().grid_column_start().is_auto()) { - int column_start = child_box.computed_values().grid_column_start().position(); - int column_end = child_box.computed_values().grid_column_end().position(); - int column_span = 1; + if (!(child_box.computed_values().grid_column_start().is_auto_positioned() && child_box.computed_values().grid_column_end().is_auto_positioned())) { + int column_start = child_box.computed_values().grid_column_start().raw_value(); + int column_end = child_box.computed_values().grid_column_end().raw_value(); + + // https://drafts.csswg.org/css-grid/#line-placement + // 8.3. Line-based Placement: the grid-row-start, grid-column-start, grid-row-end, and grid-column-end properties + + // https://drafts.csswg.org/css-grid/#grid-placement-slot + // FIXME: + // First attempt to match the grid area’s edge to a named grid area: if there is a grid line whose + // line name is -start (for grid-*-start) / -end (for grid-*-end), + // contributes the first such line to the grid item’s placement. + + // Note: Named grid areas automatically generate implicitly-assigned line names of this form, so + // specifying grid-row-start: foo will choose the start edge of that named grid area (unless another + // line named foo-start was explicitly specified before it). + + // Otherwise, treat this as if the integer 1 had been specified along with the . // https://drafts.csswg.org/css-grid/#grid-placement-int // [ | ] && ? @@ -274,28 +374,53 @@ void GridFormattingContext::run(Box const& box, LayoutMode, AvailableSpace const // instead counts in reverse, starting from the end edge of the explicit grid. if (column_end < 0) column_end = static_cast(occupation_grid[0].size()) + column_end + 2; - // FIXME: If a name is given as a , only lines with that name are counted. If not enough + + // If a name is given as a , only lines with that name are counted. If not enough // lines with that name exist, all implicit grid lines are assumed to have that name for the purpose // of finding this position. - // FIXME: An value of zero makes the declaration invalid. + // An value of zero makes the declaration invalid. + + // https://drafts.csswg.org/css-grid/#grid-placement-span-int + // span && [ || ] + // Contributes a grid span to the grid item’s placement such that the corresponding edge of the grid + // item’s grid area is N lines from its opposite edge in the corresponding direction. For example, + // grid-column-end: span 2 indicates the second grid line in the endward direction from the + // grid-column-start line. + int column_span = 1; + if (child_box.computed_values().grid_column_start().is_position() && child_box.computed_values().grid_column_end().is_span()) + column_span = child_box.computed_values().grid_column_end().raw_value(); + if (child_box.computed_values().grid_column_end().is_position() && child_box.computed_values().grid_column_start().is_span()) { + column_span = child_box.computed_values().grid_column_start().raw_value(); + column_start = column_end - column_span; + } + + // If a name is given as a , only lines with that name are counted. If not enough + // lines with that name exist, all implicit grid lines on the side of the explicit grid + // corresponding to the search direction are assumed to have that name for the purpose of counting + // this span. + + // https://drafts.csswg.org/css-grid/#grid-placement-auto + // auto + // The property contributes nothing to the grid item’s placement, indicating auto-placement or a + // default span of one. (See § 8 Placing Grid Items, above.) // https://drafts.csswg.org/css-grid/#grid-placement-errors // 8.3.1. Grid Placement Conflict Handling // If the placement for a grid item contains two lines, and the start line is further end-ward than // the end line, swap the two lines. If the start line is equal to the end line, remove the end // line. - if (!child_box.computed_values().grid_column_end().is_auto()) { - if (column_start > column_end) { - auto temp = column_end; - column_end = column_start; - column_start = temp; - } + if (child_box.computed_values().grid_column_start().is_position() && child_box.computed_values().grid_column_end().is_position()) { + if (column_start > column_end) + swap(column_start, column_end); if (column_start != column_end) column_span = column_end - column_start; } - // FIXME: If the placement contains two spans, remove the one contributed by the end grid-placement + + // If the placement contains two spans, remove the one contributed by the end grid-placement // property. + if (child_box.computed_values().grid_column_start().is_span() && child_box.computed_values().grid_column_end().is_span()) + column_span = child_box.computed_values().grid_column_start().raw_value(); // FIXME: If the placement contains only a span for a named line, replace it with a span of 1.