Attachment 373478 Details for Bug 199502 – Patch

[patch] Patch

bug-199502-20190704212318.patch (text/plain), 5.95 KB, created by zalan on 2019-07-04 21:23:19 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-07-04 21:23:19 PDT

Size: 5.95 KB

patch

obsolete

>Subversion Revision: 247146
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 9a39854285d381f6d12c7f1f03811802823981ba..b082d100b946ba3a0ddd76adb4d43711239e97e2 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-07-04  Zalan Bujtas  <zalan@apple.com>
>+
>+        [ContentChangeObserver] REGRESSION (r247015): facebook photo/video upload button is unresponsive to user interaction.
>+        https://bugs.webkit.org/show_bug.cgi?id=199502
>+        <rdar://problem/52547473>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Apparently it's a common practice to put transparent elements over visible click targets (button like divs) and use the invisible
>+        elements to catch the user input (e.g. Facebook's Photo/Video button).
>+        This patch modifies the original "do not trigger click on invisible targets" heuristic to a more restrictive "do not trigger
>+        click if the click target was previously hidden and became visible through touch start".
>+        If this still breaks some use cases, we could turn it into YouTube quirk.       
>+
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::handleSyntheticClick):
>+
> 2019-07-04  Chris Dumez  <cdumez@apple.com>
> 
>         Simplify logic that handles registering WebProcessProxy objects with their WebsiteDataStore
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index c31c7246116cf6db1fdd29be30cdd6a20b20a755..e81bf31da6d179f06b2dbdaf59240ff99caa4ba6 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -666,7 +666,7 @@ void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::F
> 
>     auto& respondingDocument = nodeRespondingToClick.document();
>     auto& contentChangeObserver = respondingDocument.contentChangeObserver();
>-    auto targetNodeisConsideredHidden = contentChangeObserver.hiddenTouchTarget() == &nodeRespondingToClick || ContentChangeObserver::isConsideredHidden(nodeRespondingToClick);
>+    auto targetNodeWentFromHiddenToVisible = contentChangeObserver.hiddenTouchTarget() == &nodeRespondingToClick && !ContentChangeObserver::isConsideredHidden(nodeRespondingToClick);
>     {
>         LOG_WITH_STREAM(ContentObservation, stream << "handleSyntheticClick: node(" << &nodeRespondingToClick << ") " << location);
>         ContentChangeObserver::MouseMovedScope observingScope(respondingDocument);
>@@ -677,8 +677,8 @@ void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::F
>             return;
>     }
> 
>-    if (targetNodeisConsideredHidden) {
>-        LOG(ContentObservation, "handleSyntheticClick: target node is considered hidden -> hover.");
>+    if (targetNodeWentFromHiddenToVisible) {
>+        LOG(ContentObservation, "handleSyntheticClick: target node was hidden and now is visible -> hover.");
>         return;
>     }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index fd255d7b7197b8669f5b0ae9e57ecf49b63a2441..97c9ccdae2e6f42e920e802c617ecdab4ca2be9e 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-07-04  Zalan Bujtas  <zalan@apple.com>
>+
>+        [ContentChangeObserver] REGRESSION (r247015): facebook photo/video upload button is unresponsive to user interaction.
>+        https://bugs.webkit.org/show_bug.cgi?id=199502
>+        <rdar://problem/52547473>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3-expected.txt: Added.
>+        * fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3.html: Added.
>+
> 2019-07-04  Zalan Bujtas  <zalan@apple.com>
> 
>         [ContentChangeObserver] Limit mouseOut dispatching after synthetic click to YouTube.com
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..441f214561e3af22be211c98ffab068be217fdb2
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3-expected.txt
>@@ -0,0 +1,3 @@
>+
>+PASS if 'clicked' text is shown below.
>+clicked
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3.html b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e800731d98370cf7c29f305d4ee8726fb3650533
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3.html
>@@ -0,0 +1,40 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<title>This tests the case when the touch target is hidden (hidden at the time of touch start and it does remain hidden throughout the hover heuristics).</title>
>+<script src="../../../../../resources/ui-helper.js"></script>
>+<style>
>+#tapThis {
>+    opacity: 0;
>+    width: 400px;
>+    height: 400px;
>+    border: 1px solid green;
>+}
>+</style>
>+<script>
>+async function test() {
>+    if (!window.testRunner || !testRunner.runUIScript)
>+        return;
>+    if (window.internals)
>+        internals.settings.setContentChangeObserverEnabled(true);
>+
>+    testRunner.waitUntilDone();
>+    testRunner.dumpAsText();
>+
>+    await UIHelper.activateElement(tapThis);
>+}
>+</script>
>+</head>
>+<body onload="test()">
>+<button id=tapThis></button>
>+<div>PASS if 'clicked' text is shown below.</div>
>+<pre id=result></pre>
>+<script>
>+tapThis.addEventListener("click", function( event ) {   
>+    result.innerHTML = "clicked";
>+    if (window.testRunner)
>+        testRunner.notifyDone();
>+}, false);
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 199502: 373470 | 373478 | 373479 | 373487