Attachment 358787 Details for Bug 193313 – Patch

[patch] Patch

bug-193313-20190110145611.patch (text/plain), 13.32 KB, created by Oriol Brufau on 2019-01-10 05:56:12 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Oriol Brufau

Created: 2019-01-10 05:56:12 PST

Size: 13.32 KB

patch

obsolete

>Subversion Revision: 239605
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a3920592b9bfc7162155886a8cb9413b43718659..5ab65a48cf8dd7d3f579ff0324567acb1616bb80 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2019-01-10  Oriol Brufau  <obrufau@igalia.com>
>+
>+        [css-grid] Let abspos items reference implicit grid lines
>+        https://bugs.webkit.org/show_bug.cgi?id=193313
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        While they can't create new implicit grid lines, abspos items
>+        can reference existing ones as clarified in
>+        https://github.com/w3c/csswg-drafts/commit/511bb63
>+
>+        This patch makes WebKit match Blink, Firefox and Edge.
>+
>+        Tests: web-platform-tests/css/css-grid/abspos/grid-positioned-items-padding-001.html
>+               web-platform-tests/css/css-grid/abspos/grid-positioned-items-unknown-named-grid-line-001.html
>+
>+        * rendering/RenderGrid.cpp:
>+        (WebCore::RenderGrid::gridAreaBreadthForOutOfFlowChild):
>+        Don't treat implicit grid lines as 'auto'.
>+        (WebCore::RenderGrid::gridPositionIsAutoForOutOfFlow const): Deleted.
>+        * rendering/RenderGrid.h:
>+        Delete gridPositionIsAutoForOutOfFlow.
>+        * rendering/style/GridPositionsResolver.cpp:
>+        (WebCore::adjustGridPositionsFromStyle):
>+        Don't treat implicit grid lines as 'auto'.
>+        (WebCore::resolveGridPositionFromStyle):
>+        Remove unnecessary assert that uses isValidNamedLineOrArea.
>+        (WebCore::NamedLineCollection::isValidNamedLineOrArea): Deleted.
>+        * rendering/style/GridPositionsResolver.h:
>+        Delete isValidNamedLineOrArea.
>+
> 2019-01-03  Zalan Bujtas  <zalan@apple.com>
> 
>         REGRESSION: -webkit-appearance test case crashes
>diff --git a/Source/WebCore/rendering/RenderGrid.cpp b/Source/WebCore/rendering/RenderGrid.cpp
>index 35a610b29973ccd9e68c654c8a45bed0cc3a5a6f..bf5fb90de0bc24cfb232a615c03f5b00feeba06b 100644
>--- a/Source/WebCore/rendering/RenderGrid.cpp
>+++ b/Source/WebCore/rendering/RenderGrid.cpp
>@@ -1470,11 +1470,6 @@ LayoutUnit RenderGrid::rowAxisOffsetForChild(const RenderBox& child) const
>     return 0;
> }
> 
>-bool RenderGrid::gridPositionIsAutoForOutOfFlow(GridPosition position, GridTrackSizingDirection direction) const
>-{
>-    return position.isAuto() || (position.isNamedGridArea() && !NamedLineCollection::isValidNamedLineOrArea(position.namedGridLine(), style(), GridPositionsResolver::initialPositionSide(direction)));
>-}
>-
> LayoutUnit RenderGrid::resolveAutoStartGridPosition(GridTrackSizingDirection direction) const
> {
>     if (direction == ForRows || style().isLeftToRightDirection())
>@@ -1520,8 +1515,8 @@ LayoutUnit RenderGrid::gridAreaBreadthForOutOfFlowChild(const RenderBox& child,
>     GridPosition startPosition = direction == ForColumns ? child.style().gridItemColumnStart() : child.style().gridItemRowStart();
>     GridPosition endPosition = direction == ForColumns ? child.style().gridItemColumnEnd() : child.style().gridItemRowEnd();
> 
>-    bool startIsAuto = gridPositionIsAutoForOutOfFlow(startPosition, direction) || startLine < 0 || startLine > lastLine;
>-    bool endIsAuto = gridPositionIsAutoForOutOfFlow(endPosition, direction) || endLine < 0 || endLine > lastLine;
>+    bool startIsAuto = startPosition.isAuto() || startLine < 0 || startLine > lastLine;
>+    bool endIsAuto = endPosition.isAuto() || endLine < 0 || endLine > lastLine;
> 
>     if (startIsAuto && endIsAuto)
>         return isRowAxis ? clientLogicalWidth() : clientLogicalHeight();
>diff --git a/Source/WebCore/rendering/RenderGrid.h b/Source/WebCore/rendering/RenderGrid.h
>index 131a00841aceb243bd50fedade4a5077955ce35f..de3c23483a0cc1f076f4e1bd4fc926175ccbfb7e 100644
>--- a/Source/WebCore/rendering/RenderGrid.h
>+++ b/Source/WebCore/rendering/RenderGrid.h
>@@ -133,7 +133,6 @@ private:
>     void layoutGridItems();
>     void populateGridPositionsForDirection(GridTrackSizingDirection);
> 
>-    bool gridPositionIsAutoForOutOfFlow(GridPosition, GridTrackSizingDirection) const;
>     LayoutUnit resolveAutoStartGridPosition(GridTrackSizingDirection) const;
>     LayoutUnit resolveAutoEndGridPosition(GridTrackSizingDirection) const;
>     LayoutUnit gridAreaBreadthForOutOfFlowChild(const RenderBox&, GridTrackSizingDirection);
>diff --git a/Source/WebCore/rendering/style/GridPositionsResolver.cpp b/Source/WebCore/rendering/style/GridPositionsResolver.cpp
>index 24c9d3801e0e8ce8a0c969ed5b254c577a809257..a3514485165e3260eb939f52082dfaaf14741017 100644
>--- a/Source/WebCore/rendering/style/GridPositionsResolver.cpp
>+++ b/Source/WebCore/rendering/style/GridPositionsResolver.cpp
>@@ -76,19 +76,6 @@ NamedLineCollection::NamedLineCollection(const RenderStyle& gridContainerStyle,
>     m_autoRepeatTrackListLength = isRowAxis ? gridContainerStyle.gridAutoRepeatColumns().size() : gridContainerStyle.gridAutoRepeatRows().size();
> }
> 
>-bool NamedLineCollection::isValidNamedLineOrArea(const String& namedLine, const RenderStyle& gridContainerStyle, GridPositionSide side)
>-{
>-    bool isRowAxis = directionFromSide(side) == ForColumns;
>-    auto& gridLineNames = isRowAxis ? gridContainerStyle.namedGridColumnLines() : gridContainerStyle.namedGridRowLines();
>-    auto& autoRepeatGridLineNames = isRowAxis ? gridContainerStyle.autoRepeatNamedGridColumnLines() : gridContainerStyle.autoRepeatNamedGridRowLines();
>-
>-    if (gridLineNames.contains(namedLine) || autoRepeatGridLineNames.contains(namedLine))
>-        return true;
>-
>-    String implicitName = implicitNamedGridLineForSide(namedLine, side);
>-    return gridLineNames.contains(implicitName) || autoRepeatGridLineNames.contains(implicitName);
>-}
>-
> bool NamedLineCollection::hasNamedLines() const
> {
>     return m_namedLinesIndexes || m_autoRepeatNamedLinesIndexes;
>@@ -159,15 +146,6 @@ static void adjustGridPositionsFromStyle(const RenderStyle& gridContainerStyle,
>     if (initialPosition.isSpan() && finalPosition.isSpan())
>         finalPosition.setAutoPosition();
> 
>-    if (gridItem.isOutOfFlowPositioned()) {
>-        // Early detect the case of non existing named grid lines for positioned items.
>-        if (initialPosition.isNamedGridArea() && !NamedLineCollection::isValidNamedLineOrArea(initialPosition.namedGridLine(), gridContainerStyle, GridPositionsResolver::initialPositionSide(direction)))
>-            initialPosition.setAutoPosition();
>-
>-        if (finalPosition.isNamedGridArea() && !NamedLineCollection::isValidNamedLineOrArea(finalPosition.namedGridLine(), gridContainerStyle, GridPositionsResolver::finalPositionSide(direction)))
>-            finalPosition.setAutoPosition();
>-    }
>-
>     // If the grid item has an automatic position and a grid span for a named line in a given dimension, instead treat the grid span as one.
>     if (initialPosition.isAuto() && finalPosition.isSpan() && !finalPosition.namedGridLine().isNull())
>         finalPosition.setSpanPosition(1, String());
>@@ -357,7 +335,6 @@ static int resolveGridPositionFromStyle(const RenderStyle& gridContainerStyle, c
>         if (explicitLines.hasNamedLines())
>             return explicitLines.firstPosition();
> 
>-        ASSERT(!NamedLineCollection::isValidNamedLineOrArea(namedGridLine, gridContainerStyle, side));
>         // If none of the above works specs mandate to assume that all the lines in the implicit grid have this name.
>         return lastLine + 1;
>     }
>diff --git a/Source/WebCore/rendering/style/GridPositionsResolver.h b/Source/WebCore/rendering/style/GridPositionsResolver.h
>index fb3311a85b36e7ff01149e441f00da3a58bf9d17..3a36195b9953f73f01c482681031e3b170e46207 100644
>--- a/Source/WebCore/rendering/style/GridPositionsResolver.h
>+++ b/Source/WebCore/rendering/style/GridPositionsResolver.h
>@@ -48,8 +48,6 @@ class NamedLineCollection {
> public:
>     NamedLineCollection(const RenderStyle&, const String& namedLine, GridTrackSizingDirection, unsigned lastLine, unsigned autoRepeatTracksCount);
> 
>-    static bool isValidNamedLineOrArea(const String& namedLine, const RenderStyle&, GridPositionSide);
>-
>     bool hasNamedLines() const;
>     unsigned firstPosition() const;
> 
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 6a43ef63a38c01ff956aa50713ad65484e05eff5..1aa2de057c9b6a768ac91b0c2ba979a524c73a45 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-01-10  Oriol Brufau  <obrufau@igalia.com>
>+
>+        [css-grid] Let abspos items reference implicit grid lines
>+        https://bugs.webkit.org/show_bug.cgi?id=193313
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Import test changes from WPT.
>+
>+        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-padding-001.html:
>+        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-unknown-named-grid-line-001.html:
>+
> 2019-01-02  Simon Fraser  <simon.fraser@apple.com>
> 
>         Support css-color-4 rgb functions
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-padding-001.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-padding-001.html
>index 4941d77a71af73c5eb2c70a14629596aacdcf767..643a68830d4aecdad182f40d8b74fb8470da8b54 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-padding-001.html
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-padding-001.html
>@@ -109,13 +109,13 @@
> 
> <div class="grid">
>   <div class="absolute" style="grid-column: foo / 1; grid-row: foo / 1;"
>-    data-offset-x="0" data-offset-y="0" data-expected-width="15" data-expected-height="15">
>+    data-offset-x="15" data-offset-y="15" data-expected-width="515" data-expected-height="315">
>   </div>
>   <div class="absolute" style="grid-column: 1 / 2; grid-row: foo / 1;"
>-    data-offset-x="15" data-offset-y="0" data-expected-width="100" data-expected-height="15">
>+    data-offset-x="15" data-offset-y="15" data-expected-width="100" data-expected-height="315">
>   </div>
>   <div class="absolute" style="grid-column: foo / 1; grid-row: 1 / 2;"
>-    data-offset-x="0" data-offset-y="15" data-expected-width="15" data-expected-height="50">
>+    data-offset-x="15" data-offset-y="15" data-expected-width="515" data-expected-height="50">
>   </div>
>   <div class="absolute" style="grid-column: 3 / foo; grid-row: 3 / foo;"
>     data-offset-x="315" data-offset-y="215" data-expected-width="215" data-expected-height="115">
>@@ -193,13 +193,13 @@
> 
> <div class="grid directionRTL">
>   <div class="absolute" style="grid-column: foo / 1; grid-row: foo / 1;"
>-    data-offset-x="515" data-offset-y="0" data-expected-width="15" data-expected-height="15">
>+    data-offset-x="0" data-offset-y="15" data-expected-width="515" data-expected-height="315">
>   </div>
>   <div class="absolute" style="grid-column: 1 / 2; grid-row: foo / 1;"
>-    data-offset-x="415" data-offset-y="0" data-expected-width="100" data-expected-height="15">
>+    data-offset-x="415" data-offset-y="15" data-expected-width="100" data-expected-height="315">
>   </div>
>   <div class="absolute" style="grid-column: foo / 1; grid-row: 1 / 2;"
>-    data-offset-x="515" data-offset-y="15" data-expected-width="15" data-expected-height="50">
>+    data-offset-x="0" data-offset-y="15" data-expected-width="515" data-expected-height="50">
>   </div>
>   <div class="absolute" style="grid-column: 3 / foo; grid-row: 3 / foo;"
>     data-offset-x="0" data-offset-y="215" data-expected-width="215" data-expected-height="115">
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-unknown-named-grid-line-001.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-unknown-named-grid-line-001.html
>index 24d9d769f99079d19519d05f82e8fd637906fe8a..6e61f7c2de36a1d799aba0de6d10c52147d406d6 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-unknown-named-grid-line-001.html
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-unknown-named-grid-line-001.html
>@@ -3,7 +3,7 @@
> <title>CSS Grid Layout Test: Grid positioned items unknown named grid line</title>
> <link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
> <link rel="help" href="https://drafts.csswg.org/css-grid-1/#abspos" title="9. Absolute Positioning">
>-<meta name="assert" content="This test checks that grid placement properties of absolutely positioned items using unknown named grid lines are treated as 'auto'.">
>+<meta name="assert" content="This test checks that grid placement properties of absolutely positioned items can reference implicit grid lines.">
> <link rel="stylesheet" href="support/grid.css">
> <style>
> 
>@@ -58,7 +58,7 @@
>     data-offset-x="15" data-offset-y="15" data-expected-width="515" data-expected-height="315">
>   </div>
>   <div class="absolute sizedToGridArea startUnknownLine"
>-    data-offset-x="0" data-offset-y="0" data-expected-width="315" data-expected-height="215">
>+    data-offset-x="315" data-offset-y="215" data-expected-width="215" data-expected-height="115">
>   </div>
> </div>
> 
>@@ -70,7 +70,7 @@
>     data-offset-x="0" data-offset-y="15" data-expected-width="515" data-expected-height="315">
>   </div>
>   <div class="absolute sizedToGridArea startUnknownLine"
>-    data-offset-x="215" data-offset-y="0" data-expected-width="315" data-expected-height="215">
>+    data-offset-x="0" data-offset-y="215" data-expected-width="215" data-expected-height="115">
>   </div>
> </div>
>

Actions: View | Formatted Diff | Diff

Attachments on bug 193313: 358787 | 358792