<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[245285] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/245285">245285</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2019-05-14 09:43:51 -0700 (Tue, 14 May 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>[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 <dabates@apple.com> 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):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsresourcesuihelperjs">trunk/LayoutTests/resources/ui-helper.js</a></li>
<li><a href="#trunkSourceWebKitChangeLog">trunk/Source/WebKit/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitUIProcessiosWKKeyboardScrollingAnimatormm">trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastscrollingiosscrolltobeginningandendofdocumentexpectedtxt">trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastscrollingiosscrolltobeginningandendofdocumenthtml">trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (245284 => 245285)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2019-05-14 16:36:39 UTC (rev 245284)
+++ trunk/LayoutTests/ChangeLog 2019-05-14 16:43:51 UTC (rev 245285)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2019-05-14  Daniel Bates  <dabates@apple.com>
+
+        [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):
+
</ins><span class="cx"> 2019-05-14  Said Abou-Hallawa  <sabouhallawa@apple.com>
</span><span class="cx"> 
</span><span class="cx">         [CG] Adding support for HEIF-sequence ('public.heics') images
</span></span></pre></div>
<a id="trunkLayoutTestsfastscrollingiosscrolltobeginningandendofdocumentexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document-expected.txt (0 => 245285)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,12 @@
</span><ins>+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
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastscrollingiosscrolltobeginningandendofdocumenthtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/scrolling/ios/scroll-to-beginning-and-end-of-document.html (0 => 245285)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,51 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<head>
+<meta name="viewport" content="width=device-width">
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+</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>
</ins></span></pre></div>
<a id="trunkLayoutTestsresourcesuihelperjs"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/resources/ui-helper.js (245284 => 245285)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -884,4 +884,26 @@
</span><span class="cx">         if (menuRect)
</span><span class="cx">             await this.activateAt(menuRect.left + menuRect.width / 2, menuRect.top + menuRect.height / 2);
</span><span class="cx">     }
</span><ins>+
+    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);
+        });
+    }
</ins><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/ChangeLog (245284 => 245285)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2019-05-14  Daniel Bates  <dabates@apple.com>
+
+        [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:]):
+
</ins><span class="cx"> 2019-05-14  Brent Fulgham  <bfulgham@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Protect current WebFrame during form submission
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessiosWKKeyboardScrollingAnimatormm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm (245284 => 245285)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -346,7 +346,11 @@
</span><span class="cx">         return;
</span><span class="cx"> 
</span><span class="cx">     auto scroll = [self keyboardScrollForEvent:event];
</span><del>-    if (!scroll || event.type == WebEventKeyUp) {
</del><ins>+
+    // 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)) {
</ins><span class="cx">         [self stopAnimatedScroll];
</span><span class="cx">         _scrollTriggeringKeyIsPressed = NO;
</span><span class="cx">     }
</span></span></pre>
</div>
</div>

</body>
</html>