Attachment 370551 Details for Bug 198208 – Patch

[patch] Patch

bug-198208-20190523193523.patch (text/plain), 5.81 KB, created by Simon Fraser (smfr) on 2019-05-23 19:35:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-05-23 19:35:24 PDT

Size: 5.81 KB

patch

obsolete

>Subversion Revision: 245694
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 538f400df4b004d62b520833f46c0a04ed389470..c29986d4954dddac445727f0a65202148cc953d2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-05-23  Simon Fraser  <simon.fraser@apple.com>
>+
>+        With async overflow scrolling, programmatic scroll to a negative offset fails to clamp the scroll offset
>+        https://bugs.webkit.org/show_bug.cgi?id=198208
>+        <rdar://problem/49720087>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        RenderLayer::scrollToOffset() needs to pass the clamped offset to scrollingCoordinator->requestScrollPositionUpdate(),
>+        otherwise the scrolling tree will round-trip a negative value and scrollLeft will end up negative.
>+
>+        Test: fast/scrolling/programmatic-scroll-to-negative-offset.html
>+
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::scrollToOffset):
>+
> 2019-05-23  Jon Davis  <jond@apple.com>
> 
>         Update feature status for shipped features
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 3c90cfbae599f60383e7328845d95e23bab68a6c..3a0d23c4620e47e06e01d11730b1b1507293abc5 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -2435,8 +2435,8 @@ ScrollOffset RenderLayer::clampScrollOffset(const ScrollOffset& scrollOffset) co
> 
> void RenderLayer::scrollToOffset(const ScrollOffset& scrollOffset, ScrollType scrollType, ScrollClamping clamping)
> {
>-    ScrollOffset newScrollOffset = clamping == ScrollClamping::Clamped ? clampScrollOffset(scrollOffset) : scrollOffset;
>-    if (newScrollOffset == this->scrollOffset())
>+    ScrollOffset clampedScrollOffset = clamping == ScrollClamping::Clamped ? clampScrollOffset(scrollOffset) : scrollOffset;
>+    if (clampedScrollOffset == this->scrollOffset())
>         return;
> 
>     auto previousScrollType = currentScrollType();
>@@ -2445,11 +2445,11 @@ void RenderLayer::scrollToOffset(const ScrollOffset& scrollOffset, ScrollType sc
>     bool handled = false;
> #if ENABLE(ASYNC_SCROLLING)
>     if (ScrollingCoordinator* scrollingCoordinator = page().scrollingCoordinator())
>-        handled = scrollingCoordinator->requestScrollPositionUpdate(*this, scrollPositionFromOffset(scrollOffset));
>+        handled = scrollingCoordinator->requestScrollPositionUpdate(*this, scrollPositionFromOffset(clampedScrollOffset));
> #endif
> 
>     if (!handled)
>-        scrollToOffsetWithoutAnimation(newScrollOffset, clamping);
>+        scrollToOffsetWithoutAnimation(clampedScrollOffset, clamping);
> 
>     setCurrentScrollType(previousScrollType);
> }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f70db7c85a93706e5ae2b49601348358e0dccd27..4338b4683a0667be0d51e67281247af559b88bd7 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-05-23  Simon Fraser  <simon.fraser@apple.com>
>+
>+        With async overflow scrolling, programmatic scroll to a negative offset fails to clamp the scroll offset
>+        https://bugs.webkit.org/show_bug.cgi?id=198208
>+        <rdar://problem/49720087>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/scrolling/programmatic-scroll-to-negative-offset-expected.txt: Added.
>+        * fast/scrolling/programmatic-scroll-to-negative-offset.html: Added.
>+
> 2019-05-23  Truitt Savell  <tsavell@apple.com>
> 
>         Add Mojave+ back to media/track/track-cue-rendering-vertical.html expectation
>diff --git a/LayoutTests/fast/scrolling/programmatic-scroll-to-negative-offset-expected.txt b/LayoutTests/fast/scrolling/programmatic-scroll-to-negative-offset-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..4564676c0cf2a9429722e880d363990fdc73c5fa
>--- /dev/null
>+++ b/LayoutTests/fast/scrolling/programmatic-scroll-to-negative-offset-expected.txt
>@@ -0,0 +1,11 @@
>+Tests a programmatic scroll to a negative offset does not result in negative scrollLeft/scrollTop
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS scroller.scrollLeft is 0
>+PASS scroller.scrollLeft is 0
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/scrolling/programmatic-scroll-to-negative-offset.html b/LayoutTests/fast/scrolling/programmatic-scroll-to-negative-offset.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..c5759d246a9e64b0d9f9daccd6ba109675a7fa98
>--- /dev/null
>+++ b/LayoutTests/fast/scrolling/programmatic-scroll-to-negative-offset.html
>@@ -0,0 +1,49 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+    <style>
>+        .container {
>+            height: 200px;
>+            width: 200px;
>+            border: 1px solid black;
>+            overflow: scroll;
>+        }
>+        .contents {
>+            height: 500px;
>+            width: 500px;
>+            background-color: silver;
>+        }
>+    </style>
>+	<script src="../../resources/js-test-pre.js"></script>
>+    <script>
>+		jsTestIsAsync = true;
>+		description("Tests a programmatic scroll to a negative offset does not result in negative scrollLeft/scrollTop");
>+
>+		var scroller;
>+		function doScroll()
>+        {
>+            scroller = document.getElementById('scroller');
>+			scroller.scrollLeft = 50;
>+
>+			scroller.addEventListener('scroll', (event) => {
>+				shouldBe("scroller.scrollLeft", "0");
>+				shouldBe("scroller.scrollLeft", "0");
>+				finishJSTest();
>+			}, false);
>+
>+			setTimeout(() => {
>+				scroller.scrollLeft = -200;
>+				scroller.scrollTop = -100;
>+			}, 0)
>+        }
>+        window.addEventListener('load', doScroll, false);
>+    </script>
>+</head>
>+<body>
>+    <div id="scroller" class="container">
>+        <div class="contents">
>+        </div>
>+    </div>
>+<script src="../../resources/js-test-post.js"></script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 198208: 370551