Attachment 373630 Details for Bug 161611 – Patch

[patch] Patch

bug-161611-20190709001004.patch (text/plain), 19.47 KB, created by cathiechen on 2019-07-08 09:10:07 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: cathiechen

Created: 2019-07-08 09:10:07 PDT

Size: 19.47 KB

patch

obsolete

>Subversion Revision: 247203
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 7b8f6a802ddbb804ed7015c8783704f7158d8d63..f6ebe9f10738d1c2337dcdcf7aefd47a53a6bfb5 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-07-07  Cathie Chen  <cathiechen@igalia.com>
>+
>+        Support writing-mode and direction for scrollIntoViewOptions.
>+        https://bugs.webkit.org/show_bug.cgi?id=161611
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        To determine which physical side to align we need to transform ScrollIntoViewOptions to scrollAlignment.
>+        We'll translate the inline and block direction separately. The writing-mode will affect the block scrollAlignment.
>+        While both writing-mode and CSS direction will affect the inline scrollAlignment. The argument for scrollRectToVisible
>+        should be physical, so if !isHorizontalWritingMode(), we need to switch alignX and alignY.
>+
>+        For direction: rtl and writing-mode: horizontal-tb box, WebKit puts the scrollbar on the left side. The visible rect
>+        starts from the right side of the scroll bar, so localExposeRect should also start from the right side of
>+        scroll bar.
>+
>+        * dom/Element.cpp:
>+        (WebCore::toScrollAlignmentForInlineDirection):
>+        (WebCore::toScrollAlignmentForBlockDirection):
>+        (WebCore::Element::scrollIntoView):
>+        (WebCore::toScrollAlignment): Deleted.
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::scrollRectToVisible):
>+
> 2019-07-07  Zalan Bujtas  <zalan@apple.com>
> 
>         [ContentChangeObserver] Difficult to control videos on iqiyi.com as the actions are mouse hover
>diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
>index f07c5ec61d275bc903a468d6a4de49847e67f317..99bd891136e7af508b361165989781dce31a95be 100644
>--- a/Source/WebCore/dom/Element.cpp
>+++ b/Source/WebCore/dom/Element.cpp
>@@ -701,16 +701,84 @@ void Element::setHovered(bool flag)
>         renderer()->theme().stateChanged(*renderer(), ControlStates::HoverState);
> }
> 
>-// FIXME(webkit.org/b/161611): Take into account orientation/direction.
>-inline ScrollAlignment toScrollAlignment(Optional<ScrollLogicalPosition> position, bool isVertical)
>-{
>-    switch (position.valueOr(isVertical ? ScrollLogicalPosition::Start : ScrollLogicalPosition::Nearest)) {
>-    case ScrollLogicalPosition::Start:
>-        return isVertical ? ScrollAlignment::alignTopAlways : ScrollAlignment::alignLeftAlways;
>+inline ScrollAlignment toScrollAlignmentForInlineDirection(Optional<ScrollLogicalPosition> position, WritingMode writingMode, bool isLTR)
>+{
>+    switch (position.valueOr(ScrollLogicalPosition::Nearest)) {
>+    case ScrollLogicalPosition::Start: {
>+        switch (writingMode) {
>+        case TopToBottomWritingMode:
>+        case BottomToTopWritingMode: {
>+            return isLTR ? ScrollAlignment::alignLeftAlways : ScrollAlignment::alignRightAlways;
>+        }
>+        case LeftToRightWritingMode:
>+        case RightToLeftWritingMode: {
>+            return isLTR ? ScrollAlignment::alignTopAlways : ScrollAlignment::alignBottomAlways;
>+        }
>+        default:
>+            ASSERT_NOT_REACHED();
>+            return ScrollAlignment::alignLeftAlways;
>+        }
>+    }
>     case ScrollLogicalPosition::Center:
>         return ScrollAlignment::alignCenterAlways;
>-    case ScrollLogicalPosition::End:
>-        return isVertical ? ScrollAlignment::alignBottomAlways : ScrollAlignment::alignRightAlways;
>+    case ScrollLogicalPosition::End: {
>+        switch (writingMode) {
>+        case TopToBottomWritingMode:
>+        case BottomToTopWritingMode: {
>+            return isLTR ? ScrollAlignment::alignRightAlways : ScrollAlignment::alignLeftAlways;
>+        }
>+        case LeftToRightWritingMode:
>+        case RightToLeftWritingMode: {
>+            return isLTR ? ScrollAlignment::alignBottomAlways : ScrollAlignment::alignTopAlways;
>+        }
>+        default:
>+            ASSERT_NOT_REACHED();
>+            return ScrollAlignment::alignRightAlways;
>+        }
>+    }
>+    case ScrollLogicalPosition::Nearest:
>+        return ScrollAlignment::alignToEdgeIfNeeded;
>+    default:
>+        ASSERT_NOT_REACHED();
>+        return ScrollAlignment::alignToEdgeIfNeeded;
>+    }
>+}
>+
>+inline ScrollAlignment toScrollAlignmentForBlockDirection(Optional<ScrollLogicalPosition> position, WritingMode writingMode)
>+{
>+    switch (position.valueOr(ScrollLogicalPosition::Start)) {
>+    case ScrollLogicalPosition::Start: {
>+        switch (writingMode) {
>+        case TopToBottomWritingMode:
>+            return ScrollAlignment::alignTopAlways;
>+        case BottomToTopWritingMode:
>+            return ScrollAlignment::alignBottomAlways;
>+        case LeftToRightWritingMode:
>+            return ScrollAlignment::alignLeftAlways;
>+        case RightToLeftWritingMode:
>+            return ScrollAlignment::alignRightAlways;
>+        default:
>+            ASSERT_NOT_REACHED();
>+            return ScrollAlignment::alignTopAlways;
>+        }
>+    }
>+    case ScrollLogicalPosition::Center:
>+        return ScrollAlignment::alignCenterAlways;
>+    case ScrollLogicalPosition::End: {
>+        switch (writingMode) {
>+        case TopToBottomWritingMode:
>+            return ScrollAlignment::alignBottomAlways;
>+        case BottomToTopWritingMode:
>+            return ScrollAlignment::alignTopAlways;
>+        case LeftToRightWritingMode:
>+            return ScrollAlignment::alignRightAlways;
>+        case RightToLeftWritingMode:
>+            return ScrollAlignment::alignLeftAlways;
>+        default:
>+            ASSERT_NOT_REACHED();
>+            return ScrollAlignment::alignBottomAlways;
>+        }
>+    }
>     case ScrollLogicalPosition::Nearest:
>         return ScrollAlignment::alignToEdgeIfNeeded;
>     default:
>@@ -739,9 +807,18 @@ void Element::scrollIntoView(Optional<Variant<bool, ScrollIntoViewOptions>>&& ar
>             options.blockPosition = ScrollLogicalPosition::End;
>     }
> 
>-    ScrollAlignment alignX = toScrollAlignment(options.inlinePosition, false);
>-    ScrollAlignment alignY = toScrollAlignment(options.blockPosition, true);
>-    renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, alignX, alignY, ShouldAllowCrossOriginScrolling::No });
>+    auto writingMode = renderer()->style().writingMode();
>+    ScrollAlignment alignX = toScrollAlignmentForInlineDirection(options.inlinePosition, writingMode, renderer()->style().isLeftToRightDirection());
>+    ScrollAlignment alignY = toScrollAlignmentForBlockDirection(options.blockPosition, writingMode);
>+
>+    bool isHorizontal = renderer()->style().isHorizontalWritingMode();
>+    ScrollRectToVisibleOptions visibleOptions {
>+        SelectionRevealMode::Reveal,
>+        isHorizontal ? alignX : alignY,
>+        isHorizontal ? alignY : alignX,
>+        ShouldAllowCrossOriginScrolling::No
>+    };
>+    renderer()->scrollRectToVisible(absoluteBounds, insideFixed, visibleOptions);
> }
> 
> void Element::scrollIntoView(bool alignToTop) 
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 79b1c872f24db5eb66b0cc56f79f8c431169b4f7..1d3eb13507ea88d92855033731afca996d853d3e 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -2623,6 +2623,11 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& absoluteRect, bool insid
>         RenderBox* box = renderBox();
>         ASSERT(box);
>         LayoutRect localExposeRect(box->absoluteToLocalQuad(FloatQuad(FloatRect(absoluteRect))).boundingBox());
>+        if (shouldPlaceBlockDirectionScrollbarOnLeft()) {
>+            // For direction: rtl; writing-mode: horizontal-tb box, the scroll bar is on the left side. The visible rect
>+            // starts from the right side of scroll bar. So the x of localExposeRect should start from the same position too.
>+            localExposeRect.moveBy(LayoutPoint(-verticalScrollbarWidth(), 0));
>+        }
>         LayoutRect layerBounds(0_lu, 0_lu, box->clientWidth(), box->clientHeight());
>         LayoutRect revealRect = getRectToExpose(layerBounds, localExposeRect, insideFixed, options.alignX, options.alignY);
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 668c3504d1983581cb1c990b609259203ce43873..6e668d908c794917641b46a0b732fa71c7577711 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2019-07-07  Cathie Chen  <cathiechen@igalia.com>
>+
>+        Support writing-mode and direction for scrollIntoViewOptions.
>+        https://bugs.webkit.org/show_bug.cgi?id=161611
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/ios-simulator-wk2/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-tb-writing-mode-and-rtl-direction-expected.txt: Removed.
>+
> 2019-07-07  Zalan Bujtas  <zalan@apple.com>
> 
>         [ContentChangeObserver] Difficult to control videos on iqiyi.com as the actions are mouse hover
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index e9aa93b87ecc524a3921ffa4c28bc88dfc3e63c9..f4c33ae6fa8e9586ddc67e00516f18e53f3eadcb 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-07-07  Cathie Chen  <cathiechen@igalia.com>
>+
>+        Support writing-mode and direction for scrollIntoViewOptions.
>+        https://bugs.webkit.org/show_bug.cgi?id=161611
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * web-platform-tests/css/cssom-view/scrollIntoView-horizontal-tb-writing-mode-and-rtl-direction-expected.txt:
>+        * web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-and-rtl-direction-expected.txt:
>+        * web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-expected.txt:
>+        * web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode-expected.txt:
>+
> 2019-07-06  Cathie Chen  <cathiechen@igalia.com>
> 
>         Import css/cssom-view testcases from WPT.
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-tb-writing-mode-and-rtl-direction-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-tb-writing-mode-and-rtl-direction-expected.txt
>index 2b9fa89570a7a574557c77933e9c166b013c57a5..7efe71b3bbd3791e2db1a8f50f804879a5f639c3 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-tb-writing-mode-and-rtl-direction-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-tb-writing-mode-and-rtl-direction-expected.txt
>@@ -1,11 +1,11 @@
> 
>-FAIL scrollIntoView({"block":"start","inline":"start"}) assert_approx_equals: scrollX expected -200 +/- 0.5 but got -100
>-FAIL scrollIntoView({"block":"start","inline":"center"}) assert_approx_equals: scrollX expected -157.5 +/- 0.5 but got -143
>-FAIL scrollIntoView({"block":"start","inline":"end"}) assert_approx_equals: scrollX expected -115 +/- 0.5 but got -185
>-FAIL scrollIntoView({"block":"center","inline":"start"}) assert_approx_equals: scrollX expected -200 +/- 0.5 but got -100
>-FAIL scrollIntoView({"block":"center","inline":"center"}) assert_approx_equals: scrollX expected -157.5 +/- 0.5 but got -143
>-FAIL scrollIntoView({"block":"center","inline":"end"}) assert_approx_equals: scrollX expected -115 +/- 0.5 but got -185
>-FAIL scrollIntoView({"block":"end","inline":"start"}) assert_approx_equals: scrollX expected -200 +/- 0.5 but got -100
>-FAIL scrollIntoView({"block":"end","inline":"center"}) assert_approx_equals: scrollX expected -157.5 +/- 0.5 but got -143
>-FAIL scrollIntoView({"block":"end","inline":"end"}) assert_approx_equals: scrollX expected -115 +/- 0.5 but got -185
>+PASS scrollIntoView({"block":"start","inline":"start"}) 
>+PASS scrollIntoView({"block":"start","inline":"center"}) 
>+PASS scrollIntoView({"block":"start","inline":"end"}) 
>+PASS scrollIntoView({"block":"center","inline":"start"}) 
>+PASS scrollIntoView({"block":"center","inline":"center"}) 
>+PASS scrollIntoView({"block":"center","inline":"end"}) 
>+PASS scrollIntoView({"block":"end","inline":"start"}) 
>+PASS scrollIntoView({"block":"end","inline":"center"}) 
>+PASS scrollIntoView({"block":"end","inline":"end"}) 
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-and-rtl-direction-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-and-rtl-direction-expected.txt
>index 28d6d6039951d0c3cd74282af702e68a117e93c3..7efe71b3bbd3791e2db1a8f50f804879a5f639c3 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-and-rtl-direction-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-and-rtl-direction-expected.txt
>@@ -1,11 +1,11 @@
> 
>-FAIL scrollIntoView({"block":"start","inline":"start"}) assert_approx_equals: scrollY expected -200 +/- 0.5 but got -115
>-FAIL scrollIntoView({"block":"start","inline":"center"}) assert_approx_equals: scrollX expected 200 +/- 0.5 but got 158
>-FAIL scrollIntoView({"block":"start","inline":"end"}) assert_approx_equals: scrollX expected 200 +/- 0.5 but got 115
>-FAIL scrollIntoView({"block":"center","inline":"start"}) assert_approx_equals: scrollX expected 157.5 +/- 0.5 but got 200
>+PASS scrollIntoView({"block":"start","inline":"start"}) 
>+PASS scrollIntoView({"block":"start","inline":"center"}) 
>+PASS scrollIntoView({"block":"start","inline":"end"}) 
>+PASS scrollIntoView({"block":"center","inline":"start"}) 
> PASS scrollIntoView({"block":"center","inline":"center"}) 
>-FAIL scrollIntoView({"block":"center","inline":"end"}) assert_approx_equals: scrollX expected 157.5 +/- 0.5 but got 115
>-FAIL scrollIntoView({"block":"end","inline":"start"}) assert_approx_equals: scrollX expected 115 +/- 0.5 but got 200
>-FAIL scrollIntoView({"block":"end","inline":"center"}) assert_approx_equals: scrollX expected 115 +/- 0.5 but got 158
>-FAIL scrollIntoView({"block":"end","inline":"end"}) assert_approx_equals: scrollY expected -115 +/- 0.5 but got -200
>+PASS scrollIntoView({"block":"center","inline":"end"}) 
>+PASS scrollIntoView({"block":"end","inline":"start"}) 
>+PASS scrollIntoView({"block":"end","inline":"center"}) 
>+PASS scrollIntoView({"block":"end","inline":"end"}) 
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-expected.txt
>index dbf97adbff9a98fd1f38b692a9c2a98b063f33b3..7efe71b3bbd3791e2db1a8f50f804879a5f639c3 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-lr-writing-mode-expected.txt
>@@ -1,11 +1,11 @@
> 
> PASS scrollIntoView({"block":"start","inline":"start"}) 
>-FAIL scrollIntoView({"block":"start","inline":"center"}) assert_approx_equals: scrollX expected 200 +/- 0.5 but got 158
>-FAIL scrollIntoView({"block":"start","inline":"end"}) assert_approx_equals: scrollX expected 200 +/- 0.5 but got 115
>-FAIL scrollIntoView({"block":"center","inline":"start"}) assert_approx_equals: scrollX expected 157.5 +/- 0.5 but got 200
>+PASS scrollIntoView({"block":"start","inline":"center"}) 
>+PASS scrollIntoView({"block":"start","inline":"end"}) 
>+PASS scrollIntoView({"block":"center","inline":"start"}) 
> PASS scrollIntoView({"block":"center","inline":"center"}) 
>-FAIL scrollIntoView({"block":"center","inline":"end"}) assert_approx_equals: scrollX expected 157.5 +/- 0.5 but got 115
>-FAIL scrollIntoView({"block":"end","inline":"start"}) assert_approx_equals: scrollX expected 115 +/- 0.5 but got 200
>-FAIL scrollIntoView({"block":"end","inline":"center"}) assert_approx_equals: scrollX expected 115 +/- 0.5 but got 158
>+PASS scrollIntoView({"block":"center","inline":"end"}) 
>+PASS scrollIntoView({"block":"end","inline":"start"}) 
>+PASS scrollIntoView({"block":"end","inline":"center"}) 
> PASS scrollIntoView({"block":"end","inline":"end"}) 
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode-expected.txt
>index 2167ede87a80dc759830d8183a21e75771dc5770..7efe71b3bbd3791e2db1a8f50f804879a5f639c3 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode-expected.txt
>@@ -1,11 +1,11 @@
> 
>-FAIL scrollIntoView({"block":"start","inline":"start"}) assert_approx_equals: scrollX expected -185 +/- 0.5 but got -100
>-FAIL scrollIntoView({"block":"start","inline":"center"}) assert_approx_equals: scrollX expected -185 +/- 0.5 but got -143
>-FAIL scrollIntoView({"block":"start","inline":"end"}) assert_approx_equals: scrollY expected 115 +/- 0.5 but got 200
>-FAIL scrollIntoView({"block":"center","inline":"start"}) assert_approx_equals: scrollX expected -142.5 +/- 0.5 but got -100
>+PASS scrollIntoView({"block":"start","inline":"start"}) 
>+PASS scrollIntoView({"block":"start","inline":"center"}) 
>+PASS scrollIntoView({"block":"start","inline":"end"}) 
>+PASS scrollIntoView({"block":"center","inline":"start"}) 
> PASS scrollIntoView({"block":"center","inline":"center"}) 
>-FAIL scrollIntoView({"block":"center","inline":"end"}) assert_approx_equals: scrollX expected -142.5 +/- 0.5 but got -185
>-FAIL scrollIntoView({"block":"end","inline":"start"}) assert_approx_equals: scrollY expected 200 +/- 0.5 but got 115
>-FAIL scrollIntoView({"block":"end","inline":"center"}) assert_approx_equals: scrollX expected -100 +/- 0.5 but got -143
>-FAIL scrollIntoView({"block":"end","inline":"end"}) assert_approx_equals: scrollX expected -100 +/- 0.5 but got -185
>+PASS scrollIntoView({"block":"center","inline":"end"}) 
>+PASS scrollIntoView({"block":"end","inline":"start"}) 
>+PASS scrollIntoView({"block":"end","inline":"center"}) 
>+PASS scrollIntoView({"block":"end","inline":"end"}) 
> 
>diff --git a/LayoutTests/platform/ios-simulator-wk2/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-tb-writing-mode-and-rtl-direction-expected.txt b/LayoutTests/platform/ios-simulator-wk2/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-tb-writing-mode-and-rtl-direction-expected.txt
>deleted file mode 100644
>index a2a8887ba6b8e023b9539c3216da222c67394f36..0000000000000000000000000000000000000000
>--- a/LayoutTests/platform/ios-simulator-wk2/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-tb-writing-mode-and-rtl-direction-expected.txt
>+++ /dev/null
>@@ -1,11 +0,0 @@
>-
>-FAIL scrollIntoView({"block":"start","inline":"start"}) assert_approx_equals: scrollX expected -200 +/- 0.5 but got -115
>-PASS scrollIntoView({"block":"start","inline":"center"}) 
>-FAIL scrollIntoView({"block":"start","inline":"end"}) assert_approx_equals: scrollX expected -115 +/- 0.5 but got -200
>-FAIL scrollIntoView({"block":"center","inline":"start"}) assert_approx_equals: scrollX expected -200 +/- 0.5 but got -115
>-PASS scrollIntoView({"block":"center","inline":"center"}) 
>-FAIL scrollIntoView({"block":"center","inline":"end"}) assert_approx_equals: scrollX expected -115 +/- 0.5 but got -200
>-FAIL scrollIntoView({"block":"end","inline":"start"}) assert_approx_equals: scrollX expected -200 +/- 0.5 but got -115
>-PASS scrollIntoView({"block":"end","inline":"center"}) 
>-FAIL scrollIntoView({"block":"end","inline":"end"}) assert_approx_equals: scrollX expected -115 +/- 0.5 but got -200
>-

Actions: View | Formatted Diff | Diff

Attachments on bug 161611: 372922 | 372925 | 372926 | 372929 | 372935 | 373246 | 373311 | 373597 | 373630 | 373700