Title: [245285] trunk
- Revision
- 245285
- Author
- [email protected]
- Date
- 2019-05-14 09:43:51 -0700 (Tue, 14 May 2019)
Log Message
[iOS] Cannot scroll to beginning of document after scrolling to end of document and vice versa via key commands
https://bugs.webkit.org/show_bug.cgi?id=197848
<rdar://problem/49523065>
Patch by Daniel Bates <[email protected]> on 2019-05-14
Reviewed by Brent Fulgham.
Source/WebKit:
Following the fix for <rdar://problem/49523065>, UIKit no longer emits a keyup event for a Command-
modified key. This breaks WebKit's own implementation of key command handling for scrolling to the
beginning or end of the document (triggered using Command + Arrow Up and Command + Arrow Down,
respectively) because it watches for keyup events to reset state after initiating a scroll. If state
is not reset then the scroll key command logic becomes confused and may not perform a subsequent scroll.
It seems like we can actually get away with supporting these key commands and future Command modified
commands by preemptively reseting state on keydown if the Command modifier is held down. If this does
not work out then we can do something more complicated.
* UIProcess/ios/WKKeyboardScrollingAnimator.mm:
(-[WKKeyboardScrollingAnimator handleKeyEvent:]):
LayoutTests:
Add a test to ensure that key commands can be used to scroll to the end of the page and then
to the beginning of the page.
* fast/scrolling/ios/scroll-to-beginning-and-end-of-document-expected.txt: Added.
* fast/scrolling/ios/scroll-to-beginning-and-end-of-document.html: Added.
* resources/ui-helper.js:
(window.UIHelper.callFunctionAndWaitForScrollToFinish): Added. Convenience function that invokes the
specified function and returns a Promise that is resolved once the page has finished scrolling. To know
if the page has finished scrolling we listen for DOM scroll events and repeatedly reset a 300ms timer.
The delay of 300ms was chosen to be > 250ms (to give some margin of error), which is the upper bound
delay between scroll event firings, last I recall. When the timer expires we assume that page has
finished scrolling.
(window.UIHelper):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (245284 => 245285)
--- trunk/LayoutTests/ChangeLog 2019-05-14 16:36:39 UTC (rev 245284)
+++ trunk/LayoutTests/ChangeLog 2019-05-14 16:43:51 UTC (rev 245285)
@@ -1,3 +1,25 @@
+2019-05-14 Daniel Bates <[email protected]>
+
+ [iOS] Cannot scroll to beginning of document after scrolling to end of document and vice versa via key commands
+ https://bugs.webkit.org/show_bug.cgi?id=197848
+ <rdar://problem/49523065>
+
+ Reviewed by Brent Fulgham.
+
+ Add a test to ensure that key commands can be used to scroll to the end of the page and then
+ to the beginning of the page.
+
+ * fast/scrolling/ios/scroll-to-beginning-and-end-of-document-expected.txt: Added.
+ * fast/scrolling/ios/scroll-to-beginning-and-end-of-document.html: Added.
+ * resources/ui-helper.js:
+ (window.UIHelper.callFunctionAndWaitForScrollToFinish): Added. Convenience function that invokes the
+ specified function and returns a Promise that is resolved once the page has finished scrolling. To know
+ if the page has finished scrolling we listen for DOM scroll events and repeatedly reset a 300ms timer.
+ The delay of 300ms was chosen to be > 250ms (to give some margin of error), which is the upper bound
+ delay between scroll event firings, last I recall. When the timer expires we assume that page has
+ finished scrolling.
+ (window.UIHelper):
+
2019-05-14 Said Abou-Hallawa <[email protected]>
[CG] Adding support for HEIF-sequence ('public.heics') images
Added: trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document-expected.txt (0 => 245285)
--- trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document-expected.txt 2019-05-14 16:43:51 UTC (rev 245285)
@@ -0,0 +1,12 @@
+Test key commands to scroll to the end of the document and then to the beginning of the document. To run this test by hand, reload the page then press Command + Down Arrow and then press Command + Up Arrow.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.scrollY is INITIAL_Y_OFFSET
+PASS window.scrollY is >= INITIAL_Y_OFFSET + 1
+PASS window.scrollY is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document.html (0 => 245285)
--- trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document.html 2019-05-14 16:43:51 UTC (rev 245285)
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta name="viewport" content="width=device-width">
+<script src=""
+<script src=""
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<div id="test" style="height: 4096px; width: 256px; background-color: blue"></div>
+<script>
+window.jsTestIsAsync = true;
+
+// This is used to detect if scrolling is completely broken.
+const INITIAL_Y_OFFSET = 256;
+
+function done()
+{
+ document.body.removeChild(document.getElementById("test"));
+ finishJSTest();
+}
+
+async function runTest()
+{
+ await UIHelper.callFunctionAndWaitForScrollToFinish(() => window.scrollTo(0, INITIAL_Y_OFFSET));
+ shouldBe("window.scrollY", "INITIAL_Y_OFFSET");
+
+ // Scroll to the end of the document.
+ await UIHelper.callFunctionAndWaitForScrollToFinish(() => {
+ if (window.testRunner)
+ UIHelper.keyDown("downArrow", ["metaKey"]);
+ });
+ shouldBeGreaterThanOrEqual("window.scrollY", "INITIAL_Y_OFFSET + 1");
+
+
+ // Scroll to the beginning of the document.
+ await UIHelper.callFunctionAndWaitForScrollToFinish(() => {
+ if (window.testRunner)
+ UIHelper.keyDown("upArrow", ["metaKey"]);
+ });
+ shouldBeZero("window.scrollY");
+
+ done();
+}
+
+description("Test key commands to scroll to the end of the document and then to the beginning of the document. To run this test by hand, reload the page then press <kbd>Command</kbd> + <kbd>Down Arrow</kbd> and then press <kbd>Command</kbd> + <kbd>Up Arrow</kbd>.");
+runTest();
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/resources/ui-helper.js (245284 => 245285)
--- trunk/LayoutTests/resources/ui-helper.js 2019-05-14 16:36:39 UTC (rev 245284)
+++ trunk/LayoutTests/resources/ui-helper.js 2019-05-14 16:43:51 UTC (rev 245285)
@@ -884,4 +884,26 @@
if (menuRect)
await this.activateAt(menuRect.left + menuRect.width / 2, menuRect.top + menuRect.height / 2);
}
+
+ static callFunctionAndWaitForScrollToFinish(functionToCall, ...theArguments)
+ {
+ return new Promise((resolved) => {
+ function scrollDidFinish() {
+ window.removeEventListener("scroll", handleScroll, true);
+ resolved();
+ }
+
+ let lastScrollTimerId = 0; // When the timer with this id fires then the page has finished scrolling.
+ function handleScroll() {
+ if (lastScrollTimerId) {
+ window.clearTimeout(lastScrollTimerId);
+ lastScrollTimerId = 0;
+ }
+ lastScrollTimerId = window.setTimeout(scrollDidFinish, 300); // Over 250ms to give some room for error.
+ }
+ window.addEventListener("scroll", handleScroll, true);
+
+ functionToCall.apply(this, theArguments);
+ });
+ }
}
Modified: trunk/Source/WebKit/ChangeLog (245284 => 245285)
--- trunk/Source/WebKit/ChangeLog 2019-05-14 16:36:39 UTC (rev 245284)
+++ trunk/Source/WebKit/ChangeLog 2019-05-14 16:43:51 UTC (rev 245285)
@@ -1,3 +1,23 @@
+2019-05-14 Daniel Bates <[email protected]>
+
+ [iOS] Cannot scroll to beginning of document after scrolling to end of document and vice versa via key commands
+ https://bugs.webkit.org/show_bug.cgi?id=197848
+ <rdar://problem/49523065>
+
+ Reviewed by Brent Fulgham.
+
+ Following the fix for <rdar://problem/49523065>, UIKit no longer emits a keyup event for a Command-
+ modified key. This breaks WebKit's own implementation of key command handling for scrolling to the
+ beginning or end of the document (triggered using Command + Arrow Up and Command + Arrow Down,
+ respectively) because it watches for keyup events to reset state after initiating a scroll. If state
+ is not reset then the scroll key command logic becomes confused and may not perform a subsequent scroll.
+ It seems like we can actually get away with supporting these key commands and future Command modified
+ commands by preemptively reseting state on keydown if the Command modifier is held down. If this does
+ not work out then we can do something more complicated.
+
+ * UIProcess/ios/WKKeyboardScrollingAnimator.mm:
+ (-[WKKeyboardScrollingAnimator handleKeyEvent:]):
+
2019-05-14 Brent Fulgham <[email protected]>
Protect current WebFrame during form submission
Modified: trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm (245284 => 245285)
--- trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm 2019-05-14 16:36:39 UTC (rev 245284)
+++ trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm 2019-05-14 16:43:51 UTC (rev 245285)
@@ -346,7 +346,11 @@
return;
auto scroll = [self keyboardScrollForEvent:event];
- if (!scroll || event.type == WebEventKeyUp) {
+
+ // UIKit does not emit a keyup event when the Command key is down. See <rdar://problem/49523065>.
+ // For recognized key commands that include the Command key (e.g. Command + Arrow Up) we reset our
+ // state on keydown.
+ if (!scroll || event.type == WebEventKeyUp || (event.modifierFlags & WebEventFlagMaskCommandKey)) {
[self stopAnimatedScroll];
_scrollTriggeringKeyIsPressed = NO;
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
