Attachment 373246 Details for Bug 161611 – Patch

[patch] Patch

bug-161611-20190702013132.patch (text/plain), 13.54 KB, created by cathiechen on 2019-07-01 10:31:34 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: cathiechen

Created: 2019-07-01 10:31:34 PDT

Size: 13.54 KB

patch

obsolete

>Subversion Revision: 246733
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9cebaa19562dfca18abd38a652f217e3a38f2c34..b5f09a0ef5ed2ff65ad0d2c86bdd7096304ddd18 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-06-27  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!).
>+
>+        According to writing-mode, toScrollAlignment*() transform the positions in ScrollIntoViewOptions to
>+        scrollAlignment that indicating which physical side to align.
>+
>+        * dom/Element.cpp:
>+        (WebCore::toScrollAlignmentForInlineDirection):
>+        (WebCore::toScrollAlignmentForBlockDirection):
>+        (WebCore::Element::scrollIntoView):
>+        (WebCore::toScrollAlignment): Deleted. Split into inline and block direction separately.
>+
> 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..aa1828fd33a9bea5558703316a464d1c8c7d98cb 100644
>--- a/Source/WebCore/dom/Element.cpp
>+++ b/Source/WebCore/dom/Element.cpp
>@@ -701,16 +701,92 @@ 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: {
>+            if (isLTR)
>+                return ScrollAlignment::alignLeftAlways;
>+            return ScrollAlignment::alignRightAlways;
>+        }
>+        case LeftToRightWritingMode:
>+        case RightToLeftWritingMode: {
>+            if (isLTR)
>+                return ScrollAlignment::alignTopAlways;
>+            return ScrollAlignment::alignBottomAlways;
>+        }
>+        default:
>+            ASSERT_NOT_REACHED();
>+            return ScrollAlignment::alignLeftAlways;
>+        }
>+    }
>+    case ScrollLogicalPosition::Center:
>+        return ScrollAlignment::alignCenterAlways;
>+    case ScrollLogicalPosition::End: {
>+        switch (writingMode) {
>+        case TopToBottomWritingMode:
>+        case BottomToTopWritingMode: {
>+            if (isLTR)
>+                return ScrollAlignment::alignRightAlways;
>+            return ScrollAlignment::alignLeftAlways;
>+        }
>+        case LeftToRightWritingMode:
>+        case RightToLeftWritingMode: {
>+            if (isLTR)
>+                return ScrollAlignment::alignBottomAlways;
>+            return 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:
>-        return isVertical ? ScrollAlignment::alignBottomAlways : ScrollAlignment::alignRightAlways;
>+    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 +815,13 @@ 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);
>+    if (renderer()->style().isHorizontalWritingMode())
>+        renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, alignX, alignY, ShouldAllowCrossOriginScrolling::No });
>+    else
>+        renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, alignY, alignX, ShouldAllowCrossOriginScrolling::No });
> }
> 
> void Element::scrollIntoView(bool alignToTop) 
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 372878c914e08cc2b35236b1b40ba771b3693ace..95492abb88359d9912870638c2a3f9d51705bb7d 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,18 @@
>+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 origin is at the left top padding edge of the Element, scroll bar is inside paddinge edges.
>+        Add test cases for direction: rtl.
>+
>+        * web-platform-tests/css/cssom-view/scrollIntoView-direction-rtl-expected.txt:
>+        * web-platform-tests/css/cssom-view/scrollIntoView-direction-rtl.html:
>+        * 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-direction-rtl-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-direction-rtl-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..08b1d806acafe5d3eb64e5817fc7c718efb394da
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-direction-rtl-expected.txt
>@@ -0,0 +1,4 @@
>+
>+PASS scrollIntoView({inline: "start"}), direction: rtl 
>+PASS scrollIntoView({inline: "end"}), direction: rtl 
>+
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-direction-rtl.html b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-direction-rtl.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..555d62b7f3af24221017896570a7b7bd6d7c7d1d
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-direction-rtl.html
>@@ -0,0 +1,69 @@
>+<!DOCTYPE html>
>+<title>CSSOM View - scrollIntoView considers vertical-rl writing mode</title>
>+<meta charset="utf-8">
>+<link rel="author" title="Suneel Kota" href="mailto:suneel.kota@samsung.com">
>+<link rel="help" href="https://drafts.csswg.org/cssom-view/#dom-element-scrollintoview">
>+<script src="/resources/testharness.js"></script>
>+<script src="/resources/testharnessreport.js"></script>
>+
>+<style>
>+.box {
>+  float: left;
>+  width: 200px;
>+  height: 200px;
>+}
>+#scroller {
>+  direction: rtl;
>+  overflow-x: scroll;
>+  width: 300px;
>+  height: 215px;
>+}
>+#container{
>+  width: 600px;
>+  height: 200px;
>+}
>+#target {
>+  background-color: #ff0;
>+}
>+</style>
>+<body>
>+<div id="scroller">
>+  <div id="container">
>+
>+    
>+    <div class="row">
>+      <div class="box"></div>
>+      <div class="box" id="target"></div>
>+      <div class="box"></div>
>+    </div>
>+
>+  </div>
>+</div>
>+
>+<script>
>+var target = document.getElementById("target");
>+var scroller = document.getElementById("scroller");
>+var box_width = target.offsetWidth;
>+
>+scroller.scrollLeft = -10000;
>+var leftEdge = scroller.scrollLeft + box_width;
>+scroller.scrollLeft = 10000;
>+var rightEdge = scroller.scrollLeft - box_width;
>+
>+
>+test(() => {
>+  scroller.scrollTo(0, 0);
>+  target.scrollIntoView({inline: "start"});
>+  assert_approx_equals(scroller.scrollLeft, rightEdge, 0.5, "start should be the right edge");
>+}, `scrollIntoView({inline: "start"}), direction: rtl`);
>+
>+test(() => {
>+  scroller.scrollTo(0, 0);
>+  target.scrollIntoView({inline: "end"});
>+  assert_approx_equals(scroller.scrollLeft, leftEdge, 0.5, "end should be the left edge");
>+}, `scrollIntoView({inline: "end"}), direction: rtl`);
>+
>+</script>
>+
>+</body>
>+</html>
>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