[webkit-changes] [WebKit/WebKit] 5780a0: [css-grid] Use ShorthandSerializer for grid-row in...

Sammy Gill noreply at github.com
Tue Sep 19 09:33:14 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 5780a097f69233bdd55feb4cf5fc16acb8400364
      https://github.com/WebKit/WebKit/commit/5780a097f69233bdd55feb4cf5fc16acb8400364
  Author: Sammy Gill <sammy.gill at apple.com>
  Date:   2023-09-19 (Tue, 19 Sep 2023)

  Changed paths:
    M LayoutTests/fast/css-grid-layout/grid-item-column-row-get-set-expected.txt
    M LayoutTests/fast/css-grid-layout/grid-item-column-row-get-set.html
    M LayoutTests/fast/css-grid-layout/grid-item-end-after-get-set-expected.txt
    M LayoutTests/fast/css-grid-layout/grid-item-end-after-get-set.html
    M LayoutTests/fast/css-grid-layout/grid-item-start-before-get-set-expected.txt
    M LayoutTests/fast/css-grid-layout/grid-item-start-before-get-set.html
    M LayoutTests/fast/css-grid-layout/named-grid-lines-with-named-grid-areas-get-set-expected.txt
    M LayoutTests/fast/css-grid-layout/named-grid-lines-with-named-grid-areas-get-set.html
    M LayoutTests/fast/css-grid-layout/resources/grid-item-column-row-parsing-utils.js
    M LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-layout-properties-expected.txt
    A LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-invalid-expected.txt
    A LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-invalid.html
    A LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-shortest-serialization-expected.txt
    A LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-shortest-serialization.html
    A LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-shorthand-expected.txt
    A LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-shorthand.html
    M Source/WebCore/css/CSSComputedStyleDeclaration.cpp

  Log Message:
  -----------
  [css-grid] Use ShorthandSerializer for grid-row in CSSComputedStyleDeclaration::getPropertyValue.
https://bugs.webkit.org/show_bug.cgi?id=261576
rdar://115521465

Reviewed by Tim Nguyen.

We can reuse the logic that the ShorthandSerializer provides to
correctly serialize the computed style of this property. This will
help us avoid duplicating logic to get the computed value.

This change also exposed some incorrect tests within
fast/css-grid-layout. The underlying issue between all of the subtests
was the same: the tests did not correctly use the shortest possible
serialization for the grid-row property. Attempting to correct the
already existing subtests would have been costly as that would have
required rewritting some custom JS that we have for these specific
tests. Instead, I opted to remove the incorrect subtests and add WPT
versions of these same tests so that we do not actually lose the
coverage provided by them. This will make it much easier to maintain
these tests in the future as we can continue to leverage the WPT testing
infrastructure.

Below is an explanation of the type of subtests that were modified and
the corresponding WPT that they were moved to.

Many of the subtests were calling testColumnRowCSSParsing,
testColumnRowJSParsing, or testColumnEndRowEndJSParsing to set the value
of grid-row (or grid-row-end for testColumnEndJSParsing) and check the
values of grid-row, grid-row-start, and grid-row-end. These subtests
ended up having grid-row take one of the following forms:
- auto / auto
- <integer> / auto
- span <integer> / auto
- <integer> <custom-ident> / auto
- span <custom-ident> / auto
- span <integer> <custom-ident> / auto
- <custom-ident> / <custom-ident> (where the idents were exactly the
same)
In all of these cases the computed value of grid-row should serialize
to just the grid-row portion in the grammars above as that is the
shortest possible representation for each. These type of tests were
moved to css/css-grid/grid-row-shortest-serialization.html

Other subtests were calling testColumnRowInvalidJSParsing to test
invalid values of grid-row and grid-column. I moved all of the grid-row
ones into their own css/css-grid/grid-row-invalid.html WPT that has the
exact same tests.

The modified inheritance tests were also problematic because in some
cases they were setting the value of grid-row to one of the problematic
versions, so I simply ended up modifying the value of grid-row they
were using since they were testing inheritance rather than the
serialization of a specific value. For example, I changed
testStartBeforeInheritJSParsing("inherit", "18") to
testStartBeforeInheritJSParsing("inherit", "a") so that we would set
grid-row-start to "a" rather than "18."

Tests that were testing the value of "initial," for grid-row could also
be removed with no issue as this is already being tested in
css/css-grid/grid-layout-properties.html.

* LayoutTests/fast/css-grid-layout/grid-item-column-row-get-set-expected.txt:
* LayoutTests/fast/css-grid-layout/grid-item-column-row-get-set.html:
* LayoutTests/fast/css-grid-layout/grid-item-end-after-get-set-expected.txt:
* LayoutTests/fast/css-grid-layout/grid-item-end-after-get-set.html:
* LayoutTests/fast/css-grid-layout/grid-item-start-before-get-set-expected.txt:
* LayoutTests/fast/css-grid-layout/grid-item-start-before-get-set.html:
* LayoutTests/fast/css-grid-layout/named-grid-lines-with-named-grid-areas-get-set-expected.txt:
* LayoutTests/fast/css-grid-layout/named-grid-lines-with-named-grid-areas-get-set.html:
* LayoutTests/fast/css-grid-layout/resources/grid-item-column-row-parsing-utils.js:
(checkColumnValue):
(checkRowValue):
(checkColumnRowValues):
(window.testColumnCSSParsing):
(window.testColumnJSParsing):
(window.testColumnInvalidJSParsing):
(window.testColumnStartJSParsing):
(window.testColumnEndJSParsing):
(window.testColumnInitialJSParsing):
(window.testColumnRowInitialJSParsing): Deleted.
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-layout-properties-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-invalid-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-invalid.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-shortest-serialization-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-shortest-serialization.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-shorthand-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-row-shorthand.html: Added.
* Source/WebCore/css/CSSComputedStyleDeclaration.cpp:
(WebCore::CSSComputedStyleDeclaration::getPropertyValue const):

Canonical link: https://commits.webkit.org/268132@main




More information about the webkit-changes mailing list