Attachment 373470 Details for Bug 199502 – Patch

[patch] Patch

bug-199502-20190704131331.patch (text/plain), 6.06 KB, created by zalan on 2019-07-04 13:13:36 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-07-04 13:13:36 PDT

Size: 6.06 KB

patch

obsolete

>Subversion Revision: 247143
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 7954010a062b1a4cbef05c5b34b9897a6cfc5a35..0b92738a9732629834a55ab425cf0d3677c8cb9d 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-03  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         Crash in WebDragClient::startDrag because GraphicsContext is nullptr
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index e173f5e26634227885ef7775903c79bcff69b755..10b9da70524f5f849b5d67d870146765bb1a3b48 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 12a91e384f0ff92798b72dda850044946dfdcbc5..32066b8a3998a738dcaa91f9725985058195ec7c 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-03  Simon Fraser  <simon.fraser@apple.com>
> 
>         Some layers inside overflow:scroll don't move back to correct positions when the contents are shrunk
>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..72f1f97bf196c848bd2343f4db4556bc346c99e1
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3-expected.txt
>@@ -0,0 +1,2 @@
>+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..bbcc9319a67d9e5ef790c381568a4763212e1238
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition3.html
>@@ -0,0 +1,44 @@
>+<!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/basic-gestures.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();
>+
>+    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()">
>+<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