Title: [271480] trunk
Revision
271480
Author
[email protected]
Date
2021-01-14 03:02:43 -0800 (Thu, 14 Jan 2021)

Log Message

[css-scroll-snap] scroll-snap-align parsing is incorrect/backwards
https://bugs.webkit.org/show_bug.cgi?id=191865
<rdar://problem/46346516>

Patch by Martin Robinson <[email protected]> on 2021-01-14
Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

* web-platform-tests/css/css-scroll-snap/scroll-snap-type-change-expected.txt: Update test expectations.
* web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt: Ditto.
* web-platform-tests/css/css-scroll-snap/snap-after-relayout/changing-scroll-snap-type-expected.txt: Ditto.
* web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt: Ditto.
* web-platform-tests/css/css-scroll-snap/snap-inline-block-expected.txt: Ditto.

Source/WebCore:

An earlier version of the scroll snap specification specified that the arguments to
scroll-snap-align should be <inline> and then <block>, but a later version reversed
the order. This change aligns the WebKit implementation with the specification.

This has some web compatibility implications, but current measurements show that
the use of the two value variant is still low.

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::valueForScrollSnapAlignment): Reverse the order of scroll-snap-align serialization.
* style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertScrollSnapAlign): Reverse the order of scroll-snap-align parsing.

LayoutTests:

* TestExpectations: Update test expectations.
* css3/scroll-snap/scroll-snap-mismatch.html: Flip scroll-snap-align arguments in this test.
* platform/ios-wk2/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt: Update results to reflect new pass.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (271479 => 271480)


--- trunk/LayoutTests/ChangeLog	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/ChangeLog	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1,3 +1,15 @@
+2021-01-14  Martin Robinson  <[email protected]>
+
+        [css-scroll-snap] scroll-snap-align parsing is incorrect/backwards
+        https://bugs.webkit.org/show_bug.cgi?id=191865
+        <rdar://problem/46346516>
+
+        Reviewed by Simon Fraser.
+
+        * TestExpectations: Update test expectations.
+        * css3/scroll-snap/scroll-snap-mismatch.html: Flip scroll-snap-align arguments in this test.
+        * platform/ios-wk2/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt: Update results to reflect new pass.
+
 2021-01-13  Lauro Moura  <[email protected]>
 
         [WPE] Rebaseline and gardening a few tests

Modified: trunk/LayoutTests/TestExpectations (271479 => 271480)


--- trunk/LayoutTests/TestExpectations	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/TestExpectations	2021-01-14 11:02:43 UTC (rev 271480)
@@ -4520,7 +4520,7 @@
 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-snap-002.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-snap-003.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/direction-rtl.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/writing-mode-horizontal-tb.html [ ImageOnlyFailure ]
+imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/writing-mode-vertical-lr.html [ ImageOnlyFailure ]
 
 # Cocoa-only
 http/tests/media/hls/hls-hdr-switch.html [ Skip ]

Modified: trunk/LayoutTests/css3/scroll-snap/scroll-snap-mismatch.html (271479 => 271480)


--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-mismatch.html	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-mismatch.html	2021-01-14 11:02:43 UTC (rev 271480)
@@ -12,11 +12,11 @@
             }
 
             .good {
-                scroll-snap-align: start none;
+                scroll-snap-align: none start;
             }
 
             .bad {
-                scroll-snap-align: none start;
+                scroll-snap-align: start none;
             }
 
             .horizontalGalleryDrawer {
@@ -82,4 +82,4 @@
             <div id="console"></div>
         </div>
     </body>
-</html>
\ No newline at end of file
+</html>

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (271479 => 271480)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1,3 +1,17 @@
+2021-01-14  Martin Robinson  <[email protected]>
+
+        [css-scroll-snap] scroll-snap-align parsing is incorrect/backwards
+        https://bugs.webkit.org/show_bug.cgi?id=191865
+        <rdar://problem/46346516>
+
+        Reviewed by Simon Fraser.
+
+        * web-platform-tests/css/css-scroll-snap/scroll-snap-type-change-expected.txt: Update test expectations.
+        * web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt: Ditto.
+        * web-platform-tests/css/css-scroll-snap/snap-after-relayout/changing-scroll-snap-type-expected.txt: Ditto.
+        * web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt: Ditto.
+        * web-platform-tests/css/css-scroll-snap/snap-inline-block-expected.txt: Ditto.
+
 2021-01-13  Ziran Sun  <[email protected]>
 
         [css-logical] Implement logical border-radius

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-change-expected.txt (271479 => 271480)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-change-expected.txt	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-change-expected.txt	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1,4 +1,4 @@
 
-FAIL scroll-snap-type on HTML should control snapping behavior and changing it takes effect assert_equals: scrolling should snap expected 200 but got 100
-FAIL scroll-snap-type on DIV should control snapping behavior and changing it takes effect assert_equals: scrolling should snap expected 200 but got 100
+FAIL scroll-snap-type on HTML should control snapping behavior and changing it takes effect assert_equals: scrolling should not snap expected 100 but got 200
+PASS scroll-snap-type on DIV should control snapping behavior and changing it takes effect
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt (271479 => 271480)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1,5 +1,5 @@
 
 FAIL The scroll-snap-type on the root element is applied assert_equals: expected 515 but got 800
 FAIL The writing-mode (vertical-lr) on the body is used assert_equals: inline should snap expected 515 but got 800
-FAIL The writing-mode (horizontal-tb) on the body is used  assert_equals: inline should snap expected 100 but got 200
+PASS The writing-mode (horizontal-tb) on the body is used
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/changing-scroll-snap-type-expected.txt (271479 => 271480)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/changing-scroll-snap-type-expected.txt	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/changing-scroll-snap-type-expected.txt	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1,5 +1,5 @@
 
-FAIL Changing the scroller's snap type to y should make it resnap on the y-axis. assert_equals: expected 100 but got 0
-FAIL Changing the scroller's snap type to x should make it resnap on the x-axis. assert_equals: expected 100 but got 0
-FAIL Changing the scroller's snap type axis should make it resnap. assert_equals: expected 100 but got 0
+PASS Changing the scroller's snap type to y should make it resnap on the y-axis.
+PASS Changing the scroller's snap type to x should make it resnap on the x-axis.
+PASS Changing the scroller's snap type axis should make it resnap.
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt (271479 => 271480)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1,3 +1,3 @@
 
-FAIL Scroller should snap to at least one of the targets if unable to snap toboth after a layout change. assert_equals: expected 200 but got 400
+FAIL Scroller should snap to at least one of the targets if unable to snap toboth after a layout change. assert_true: expected true got false
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-inline-block-expected.txt (271479 => 271480)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-inline-block-expected.txt	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-inline-block-expected.txt	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1,10 +1,10 @@
 
-FAIL Snaps correctly for horizontal-tb writing mode with 'scroll-snap-align: end start' alignment assert_equals: aligns correctly on x expected 300 but got 115
-PASS Snaps correctly for vertical-lr writing mode with 'scroll-snap-align: end start' alignment
-FAIL Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: end start' alignment assert_equals: aligns correctly on x expected -315 but got -485
+PASS Snaps correctly for horizontal-tb writing mode with 'scroll-snap-align: end start' alignment
+FAIL Snaps correctly for vertical-lr writing mode with 'scroll-snap-align: end start' alignment assert_equals: aligns correctly on x expected 115 but got 300
+FAIL Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: end start' alignment assert_equals: aligns correctly on x expected -315 but got -300
 FAIL Snaps correctly for horizontal-tb writing mode with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on x expected 115 but got 0
 FAIL Snaps correctly for vertical-lr writing mode with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on x expected 300 but got 0
-FAIL Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on x expected -500 but got -300
-FAIL Snaps correctly for 'direction: rtl' with 'scroll-snap-align: end start' alignment assert_equals: aligns correctly on x expected -500 but got -300
-FAIL Snaps correctly for 'direction: rtl' with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on x expected -315 but got -485
+FAIL Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on x expected -500 but got -485
+FAIL Snaps correctly for 'direction: rtl' with 'scroll-snap-align: end start' alignment assert_equals: aligns correctly on x expected -500 but got -485
+FAIL Snaps correctly for 'direction: rtl' with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on x expected -315 but got -300
 

Modified: trunk/LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt (271479 => 271480)


--- trunk/LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1,5 +1,5 @@
 
 FAIL The scroll-snap-type on the root element is applied assert_equals: expected 500 but got 800
 FAIL The writing-mode (vertical-lr) on the body is used assert_equals: inline should snap expected 500 but got 800
-FAIL The writing-mode (horizontal-tb) on the body is used  assert_equals: inline should snap expected 100 but got 200
+PASS The writing-mode (horizontal-tb) on the body is used
 

Modified: trunk/Source/WebCore/ChangeLog (271479 => 271480)


--- trunk/Source/WebCore/ChangeLog	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/Source/WebCore/ChangeLog	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1,3 +1,23 @@
+2021-01-14  Martin Robinson  <[email protected]>
+
+        [css-scroll-snap] scroll-snap-align parsing is incorrect/backwards
+        https://bugs.webkit.org/show_bug.cgi?id=191865
+        <rdar://problem/46346516>
+
+        Reviewed by Simon Fraser.
+
+        An earlier version of the scroll snap specification specified that the arguments to
+        scroll-snap-align should be <inline> and then <block>, but a later version reversed
+        the order. This change aligns the WebKit implementation with the specification.
+
+        This has some web compatibility implications, but current measurements show that
+        the use of the two value variant is still low.
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::valueForScrollSnapAlignment): Reverse the order of scroll-snap-align serialization.
+        * style/StyleBuilderConverter.h:
+        (WebCore::Style::BuilderConverter::convertScrollSnapAlign): Reverse the order of scroll-snap-align parsing.
+
 2021-01-13  Andres Gonzalez  <[email protected]>
 
         Crash ASSERT in AccessibilityRenderObject::textUnderElement during AXIsolatedObject initialization.

Modified: trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp (271479 => 271480)


--- trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp	2021-01-14 11:02:43 UTC (rev 271480)
@@ -1112,8 +1112,8 @@
 static Ref<CSSValueList> valueForScrollSnapAlignment(const ScrollSnapAlign& alignment)
 {
     auto value = CSSValueList::createSpaceSeparated();
+    value->append(CSSPrimitiveValue::create(alignment.y));
     value->append(CSSPrimitiveValue::create(alignment.x));
-    value->append(CSSPrimitiveValue::create(alignment.y));
     return value;
 }
 

Modified: trunk/Source/WebCore/style/StyleBuilderConverter.h (271479 => 271480)


--- trunk/Source/WebCore/style/StyleBuilderConverter.h	2021-01-14 06:31:21 UTC (rev 271479)
+++ trunk/Source/WebCore/style/StyleBuilderConverter.h	2021-01-14 11:02:43 UTC (rev 271480)
@@ -922,11 +922,11 @@
 {
     auto& values = downcast<CSSValueList>(value);
     ScrollSnapAlign alignment;
-    alignment.x = downcast<CSSPrimitiveValue>(*values.item(0));
+    alignment.y = downcast<CSSPrimitiveValue>(*values.item(0));
     if (values.length() == 1)
-        alignment.y = alignment.x;
+        alignment.x = alignment.y;
     else
-        alignment.y = downcast<CSSPrimitiveValue>(*values.item(1));
+        alignment.x = downcast<CSSPrimitiveValue>(*values.item(1));
     return alignment;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to