Attachment 372922 Details for Bug 161611 – Patch

[patch] Patch

bug-161611-20190626221935.patch (text/plain), 12.77 KB, created by cathiechen on 2019-06-26 07:19:38 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: cathiechen

Created: 2019-06-26 07:19:38 PDT

Size: 12.77 KB

patch

obsolete

>Subversion Revision: 246733
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9cebaa19562dfca18abd38a652f217e3a38f2c34..9fdeec5d664b228af101e4fed8fee75ef636d985 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-06-26  Cathie Chen  <cathiechen@igalia.com>
>+
>+        Add supporting for writing-mode of scrollIntoViewOptions.
>+        https://bugs.webkit.org/show_bug.cgi?id=161611
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        To support writing-mode, toScrollAlignment*() will return the physical align side.
>+        RenderLayer::getRectToExpose() will compute the scroll offset according to the alignment.
>+
>+        * dom/Element.cpp:
>+        (WebCore::toScrollAlignmentForInlineDirection):
>+        (WebCore::toScrollAlignmentForBlockDirection):
>+        (WebCore::Element::scrollIntoView):
>+        (WebCore::toScrollAlignment): Deleted. Split to two.
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::getRectToExpose const): Support writing-mode.
>+
> 2019-06-24  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         Add user agent quirk for Google Drive
>diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
>index f07c5ec61d275bc903a468d6a4de49847e67f317..f1f811a56f2c5cb1ec25d35c80824d68f76f2cb8 100644
>--- a/Source/WebCore/dom/Element.cpp
>+++ b/Source/WebCore/dom/Element.cpp
>@@ -701,16 +701,80 @@ 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)
>+{
>+    switch (position.valueOr(ScrollLogicalPosition::Nearest)) {
>+    case ScrollLogicalPosition::Start: {
>+        switch (writingMode) {
>+        case TopToBottomWritingMode:
>+        case BottomToTopWritingMode:
>+            return ScrollAlignment::alignLeftAlways;
>+        case LeftToRightWritingMode:
>+        case RightToLeftWritingMode:
>+            return ScrollAlignment::alignTopAlways;
>+        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 ScrollAlignment::alignRightAlways;
>+        case LeftToRightWritingMode:
>+        case RightToLeftWritingMode:
>+            return ScrollAlignment::alignBottomAlways;
>+        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,8 +803,9 @@ void Element::scrollIntoView(Optional<Variant<bool, ScrollIntoViewOptions>>&& ar
>             options.blockPosition = ScrollLogicalPosition::End;
>     }
> 
>-    ScrollAlignment alignX = toScrollAlignment(options.inlinePosition, false);
>-    ScrollAlignment alignY = toScrollAlignment(options.blockPosition, true);
>+    auto writingMode = renderer()->style().writingMode();
>+    ScrollAlignment alignX = toScrollAlignmentForInlineDirection(options.inlinePosition, writingMode);
>+    ScrollAlignment alignY = toScrollAlignmentForBlockDirection(options.blockPosition, writingMode);
>     renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, alignX, alignY, ShouldAllowCrossOriginScrolling::No });
> }
> 
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index ca7290a02d3b7033d06c6394fa909f80f3efd016..ecb1d6011440c0d1baafa3075f3f3aef08a09102 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -2773,17 +2773,6 @@ LayoutRect RenderLayer::getRectToExpose(const LayoutRect& visibleRect, const Lay
>     if (scrollX == ScrollAlignment::Behavior::AlignToClosestEdge && exposeRect.maxX() > visibleRect.maxX() && exposeRect.width() < visibleRect.width())
>         scrollX = ScrollAlignment::Behavior::AlignRight;
> 
>-    // Given the X behavior, compute the X coordinate.
>-    LayoutUnit x;
>-    if (scrollX == ScrollAlignment::Behavior::NoScroll)
>-        x = visibleRect.x();
>-    else if (scrollX == ScrollAlignment::Behavior::AlignRight)
>-        x = exposeRect.maxX() - visibleRect.width();
>-    else if (scrollX == ScrollAlignment::Behavior::AlignCenter)
>-        x = exposeRect.x() + (exposeRect.width() - visibleRect.width()) / 2;
>-    else
>-        x = exposeRect.x();
>-
>     // Determine the appropriate Y behavior.
>     ScrollAlignment::Behavior scrollY;
>     LayoutRect exposeRectY(visibleRect.x(), exposeRect.y(), visibleRect.width(), exposeRect.height());
>@@ -2806,13 +2795,27 @@ LayoutRect RenderLayer::getRectToExpose(const LayoutRect& visibleRect, const Lay
>     if (scrollY == ScrollAlignment::Behavior::AlignToClosestEdge && exposeRect.maxY() > visibleRect.maxY() && exposeRect.height() < visibleRect.height())
>         scrollY = ScrollAlignment::Behavior::AlignBottom;
> 
>-    // Given the Y behavior, compute the Y coordinate.
>+    // Given the behavior and WritingMode, compute the X and Y coordinate.
>+    LayoutUnit x;
>     LayoutUnit y;
>-    if (scrollY == ScrollAlignment::Behavior::NoScroll)
>+    bool isHorizontal = renderer().style().isHorizontalWritingMode();
>+    auto physicalScrollX = isHorizontal ? scrollX : scrollY;
>+    auto physicalScrollY = isHorizontal ? scrollY : scrollX;
>+
>+    if (physicalScrollX == ScrollAlignment::Behavior::NoScroll)
>+        x = visibleRect.x();
>+    else if (physicalScrollX == ScrollAlignment::Behavior::AlignCenter)
>+        x = exposeRect.x() + (exposeRect.width() - visibleRect.width()) / 2;
>+    else if (physicalScrollX == ScrollAlignment::Behavior::AlignRight)
>+        x = exposeRect.maxX() - visibleRect.width();
>+    else
>+        x = exposeRect.x();
>+
>+    if (physicalScrollY == ScrollAlignment::Behavior::NoScroll)
>         y = visibleRect.y();
>-    else if (scrollY == ScrollAlignment::Behavior::AlignBottom)
>+    else if (physicalScrollY == ScrollAlignment::Behavior::AlignBottom)
>         y = exposeRect.maxY() - visibleRect.height();
>-    else if (scrollY == ScrollAlignment::Behavior::AlignCenter)
>+    else if (physicalScrollY == ScrollAlignment::Behavior::AlignCenter)
>         y = exposeRect.y() + (exposeRect.height() - visibleRect.height()) / 2;
>     else
>         y = exposeRect.y();
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 372878c914e08cc2b35236b1b40ba771b3693ace..8d9e51e85342a7edc4888a126a64b561c9e1ef61 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-06-26  Cathie Chen  <cathiechen@igalia.com>
>+
>+        Add supporting for writing-mode of scrollIntoViewOptions.
>+        https://bugs.webkit.org/show_bug.cgi?id=161611
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The x scroll coordinate of vertical-rl starts from the right side of scroll bar not left.
>+
>+        * web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode-expected.txt:
>+        * web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode.html:
>+
> 2019-06-24  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] Respect pointer capture when dispatching mouse boundary events and updating :hover
>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 b605c84842e88574e001b3c2e66d491825ac51ca..d2996604971913121befb583723f8490f6dc1f10 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 -200 +/- 0.5 but got -100
>-FAIL scrollIntoView({block: "start", inline: "center"}) assert_approx_equals: scrollX expected -200 +/- 0.5 but got -143
>-FAIL scrollIntoView({block: "start", inline: "end"}) assert_approx_equals: scrollX expected -200 +/- 0.5 but got -185
>-FAIL scrollIntoView({block: "center", inline: "start"}) assert_approx_equals: scrollX expected -157.5 +/- 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 -157.5 +/- 0.5 but got -185
>-FAIL scrollIntoView({block: "end", inline: "start"}) assert_approx_equals: scrollX expected -115 +/- 0.5 but got -100
>-FAIL scrollIntoView({block: "end", inline: "center"}) assert_approx_equals: scrollX expected -115 +/- 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-rl-writing-mode.html b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode.html
>index c404931f79b43a3060eaaa8a1fbc235ee16e5169..cff29b0088b92885d3d8e9d64b0ed99824738af3 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode.html
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-vertical-rl-writing-mode.html
>@@ -69,7 +69,7 @@ var expectedY = [ box_height, ((3*box_height - scroller_height)/2) + (scrollbar_
> // in a right-to-left mode, we adjust the expectation
> // to match the semantics of scrollLeft.
> if(scroller.scrollLeft === 0)
>-  expectedX = [ -box_width, -(((3*box_width - scroller_width)/2)+ (scrollbar_width/2)), -(((2*box_width)-scroller_width)+scrollbar_width)];
>+  expectedX = [ -(box_width - scrollbar_width), -(((3*box_width - scroller_width)/2)+ (scrollbar_width/2) - scrollbar_width), -((2*box_width)-scroller_width)];
> 
> // This formats dict as a string suitable as test name.
> // format_value() is provided by testharness.js,

Actions: View | Formatted Diff | Diff

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