Attachment 373404 Details for Bug 199457 – Patch

[patch] Patch

bug-199457-20190703120525.patch (text/plain), 13.57 KB, created by zalan on 2019-07-03 12:05:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-07-03 12:05:26 PDT

Size: 13.57 KB

patch

obsolete

>Subversion Revision: 247013
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index c4a9acc71316eac20e11be74a28399792d941eb5..1977e595d6292964cfe0664947713b4a19fbe4f4 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-07-03  Zalan Bujtas  <zalan@apple.com>
>+
>+        [ContentChangeObserver] REGRESSION (r244356): Drop down menus collapse without user input - Ebay.com
>+        https://bugs.webkit.org/show_bug.cgi?id=199457
>+        <rdar://problem/52386563>
>+
>+        Reviewed by Simon Fraser.
>+
>+        There's a fixed, 32ms window for observing content changes after the tap is committed. r244356 introduced the fast-click behavior on form elements by omitting this fixed window and
>+        dispatch the synthetic click on the target node.
>+        This patch preserves the fast-click behavior, but now we stay at hover if the mouseMove event triggers a synchronous actionable visiblity change (as opposed to always proceed with click).
>+
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::handleSyntheticClick):
>+
> 2019-07-01  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         iOS: REGRESSION(async scroll): Caret doesn't scroll when scrolling textarea
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index e332e9b6fba3035f29606b4082bc3eb995363ad0..3c50691d61fff927a56fa95c44c745dd8dcd88e8 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -574,7 +574,7 @@ static void dispatchSyntheticMouseMove(Frame& mainFrame, const WebCore::FloatPoi
>     mainFrame.eventHandler().dispatchSyntheticMouseMove(mouseEvent);
> }
> 
>-static bool nodeAlwaysTriggersClick(const Node& targetNode)
>+static bool nodeTriggersFastPath(const Node& targetNode)
> {
>     if (!is<Element>(targetNode))
>         return false;
>@@ -677,9 +677,9 @@ void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::F
> 
>     auto& contentChangeObserver = respondingDocument.contentChangeObserver();
>     auto observedContentChange = contentChangeObserver.observedContentChange();
>-    auto targetNodeTriggersClick = nodeAlwaysTriggersClick(nodeRespondingToClick);
>+    auto targetNodeTriggersFastPath = nodeTriggersFastPath(nodeRespondingToClick);
> 
>-    auto continueContentObservation = !(observedContentChange == WKContentVisibilityChange || targetNodeTriggersClick);
>+    auto continueContentObservation = !(observedContentChange == WKContentVisibilityChange || targetNodeTriggersFastPath);
>     if (continueContentObservation) {
>         // Wait for callback to completePendingSyntheticClickForContentChangeObserver() to decide whether to send the click event.
>         const Seconds observationDuration = 32_ms;
>@@ -692,11 +692,11 @@ void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::F
>         return;
>     }
> 
>-    callOnMainThread([protectedThis = makeRefPtr(this), targetNode = Ref<Node>(nodeRespondingToClick), location, modifiers, observedContentChange, targetNodeTriggersClick, pointerId] {
>+    callOnMainThread([protectedThis = makeRefPtr(this), targetNode = Ref<Node>(nodeRespondingToClick), location, modifiers, observedContentChange, pointerId] {
>         if (protectedThis->m_isClosed || !protectedThis->corePage())
>             return;
> 
>-        auto shouldStayAtHoverState = observedContentChange == WKContentVisibilityChange && !targetNodeTriggersClick;
>+        auto shouldStayAtHoverState = observedContentChange == WKContentVisibilityChange;
>         if (shouldStayAtHoverState) {
>             // The move event caused new contents to appear. Don't send synthetic click event, but just ensure that the mouse is on the most recent content.
>             dispatchSyntheticMouseMove(protectedThis->corePage()->mainFrame(), location, modifiers, pointerId);
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 0a4eeb9e6881e34e2ad055c99731e3285adcd5fa..ced97b98e0f7cbedf86bec7a67c214856e8ef3bd 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-07-03  Zalan Bujtas  <zalan@apple.com>
>+
>+        REGRESSION (r244356): Drop down menus collapse without user input - Ebay.com
>+        https://bugs.webkit.org/show_bug.cgi?id=199457
>+        <rdar://problem/52386563>
>+
>+        Reviewed by Simon Fraser.
>+
>+        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change-expected.txt: Added.
>+        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change.html: Copied from LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html.
>+        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change-expected.txt: Added.
>+        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change.html: Copied from LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html.
>+        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html:
>+
> 2019-07-01  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         iOS: REGRESSION(async scroll): Caret doesn't scroll when scrolling textarea
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html
>index ad397fbcc20ab9feeda9e1c5c52df3c303dec97b..69eae14582dc4254f283384629378e809fd8a158 100644
>--- a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html
>@@ -43,10 +43,6 @@ tapthis.addEventListener("mousemove", function( event ) {
>     becomesVisible.style.visibility = "visible";
> }, false);
> 
>-becomesVisible.addEventListener("click", function( event ) {   
>-    result.innerHTML = "clicked hidden";
>-}, false);
>-
> tapthis.addEventListener("click", function( event ) {   
>     result.innerHTML = "clicked";
>     if (window.testRunner)
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..31954a39d709a4a7614a108814824b101a42cdfb
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change-expected.txt
>@@ -0,0 +1,3 @@
>+PASS if 'clicked' text is shown below.
>+
>+clicked
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change.html b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e067e51caad63989d9ef3b9846bb1fb26b130d77
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change.html
>@@ -0,0 +1,57 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<title>This tests the case when the tap target node is a form control element but it triggers async actionable visibility change -> click (we want to preserve fast click behavior on form controls).</title>
>+<script src="../../../../../resources/basic-gestures.js"></script>
>+<style>
>+#becomesVisible {
>+    visibility: hidden;
>+    width: 100px;
>+    height: 100px;
>+    background-color: green;
>+	transition-property: opacity, left;
>+	transition-duration: 100ms, 200ms;
>+}
>+</style>
>+<script>
>+async function test() {
>+    if (!window.testRunner || !testRunner.runUIScript)
>+        return;
>+    if (window.internals)
>+        internals.settings.setContentChangeObserverEnabled(true);
>+
>+    testRunner.waitUntilDone();
>+    testRunner.dumpAsText();
>+
>+    let rect = tapthis.getBoundingClientRect();
>+    let x = rect.left + rect.width / 2;
>+    let y = rect.top + rect.height / 2;
>+
>+    await tapAtPoint(x, y);
>+}
>+</script>
>+</head>
>+<body onload="test()">
>+PASS if 'clicked' text is shown below.<br>
>+<input type="button" value="tap this button" id=tapthis>
>+<div id=becomesVisible></div>
>+<pre id=result></pre>
>+<script>
>+tapthis.addEventListener("mousemove", function( event ) {
>+    setTimeout(function() {
>+        becomesVisible.style.visibility = "visible";
>+    }, 100);
>+}, false);
>+
>+becomesVisible.addEventListener("click", function( event ) {   
>+    result.innerHTML = "clicked hidden";
>+}, false);
>+
>+tapthis.addEventListener("click", function( event ) {   
>+    result.innerHTML = "clicked";
>+    if (window.testRunner)
>+        testRunner.notifyDone();
>+}, false);
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..a7047838af69b67c4c926dfbfe8202f408fee95a
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change-expected.txt
>@@ -0,0 +1,3 @@
>+PASS if 'clicked' text is NOT shown below.
>+
>+
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change.html b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..d73cdeaec12a7e4baa518b55afa901ca3950dd8d
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change.html
>@@ -0,0 +1,55 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<title>This tests the case when the tap target node is a form control element but it triggers synchronous actionable visibility change -> "fast" hover.</title>
>+<script src="../../../../../resources/basic-gestures.js"></script>
>+<style>
>+#becomesVisible {
>+    visibility: hidden;
>+    width: 100px;
>+    height: 100px;
>+    background-color: green;
>+	transition-property: opacity, left;
>+	transition-duration: 100ms, 200ms;
>+}
>+</style>
>+<script>
>+async function test() {
>+    if (!window.testRunner || !testRunner.runUIScript)
>+        return;
>+    if (window.internals)
>+        internals.settings.setContentChangeObserverEnabled(true);
>+
>+    testRunner.waitUntilDone();
>+    testRunner.dumpAsText();
>+
>+    let rect = tapthis.getBoundingClientRect();
>+    let x = rect.left + rect.width / 2;
>+    let y = rect.top + rect.height / 2;
>+
>+    await tapAtPoint(x, y);
>+}
>+</script>
>+</head>
>+<body onload="test()">
>+PASS if 'clicked' text is NOT shown below.<br>
>+<input type="button" value="tap this button" id=tapthis>
>+<div id=becomesVisible></div>
>+<pre id=result></pre>
>+<script>
>+tapthis.addEventListener("mousemove", function( event ) {
>+    becomesVisible.style.visibility = "visible";
>+    if (window.testRunner)
>+        setTimeout("testRunner.notifyDone()", 50);
>+}, false);
>+
>+becomesVisible.addEventListener("click", function( event ) {   
>+    result.innerHTML = "clicked hidden";
>+}, false);
>+
>+tapthis.addEventListener("click", function( event ) {   
>+    result.innerHTML = "clicked";
>+}, false);
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html
>index fde28249bdc91d2f1566fccf507b517148ec14a4..1a2da91b6dc8f6acbeb679ecb732c8fbd30fa887 100644
>--- a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html
>@@ -1,7 +1,7 @@
> <!DOCTYPE html>
> <html>
> <head>
>-<title>This tests the case when the tap target node is a form control element.</title>
>+<title>This tests the case when the tap target node is a form control element -> "fast" click.</title>
> <script src="../../../../../resources/basic-gestures.js"></script>
> <style>
> #becomesVisible {
>@@ -41,10 +41,6 @@ tapthis.addEventListener("mousemove", function( event ) {
>     becomesVisible.style.visibility = "visible";
> }, false);
> 
>-becomesVisible.addEventListener("click", function( event ) {   
>-    result.innerHTML = "clicked hidden";
>-}, false);
>-
> tapthis.addEventListener("click", function( event ) {   
>     result.innerHTML = "clicked";
>     if (window.testRunner)
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-text-element.html b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-text-element.html
>index a73cec765bd3525f8347f5adb4f46c78d09e318b..ce768d50c9eddccb65cb2c20366e760268bb0d0b 100644
>--- a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-text-element.html
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-text-element.html
>@@ -41,10 +41,6 @@ tapthis.addEventListener("mousemove", function( event ) {
>     becomesVisible.style.visibility = "visible";
> }, false);
> 
>-becomesVisible.addEventListener("click", function( event ) {   
>-    result.innerHTML = "clicked hidden";
>-}, false);
>-
> tapthis.addEventListener("click", function( event ) {   
>     result.innerHTML = "clicked";
>     if (window.testRunner)

Actions: View | Formatted Diff | Diff

Attachments on bug 199457: 373400 | 373404