- Revision
- 247013
- Author
- [email protected]
- Date
- 2019-07-01 11:14:10 -0700 (Mon, 01 Jul 2019)
Log Message
iOS: REGRESSION(async scroll): Caret doesn't scroll when scrolling textarea
https://bugs.webkit.org/show_bug.cgi?id=198217
<rdar://problem/51097296>
Reviewed by Simon Fraser.
Source/WebCore:
Add a ScrollingLayerPositionAction argument to ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling, and
avoid bailing early in the case where ScrollingLayerPositionAction::Set is used. See the WebKit ChangeLog for
more detail.
Test: editing/selection/ios/update-selection-after-overflow-scroll.html
* page/scrolling/ScrollingTreeScrollingNode.cpp:
(WebCore::ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling):
* page/scrolling/ScrollingTreeScrollingNode.h:
Source/WebKit:
In iOS 12, when scrolling a text selection in an fast-scrolling container, editor state updates are scheduled
under AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll after the end of the scrolling gesture,
when the scrolling layer action is ScrollingLayerPositionAction::Set. This is no longer the case in iOS 13,
because we now bail in ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling after scroll deceleration
finishes since the scroll position didn't end up changing. Additionally, we no longer use
ScrollingLayerPositionAction::Set in the case where scrolling finished decelerating, since
ScrollingTreeScrollingNodeDelegateIOS::scrollViewDidScroll no longer uses to value of inUserInteraction to
determine whether to Set or Sync scrolling layer positions.
To restore iOS 12 behavior, ensure that we send a scrolling tree update using ScrollingLayerPositionAction::Set
after scrolling ends.
* UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:
(WebKit::ScrollingTreeScrollingNodeDelegateIOS::scrollViewDidScroll):
LayoutTests:
Add a new layout test to check that the text selection views are updated after scrolling in a fast overflow
scrolling container.
* editing/selection/ios/update-selection-after-overflow-scroll-expected.txt: Added.
* editing/selection/ios/update-selection-after-overflow-scroll.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (247012 => 247013)
--- trunk/LayoutTests/ChangeLog 2019-07-01 18:09:27 UTC (rev 247012)
+++ trunk/LayoutTests/ChangeLog 2019-07-01 18:14:10 UTC (rev 247013)
@@ -1,3 +1,17 @@
+2019-07-01 Wenson Hsieh <[email protected]>
+
+ iOS: REGRESSION(async scroll): Caret doesn't scroll when scrolling textarea
+ https://bugs.webkit.org/show_bug.cgi?id=198217
+ <rdar://problem/51097296>
+
+ Reviewed by Simon Fraser.
+
+ Add a new layout test to check that the text selection views are updated after scrolling in a fast overflow
+ scrolling container.
+
+ * editing/selection/ios/update-selection-after-overflow-scroll-expected.txt: Added.
+ * editing/selection/ios/update-selection-after-overflow-scroll.html: Added.
+
2019-06-30 Fujii Hironori <[email protected]>
Unreviewed, rolling out r246959.
Added: trunk/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll-expected.txt (0 => 247013)
--- trunk/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll-expected.txt 2019-07-01 18:14:10 UTC (rev 247013)
@@ -0,0 +1,24 @@
+This test verifies that a selection in a fast scrollable area is kept up to date after scrolling. To test manually, tap the button and scroll the editable area down; the selection move to account for the new scroll position.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS selectionRectsBefore[0].top is 108
+PASS selectionRectsBefore[0].width is 320
+PASS selectionRectsBefore[0].left is 0
+PASS selectionRectsBefore[0].height is 29
+PASS selectionRectsBefore[1].top is 137
+PASS selectionRectsBefore[1].width is 172
+PASS selectionRectsBefore[1].left is 0
+PASS selectionRectsBefore[1].height is 30
+PASS selectionRectsAfter[0].top is 58
+PASS selectionRectsAfter[0].width is 320
+PASS selectionRectsAfter[0].left is 0
+PASS selectionRectsAfter[0].height is 29
+PASS selectionRectsAfter[1].top is 87
+PASS selectionRectsAfter[1].width is 172
+PASS selectionRectsAfter[1].left is 0
+PASS selectionRectsAfter[1].height is 30
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll.html (0 => 247013)
--- trunk/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll.html (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll.html 2019-07-01 18:14:10 UTC (rev 247013)
@@ -0,0 +1,103 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+<script src=""
+<script src=""
+<meta name=viewport content="width=device-width, initial-scale=1">
+<style>
+body, html {
+ width: 100%;
+ height: 100%;
+ margin: 0;
+}
+
+#editor {
+ font-size: 24px;
+ width: 320px;
+ height: 200px;
+ overflow: scroll;
+}
+
+#console, #description {
+ width: 100%;
+}
+</style>
+<script>
+jsTestIsAsync = true;
+
+function rectsAreEqual(rects, otherRects)
+{
+ if (rects.length != otherRects.length)
+ return false;
+
+ for (let i = 0; i < rects.length; ++i) {
+ if (rects[i].top != otherRects[i].top
+ || rects[i].left != otherRects[i].left
+ || rects[i].width != otherRects[i].width
+ || rects[i].height != otherRects[i].height)
+ return false;
+ }
+
+ return true;
+}
+
+async function waitForSelectionRectsToChange(fromRects)
+{
+ let rects = fromRects;
+ while (rectsAreEqual(rects, fromRects))
+ rects = await UIHelper.getUISelectionViewRects();
+ return rects;
+}
+
+addEventListener("load", async function() {
+ description("This test verifies that a selection in a fast scrollable area is kept up to date after scrolling. To test manually, tap the button and scroll the editable area down; the selection move to account for the new scroll position.");
+
+ const editor = document.getElementById("editor");
+ const button = document.querySelector("button");
+ button.addEventListener("click", event => {
+ editor.focus();
+ getSelection().selectAllChildren(document.getElementById("select-target"));
+ event.preventDefault();
+ });
+
+ await UIHelper.activateElementAndWaitForInputSession(button);
+ selectionRectsBefore = await waitForSelectionRectsToChange([]);
+
+ await UIHelper.immediateScrollElementAtContentPointToOffset(150, 100, 0, 50);
+ selectionRectsAfter = await waitForSelectionRectsToChange(selectionRectsBefore);
+
+ shouldBe("selectionRectsBefore[0].top", "108");
+ shouldBe("selectionRectsBefore[0].width", "320");
+ shouldBe("selectionRectsBefore[0].left", "0");
+ shouldBe("selectionRectsBefore[0].height", "29");
+ shouldBe("selectionRectsBefore[1].top", "137");
+ shouldBe("selectionRectsBefore[1].width", "172");
+ shouldBe("selectionRectsBefore[1].left", "0");
+ shouldBe("selectionRectsBefore[1].height", "30");
+
+ shouldBe("selectionRectsAfter[0].top", "58");
+ shouldBe("selectionRectsAfter[0].width", "320");
+ shouldBe("selectionRectsAfter[0].left", "0");
+ shouldBe("selectionRectsAfter[0].height", "29");
+ shouldBe("selectionRectsAfter[1].top", "87");
+ shouldBe("selectionRectsAfter[1].width", "172");
+ shouldBe("selectionRectsAfter[1].left", "0");
+ shouldBe("selectionRectsAfter[1].height", "30");
+
+ editor.remove();
+ button.remove();
+ finishJSTest();
+});
+</script>
+</head>
+<body>
+<div id="editor" contenteditable>
+ <p>The quick brown fox jumped over the lazy dog.</p>
+ <p id="select-target">The quick brown fox jumped over the lazy dog.</p>
+ <p>The quick brown fox jumped over the lazy dog.</p>
+</div>
+<button>Click to select text</button>
+<div id="description"></div>
+<div id="console"></div>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (247012 => 247013)
--- trunk/Source/WebCore/ChangeLog 2019-07-01 18:09:27 UTC (rev 247012)
+++ trunk/Source/WebCore/ChangeLog 2019-07-01 18:14:10 UTC (rev 247013)
@@ -1,3 +1,21 @@
+2019-07-01 Wenson Hsieh <[email protected]>
+
+ iOS: REGRESSION(async scroll): Caret doesn't scroll when scrolling textarea
+ https://bugs.webkit.org/show_bug.cgi?id=198217
+ <rdar://problem/51097296>
+
+ Reviewed by Simon Fraser.
+
+ Add a ScrollingLayerPositionAction argument to ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling, and
+ avoid bailing early in the case where ScrollingLayerPositionAction::Set is used. See the WebKit ChangeLog for
+ more detail.
+
+ Test: editing/selection/ios/update-selection-after-overflow-scroll.html
+
+ * page/scrolling/ScrollingTreeScrollingNode.cpp:
+ (WebCore::ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling):
+ * page/scrolling/ScrollingTreeScrollingNode.h:
+
2019-07-01 Antti Koivisto <[email protected]>
REGRESSION(r240047): Overflow scrollers on WK1 fail to update their content size when it changes
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp (247012 => 247013)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-07-01 18:09:27 UTC (rev 247012)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2019-07-01 18:14:10 UTC (rev 247013)
@@ -194,10 +194,10 @@
repositionRelatedLayers();
}
-void ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport)
+void ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport, ScrollingLayerPositionAction scrollingLayerPositionAction)
{
bool scrollPositionChanged = !scrollPositionAndLayoutViewportMatch(position, overrideLayoutViewport);
- if (!scrollPositionChanged)
+ if (!scrollPositionChanged && scrollingLayerPositionAction != ScrollingLayerPositionAction::Set)
return;
m_currentScrollPosition = adjustedScrollPosition(position, ScrollPositionClamp::None);
@@ -206,7 +206,7 @@
repositionRelatedLayers();
scrollingTree().notifyRelatedNodesAfterScrollPositionChange(*this);
- scrollingTree().scrollingTreeNodeDidScroll(*this);
+ scrollingTree().scrollingTreeNodeDidScroll(*this, scrollingLayerPositionAction);
scrollingTree().didScrollByDelegatedScrolling();
}
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h (247012 => 247013)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2019-07-01 18:09:27 UTC (rev 247012)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2019-07-01 18:14:10 UTC (rev 247013)
@@ -63,7 +63,7 @@
void scrollTo(const FloatPoint&, ScrollType = ScrollType::User, ScrollPositionClamp = ScrollPositionClamp::ToContentEdges);
void scrollBy(const FloatSize&, ScrollPositionClamp = ScrollPositionClamp::ToContentEdges);
- void wasScrolledByDelegatedScrolling(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport = { });
+ void wasScrolledByDelegatedScrolling(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport = { }, ScrollingLayerPositionAction = ScrollingLayerPositionAction::Sync);
const FloatSize& scrollableAreaSize() const { return m_scrollableAreaSize; }
const FloatSize& totalContentsSize() const { return m_totalContentsSize; }
Modified: trunk/Source/WebKit/ChangeLog (247012 => 247013)
--- trunk/Source/WebKit/ChangeLog 2019-07-01 18:09:27 UTC (rev 247012)
+++ trunk/Source/WebKit/ChangeLog 2019-07-01 18:14:10 UTC (rev 247013)
@@ -1,3 +1,26 @@
+2019-07-01 Wenson Hsieh <[email protected]>
+
+ iOS: REGRESSION(async scroll): Caret doesn't scroll when scrolling textarea
+ https://bugs.webkit.org/show_bug.cgi?id=198217
+ <rdar://problem/51097296>
+
+ Reviewed by Simon Fraser.
+
+ In iOS 12, when scrolling a text selection in an fast-scrolling container, editor state updates are scheduled
+ under AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll after the end of the scrolling gesture,
+ when the scrolling layer action is ScrollingLayerPositionAction::Set. This is no longer the case in iOS 13,
+ because we now bail in ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling after scroll deceleration
+ finishes since the scroll position didn't end up changing. Additionally, we no longer use
+ ScrollingLayerPositionAction::Set in the case where scrolling finished decelerating, since
+ ScrollingTreeScrollingNodeDelegateIOS::scrollViewDidScroll no longer uses to value of inUserInteraction to
+ determine whether to Set or Sync scrolling layer positions.
+
+ To restore iOS 12 behavior, ensure that we send a scrolling tree update using ScrollingLayerPositionAction::Set
+ after scrolling ends.
+
+ * UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:
+ (WebKit::ScrollingTreeScrollingNodeDelegateIOS::scrollViewDidScroll):
+
2019-07-01 Per Arne Vollan <[email protected]>
Perform less work when a pre-warmed WebProcess is suspended or resumed.
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm (247012 => 247013)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm 2019-07-01 18:09:27 UTC (rev 247012)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm 2019-07-01 18:14:10 UTC (rev 247013)
@@ -327,7 +327,7 @@
return;
auto scrollPosition = ScrollableArea::scrollPositionFromOffset(scrollOffset, toFloatSize(scrollOrigin()));
- scrollingNode().wasScrolledByDelegatedScrolling(scrollPosition);
+ scrollingNode().wasScrolledByDelegatedScrolling(scrollPosition, { }, inUserInteraction ? ScrollingLayerPositionAction::Sync : ScrollingLayerPositionAction::Set);
}
void ScrollingTreeScrollingNodeDelegateIOS::currentSnapPointIndicesDidChange(unsigned horizontal, unsigned vertical) const
