WebKit Bugzilla
Attachment 373232 Details for
Bug 198217
: iOS: REGRESSION(async scroll): Caret doesn't scroll when scrolling textarea
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Approach #2
bug-198217-20190701073625.patch (text/plain), 11.54 KB, created by
Wenson Hsieh
on 2019-07-01 07:36:26 PDT
(
hide
)
Description:
Approach #2
Filename:
MIME Type:
Creator:
Wenson Hsieh
Created:
2019-07-01 07:36:26 PDT
Size:
11.54 KB
patch
obsolete
>Subversion Revision: 246947 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 5e5bacdb18fb9788f8cfb0c2a6a97f5efd1a8c54..5b45f3179091581241118edd0c4e3ddb95b07253 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,29 @@ >+2019-06-30 Wenson Hsieh <wenson_hsieh@apple.com> >+ >+ 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 NOBODY (OOPS!). >+ >+ 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. >+ >+ Instead, we can fix this by having async scrolling schedule editor state updates, but only when the selection is >+ either a range, or a caret in editable content (in which case up-to-date selection information is needed). >+ >+ Test: editing/selection/ios/update-selection-after-overflow-scroll.html >+ >+ * editing/VisibleSelection.h: >+ * page/scrolling/AsyncScrollingCoordinator.cpp: >+ (WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll): >+ > 2019-06-28 Zalan Bujtas <zalan@apple.com> > > [Text autosizing][iPadOS] bing.com is hard to read even with boosted text because of the line height >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 8ead54ff6f4abefb58a286e19b91931d0abf45a2..eea32bfb8cfbfa8428ac3286cbf2d6c67019eef1 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,17 @@ >+2019-06-30 Wenson Hsieh <wenson_hsieh@apple.com> >+ >+ 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 NOBODY (OOPS!). >+ >+ Don't bother scheduling editor state updates during overflow scrolling in the case where we don't have a >+ selection, or the selection is a caret in non-editable content. See the WebCore ChangeLog for more details. >+ >+ * WebProcess/WebPage/WebPage.cpp: >+ (WebKit::WebPage::didChangeOverflowScrollPosition): >+ > 2019-06-28 Ryan Haddad <ryanhaddad@apple.com> > > Unreviewed build fix attempt after r246928. >diff --git a/Source/WebCore/editing/VisibleSelection.h b/Source/WebCore/editing/VisibleSelection.h >index a943da8a7772d6cccbfe2ac7d8effa147b1f8c1a..0d342be3768538c7461a92af2554246da4403617 100644 >--- a/Source/WebCore/editing/VisibleSelection.h >+++ b/Source/WebCore/editing/VisibleSelection.h >@@ -98,7 +98,7 @@ public: > > WEBCORE_EXPORT Element* rootEditableElement() const; > WEBCORE_EXPORT bool isContentEditable() const; >- bool hasEditableStyle() const; >+ WEBCORE_EXPORT bool hasEditableStyle() const; > WEBCORE_EXPORT bool isContentRichlyEditable() const; > // Returns a shadow tree node for legacy shadow trees, a child of the > // ShadowRoot node for new shadow trees, or 0 for non-shadow trees. >diff --git a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp >index ca68f7630765cbcbe598d05e70ffeaf10237dac2..aa4c93189601741852a55ffdad41c6b057ec2b4b 100644 >--- a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp >+++ b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp >@@ -339,7 +339,7 @@ void AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScroll(ScrollingNo > scrollableArea->scrollToOffsetWithoutAnimation(ScrollableArea::scrollOffsetFromPosition(scrollPosition, toFloatSize(scrollableArea->scrollOrigin()))); > scrollableArea->setCurrentScrollType(previousScrollType); > >- if (scrollingLayerPositionAction == ScrollingLayerPositionAction::Set) >+ if (scrollingLayerPositionAction == ScrollingLayerPositionAction::Set || scrollingLayerPositionAction == ScrollingLayerPositionAction::Sync) > m_page->editorClient().overflowScrollPositionChanged(); > > #if PLATFORM(COCOA) >diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp >index 957fff1ab7238772c97debe76fbe9f34c9910ce8..b03779fd01b7f965ad79f0bfe3c25c55896b8ded 100644 >--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp >+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp >@@ -5359,6 +5359,13 @@ void WebPage::didChangeContents() > > void WebPage::didChangeOverflowScrollPosition() > { >+ auto frame = makeRef(m_page->focusController().focusedOrMainFrame()); >+ if (frame->selection().isNone() || frame->editor().ignoreSelectionChanges()) >+ return; >+ >+ if (frame->selection().isCaret() && !frame->selection().selection().hasEditableStyle()) >+ return; >+ > didChangeSelectionOrOverflowScrollPosition(); > } > >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 125350d262ea011a896992ab26f7f9d6ba6c6b2e..721c9627720bf9041c2098ef8b6c3d7adec7ad4f 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,17 @@ >+2019-06-30 Wenson Hsieh <wenson_hsieh@apple.com> >+ >+ 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 NOBODY (OOPS!). >+ >+ 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-28 Zalan Bujtas <zalan@apple.com> > > [Text autosizing][iPadOS] bing.com is hard to read even with boosted text because of the line height >diff --git a/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll-expected.txt b/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..c0f3da8c44bb2a05501675669f506aa986747e4c >--- /dev/null >+++ b/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll-expected.txt >@@ -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 >+ >diff --git a/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll.html b/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll.html >new file mode 100644 >index 0000000000000000000000000000000000000000..a2bdb6d1046708d0c8ef24038b9879dae7e4cfc7 >--- /dev/null >+++ b/LayoutTests/editing/selection/ios/update-selection-after-overflow-scroll.html >@@ -0,0 +1,103 @@ >+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true ] --> >+<html> >+<head> >+<script src="../../../resources/js-test.js"></script> >+<script src="../../../resources/ui-helper.js"></script> >+<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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
simon.fraser
:
review-
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 198217
:
373198
|
373232
|
373235