Attachment 359340 Details for Bug 193522 – Patch

[patch] Patch

bug-193522-20190117130608.patch (text/plain), 16.65 KB, created by Dean Jackson on 2019-01-16 18:06:09 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Dean Jackson

Created: 2019-01-16 18:06:09 PST

Size: 16.65 KB

patch

obsolete

>Subversion Revision: 239725
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index e04b29e7e87ea0c78b37c70907752396ecd3ec39..0515ccadf3945e2c385309a3b23b1364942d4fc9 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,23 @@
>+2019-01-16  Dean Jackson  <dino@apple.com>
>+
>+        (iPad) Link tapping is sluggish on many sites
>+        https://bugs.webkit.org/show_bug.cgi?id=193522
>+        <rdar://problem/47102987>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Some WKWebView clients might set the initial zoom scale of the page to
>+        something other than 1, which disables the "fast tap" behaviour.
>+        The fix is very simple -- just check against the initial scale rather
>+        than 1.
>+
>+        The most likely regression from this would be pages designed for desktop,
>+        but provide a viewport tag saying width=device-width and initial-scale.
>+        They might stop allowing double-tap-to-zoom.
>+
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _allowsDoubleTapGestures]): Check against initial page scale.
>+
> 2019-01-08  Alex Christensen  <achristensen@webkit.org>
> 
>         Always call CompletionHandler in Cache::store
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index cf1919c93a738530759e37f33dbb10b4cd045878..97e2ee8e106863a2a17374b6132b9b48a5626113 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -2467,9 +2467,9 @@ static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOff
> 
>     // At this point, we have a page that asked for width = device-width. However,
>     // if the content's width and height were large, we might have had to shrink it.
>-    // Since we'll enable double tap zoom whenever we're not at the actual
>-    // initial scale, this simply becomes a test of the current scale against 1.
>-    return !areEssentiallyEqualAsFloat(contentZoomScale(self), 1);
>+    // We'll enable double tap zoom whenever we're not at the actual initial scale.
>+    // return !areEssentiallyEqualAsFloat(contentZoomScale(self), 1);
>+    return !areEssentiallyEqualAsFloat(contentZoomScale(self), _initialScaleFactor);
> }
> 
> - (BOOL)_stylusTapGestureShouldCreateEditableImage
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 763ea36bc98be0d74c0e89316b200867f5348ec5..af0fd667a28f5857b549796f214dd91b508f328f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,47 @@
>+2019-01-16  Dean Jackson  <dino@apple.com>
>+
>+        (iPad) Link tapping is sluggish on many sites
>+        https://bugs.webkit.org/show_bug.cgi?id=193522
>+        <rdar://problem/47102987>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a test that checks a double tap will trigger a click
>+        event on a page that is at initial scale.
>+
>+        Extra bonus: for some reason adding this test, or making this code
>+        change, uncovered a couple of bugs in existing tests. The
>+        viewport-zooms-from-element-to-initial-scale test was completely wrong
>+        because it was expecting the incorrect result, which was triggered by
>+        the zoom callback firing early at a forced scale value. The
>+        viewport-no-width-value-allows-double-tap test was triggering a JS
>+        error in its UI script. I modernised both of these to use UIHelper instead.
>+
>+        * fast/events/ios/fast-click-double-tap-sends-click-when-initial-scale-expected.txt: Added.
>+        * fast/events/ios/fast-click-double-tap-sends-click-when-initial-scale.html: Added.
>+        * fast/events/ios/viewport-no-width-value-allows-double-tap.html:
>+        * fast/events/ios/viewport-zooms-from-element-to-initial-scale-expected.txt:
>+        * fast/events/ios/viewport-zooms-from-element-to-initial-scale.html:
>+        * resources/ui-helper.js: Add doubleTapAt and zoomByDoubleTapAt helpers. Remove the
>+        unnecessary "Done" return value from many of the callbacks. Give zoomToScale a return
>+        value.
>+        (window.UIHelper.tapAt.return.new.Promise):
>+        (window.UIHelper.tapAt):
>+        (window.UIHelper.doubleTapAt.return.new.Promise):
>+        (window.UIHelper.doubleTapAt):
>+        (window.UIHelper.zoomByDoubleTapAt):
>+        (window.UIHelper.activateAt.return.new.Promise):
>+        (window.UIHelper.activateAt):
>+        (window.UIHelper.toggleCapsLock):
>+        (window.UIHelper.ensurePresentationUpdate.return.new.Promise):
>+        (window.UIHelper.ensurePresentationUpdate):
>+        (window.UIHelper.activateAndWaitForInputSessionAt.return.new.Promise.):
>+        (window.UIHelper.activateFormControl.return.new.Promise.):
>+        (window.UIHelper.replaceTextAtRange):
>+        (window.UIHelper.zoomToScale):
>+        (window.UIHelper.stylusTapAt.return.new.Promise):
>+        (window.UIHelper.stylusTapAt):
>+
> 2019-01-07  Youenn Fablet  <youenn@apple.com>
> 
>         LayoutTests/http/wpt/fetch/csp-reports-bypass-csp-checks.html is flaky
>diff --git a/LayoutTests/fast/events/ios/fast-click-double-tap-sends-click-when-initial-scale-expected.txt b/LayoutTests/fast/events/ios/fast-click-double-tap-sends-click-when-initial-scale-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..651c5b9e74c6eaf1a74a0af9dfce25a87fa0b1e9
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/fast-click-double-tap-sends-click-when-initial-scale-expected.txt
>@@ -0,0 +1,2 @@
>+PASS: Click fired on element with handler.
>+This document is set up to enable fast clicks, because it has not moved from its initial scale. Double tapping on the rectangle above should send a click event, not trigger a zoom.
>diff --git a/LayoutTests/fast/events/ios/fast-click-double-tap-sends-click-when-initial-scale.html b/LayoutTests/fast/events/ios/fast-click-double-tap-sends-click-when-initial-scale.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..ae1a353d019318e3c97662a45e841248d601c715
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/fast-click-double-tap-sends-click-when-initial-scale.html
>@@ -0,0 +1,45 @@
>+<!DOCTYPE html> 
>+
>+<html>
>+<meta name="viewport" content="initial-scale=0.98,width=device-width">
>+<head>
>+    <script src="../../../resources/ui-helper.js"></script>
>+    <script>
>+        if (window.testRunner) {
>+            testRunner.dumpAsText();
>+            testRunner.waitUntilDone();
>+        }
>+
>+        async function runTest()
>+        {
>+            document.getElementById("target").addEventListener("click", handleClick, false);
>+
>+            if (!window.UIHelper)
>+                return;
>+            await UIHelper.doubleTapAt(50, 50);
>+        }
>+
>+        function handleClick(event)
>+        {
>+            document.getElementById("target").textContent = "PASS: Click fired on element with handler.";
>+            testRunner.notifyDone();
>+        }
>+    </script>
>+    <style>
>+        body {
>+            margin: 0;
>+        }
>+        #target {
>+            height: 100px;
>+            width: 100px;
>+            background-color: silver;
>+        }
>+    </style>
>+</head>
>+<body onload="runTest()">
>+<div id="target"></div>
>+<div id="description">This document is set up to enable fast clicks, because
>+    it has not moved from its initial scale. Double tapping on the rectangle
>+    above should send a click event, not trigger a zoom.</div>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/events/ios/viewport-no-width-value-allows-double-tap.html b/LayoutTests/fast/events/ios/viewport-no-width-value-allows-double-tap.html
>index 4802253e69657d2a82b146702b1df3bccc680f5a..4f39bd3a2eade97a29ccfdc87196bf1c68f03bca 100644
>--- a/LayoutTests/fast/events/ios/viewport-no-width-value-allows-double-tap.html
>+++ b/LayoutTests/fast/events/ios/viewport-no-width-value-allows-double-tap.html
>@@ -5,28 +5,16 @@
> 
> <head>
>     <script src="../../../resources/js-test-pre.js"></script>
>-    <script id="ui-script" type="text/plain">
>-        (function() {
>-            uiController.doubleTapAtPoint(15, 400, function() {
>-                uiController.uiScriptComplete();
>-            });
>-        })();
>-    </script>
>+    <script src="../../../resources/ui-helper.js"></script>
> 
>     <script>
>-    var scriptCompleted = false;
>-    var clickCount = 0;
>     if (window.testRunner)
>         testRunner.waitUntilDone();
> 
>-    function getUIScript() {
>-        return document.getElementById("ui-script").text;
>-    }
>-
>-    function runTest() {
>+    async function runTest() {
>         window.addEventListener("scroll", scrolled, false);
>-        if (testRunner.runUIScript)
>-            testRunner.runUIScript(getUIScript());
>+        if (window.UIHelper)
>+            await UIHelper.doubleTapAt(15, 400);
>     }
>     function scrolled() {
>         if (window.testRunner)
>diff --git a/LayoutTests/fast/events/ios/viewport-zooms-from-element-to-initial-scale-expected.txt b/LayoutTests/fast/events/ios/viewport-zooms-from-element-to-initial-scale-expected.txt
>index 12f1e9099fb6230a9157586d252660ba3772a6ee..c97946492bf1845dd585a4b842c972a08f95098b 100644
>--- a/LayoutTests/fast/events/ios/viewport-zooms-from-element-to-initial-scale-expected.txt
>+++ b/LayoutTests/fast/events/ios/viewport-zooms-from-element-to-initial-scale-expected.txt
>@@ -1 +1 @@
>-The viewport zoomed to scale: 1.6
>+The viewport zoomed to scale: 5
>diff --git a/LayoutTests/fast/events/ios/viewport-zooms-from-element-to-initial-scale.html b/LayoutTests/fast/events/ios/viewport-zooms-from-element-to-initial-scale.html
>index f9fe216f6da40f3c414a4b1daeeb05fe4c1b1ef4..5b435d0e5e587f7620de19a6379dee8b4f60c675 100644
>--- a/LayoutTests/fast/events/ios/viewport-zooms-from-element-to-initial-scale.html
>+++ b/LayoutTests/fast/events/ios/viewport-zooms-from-element-to-initial-scale.html
>@@ -3,35 +3,22 @@
> <html>
> <meta name="viewport" content="initial-scale=5, width=device-width">
> <head>
>-    <script id="ui-script" type="text/plain">
>-        (function() {
>-            uiController.didEndZoomingCallback = function() {
>-                uiController.uiScriptComplete(uiController.zoomScale);
>-            };
>-            uiController.doubleTapAtPoint(15, 15, function() {});
>-        })();
>-    </script>
>+    <script src="../../../resources/ui-helper.js"></script>
>     <script>
>         if (window.testRunner) {
>             testRunner.dumpAsText();
>             testRunner.waitUntilDone();
>         }
> 
>-        function getUIScript()
>-        {
>-            return document.getElementById("ui-script").text;
>-        }
>-
>-        function runTest()
>+        async function runTest()
>         {
>-            if (!window.eventSender || !testRunner.runUIScript)
>+            if (!window.UIHelper)
>                 return;
> 
>-            eventSender.scalePageBy(1.6, 1.6);
>-            testRunner.runUIScript(getUIScript(), function(result) {
>-                document.getElementById("target").innerText = "The viewport zoomed to scale: " + Number(result);
>-                testRunner.notifyDone();
>-            });
>+            await UIHelper.zoomToScale(1.6);
>+            let result = await UIHelper.zoomByDoubleTapAt(15, 15);
>+            document.getElementById("target").innerText = "The viewport zoomed to scale: " + Number(result);
>+            testRunner.notifyDone();
>         }
>     </script>
>     <style>
>diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js
>index 2f569aacc55c45c7025ce216a70345729224bc6e..eb40525d2448ad5f9573804a0a42b00f68c235a8 100644
>--- a/LayoutTests/resources/ui-helper.js
>+++ b/LayoutTests/resources/ui-helper.js
>@@ -25,11 +25,61 @@ window.UIHelper = class UIHelper {
>         return new Promise((resolve) => {
>             testRunner.runUIScript(`
>                 uiController.singleTapAtPoint(${x}, ${y}, function() {
>-                    uiController.uiScriptComplete('Done');
>+                    uiController.uiScriptComplete();
>                 });`, resolve);
>         });
>     }
> 
>+    static doubleTapAt(x, y)
>+    {
>+        console.assert(this.isIOS());
>+
>+        if (!this.isWebKit2()) {
>+            eventSender.addTouchPoint(x, y);
>+            eventSender.touchStart();
>+            eventSender.releaseTouchPoint(0);
>+            eventSender.touchEnd();
>+            eventSender.addTouchPoint(x, y);
>+            eventSender.touchStart();
>+            eventSender.releaseTouchPoint(0);
>+            eventSender.touchEnd();
>+            return Promise.resolve();
>+        }
>+
>+        return new Promise((resolve) => {
>+            testRunner.runUIScript(`
>+                uiController.doubleTapAtPoint(${x}, ${y}, function() {
>+                    uiController.uiScriptComplete();
>+                });`, resolve);
>+        });
>+    }
>+
>+    static zoomByDoubleTapAt(x, y)
>+    {
>+        console.assert(this.isIOS());
>+
>+        if (!this.isWebKit2()) {
>+            eventSender.addTouchPoint(x, y);
>+            eventSender.touchStart();
>+            eventSender.releaseTouchPoint(0);
>+            eventSender.touchEnd();
>+            eventSender.addTouchPoint(x, y);
>+            eventSender.touchStart();
>+            eventSender.releaseTouchPoint(0);
>+            eventSender.touchEnd();
>+            return Promise.resolve();
>+        }
>+
>+        return new Promise((resolve) => {
>+            testRunner.runUIScript(`
>+                uiController.didEndZoomingCallback = () => {
>+                    uiController.didEndZoomingCallback = null;
>+                    uiController.uiScriptComplete(uiController.zoomScale);
>+                };
>+                uiController.doubleTapAtPoint(${x}, ${y}, () => {});`, resolve);
>+        });
>+    }
>+
>     static activateAt(x, y)
>     {
>         if (!this.isWebKit2() || !this.isIOS()) {
>@@ -42,7 +92,7 @@ window.UIHelper = class UIHelper {
>         return new Promise((resolve) => {
>             testRunner.runUIScript(`
>                 uiController.singleTapAtPoint(${x}, ${y}, function() {
>-                    uiController.uiScriptComplete('Done');
>+                    uiController.uiScriptComplete();
>                 });`, resolve);
>         });
>     }
>@@ -69,7 +119,7 @@ window.UIHelper = class UIHelper {
>     static toggleCapsLock()
>     {
>         return new Promise((resolve) => {
>-            testRunner.runUIScript(`uiController.toggleCapsLock(() => uiController.uiScriptComplete('Done'));`, resolve);
>+            testRunner.runUIScript(`uiController.toggleCapsLock(() => uiController.uiScriptComplete());`, resolve);
>         });
>     }
> 
>@@ -83,7 +133,7 @@ window.UIHelper = class UIHelper {
>         return new Promise(resolve => {
>             testRunner.runUIScript(`
>                 uiController.doAfterPresentationUpdate(function() {
>-                    uiController.uiScriptComplete('Done');
>+                    uiController.uiScriptComplete();
>                 });`, resolve);
>         });
>     }
>@@ -108,7 +158,7 @@ window.UIHelper = class UIHelper {
>             testRunner.runUIScript(`
>                 (function() {
>                     uiController.didShowKeyboardCallback = function() {
>-                        uiController.uiScriptComplete("Done");
>+                        uiController.uiScriptComplete();
>                     };
>                     uiController.singleTapAtPoint(${x}, ${y}, function() { });
>                 })()`, resolve);
>@@ -127,7 +177,7 @@ window.UIHelper = class UIHelper {
>             testRunner.runUIScript(`
>                 (function() {
>                     uiController.didStartFormControlInteractionCallback = function() {
>-                        uiController.uiScriptComplete("Done");
>+                        uiController.uiScriptComplete();
>                     };
>                     uiController.singleTapAtPoint(${x}, ${y}, function() { });
>                 })()`, resolve);
>@@ -250,7 +300,7 @@ window.UIHelper = class UIHelper {
>         return new Promise(resolve => {
>             testRunner.runUIScript(`(() => {
>                 uiController.replaceTextAtRange("${text}", ${location}, ${length});
>-                uiController.uiScriptComplete('Done');
>+                uiController.uiScriptComplete();
>             })()`, resolve);
>         });
>     }
>@@ -348,7 +398,7 @@ window.UIHelper = class UIHelper {
> 
>     static zoomToScale(scale)
>     {
>-        const uiScript = `uiController.zoomToScale(${scale}, () => uiController.uiScriptComplete())`;
>+        const uiScript = `uiController.zoomToScale(${scale}, () => uiController.uiScriptComplete(uiController.zoomScale))`;
>         return new Promise(resolve => testRunner.runUIScript(uiScript, resolve));
>     }
> 
>@@ -457,7 +507,7 @@ window.UIHelper = class UIHelper {
>         return new Promise((resolve) => {
>             testRunner.runUIScript(`
>                 uiController.stylusTapAtPoint(${x}, ${y}, 2, 1, 0.5, function() {
>-                    uiController.uiScriptComplete('Done');
>+                    uiController.uiScriptComplete();
>                 });`, resolve);
>         });
>     }

Flags:

wenson_hsieh: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193522: 359340