Attachment 373454 Details for Bug 185001 – Patch

[patch] Patch

bug-185001-20190703224935.patch (text/plain), 12.28 KB, created by Wenson Hsieh on 2019-07-03 22:49:36 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-07-03 22:49:36 PDT

Size: 12.28 KB

patch

obsolete

>Subversion Revision: 247030
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index c325a15413eabed66046e4987b8688d7b9ac9936..f94edefc73d575a1c74dfb33b170d71b0a2db971 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,47 @@
>+2019-07-03  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        Click events on outer page are not being dispatched correctly after touch-zooming within an iframe
>+        https://bugs.webkit.org/show_bug.cgi?id=185001
>+        <rdar://problem/40569615>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Mitigations introduced in r227759 prevent a touch inside a document of origin A to result in a click event being
>+        dispatched on an element inside a document of origin B. It accomplishes this by keeping track of the security
>+        origin of the last touch via m_potentialTapSecurityOrigin on WebPage. However, there exists a corner case in
>+        which m_potentialTapSecurityOrigin, set after touching a document in a subframe, may persist even after the user
>+        has finished interacting, causing taps in subsequent documents to not result in synthetic click events due to
>+        mismatched potential tap origins.
>+
>+        This may happen if the first user gesture happens inside an element in a subframe with touch event handlers, but
>+        no click event handler (and the touch is additionally not over some clickable element). In this case,
>+        m_potentialTapNode is set to null in WebPage::potentialTapAtPosition, and when we consult it in
>+        WebPage::commitPotentialTap, we just end up calling commitPotentialTapFailed(); and return early. This means
>+        that m_potentialTapNode, m_potentialTapLocation, and (importantly) m_potentialTapSecurityOrigin are not reset.
>+        This causes subsequent taps in the top-level document to never dispatch click events, if the security origin
>+        does not match with that of the subframe.
>+
>+        To fix this, we add a new async IPC message from the UI process to the web process to ensure that our current
>+        security origin is reset before attempting to handle a tap.
>+
>+        Test: http/tests/events/touch/ios/click-after-handling-touch-in-cross-origin-frame.https.html
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::resetPotentialTapSecurityOrigin):
>+
>+        Reset any stale potential tap security origin if needed.
>+
>+        * UIProcess/WebPageProxy.h:
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView _webTouchEventsRecognized:]):
>+
>+        Send the new IPC message when starting a touch.
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::resetPotentialTapSecurityOrigin):
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/WebPage.messages.in:
>+
> 2019-07-01  Chris Dumez  <cdumez@apple.com>
> 
>         Remove virtual functions on WebProcessLifetimeObserver that are unused after r245540
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 18ffde722c55a15e9cb13e9a545e5f85b7b9f394..f9f8eaa3eebe3ea9244109fe02316afa05ef11a7 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -2669,6 +2669,14 @@ void WebPageProxy::handleTouchEventSynchronously(NativeWebTouchEvent& event)
>         m_touchAndPointerEventTracking.reset();
> }
> 
>+void WebPageProxy::resetPotentialTapSecurityOrigin()
>+{
>+    if (!hasRunningProcess())
>+        return;
>+
>+    m_process->send(Messages::WebPage::ResetPotentialTapSecurityOrigin(), m_pageID);
>+}
>+
> void WebPageProxy::handleTouchEventAsynchronously(const NativeWebTouchEvent& event)
> {
>     if (!hasRunningProcess())
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index a19ee09649d4a737e889b2765b6c834d13b28a96..aa257d46aef9b4208d5c98e0e58dcbeeb652937f 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -831,6 +831,7 @@ public:
> #endif
> 
> #if ENABLE(IOS_TOUCH_EVENTS)
>+    void resetPotentialTapSecurityOrigin();
>     void handleTouchEventSynchronously(NativeWebTouchEvent&);
>     void handleTouchEventAsynchronously(const NativeWebTouchEvent&);
> 
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index 0fc3e4368868f4af0c8fcb45026b4af9d9458b5c..b6e66d5e0cabfa99a4644b136bf828ff877aa512 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -1315,6 +1315,8 @@ - (void)_webTouchEventsRecognized:(UIWebTouchEventsGestureRecognizer *)gestureRe
>         [self _handleDOMPasteRequestWithResult:WebCore::DOMPasteAccessResponse::DeniedForGesture];
>         _layerTreeTransactionIdAtLastTouchStart = downcast<WebKit::RemoteLayerTreeDrawingAreaProxy>(*_page->drawingArea()).lastCommittedLayerTreeTransactionID();
> 
>+        _page->resetPotentialTapSecurityOrigin();
>+
>         WebKit::InteractionInformationRequest positionInformationRequest { WebCore::IntPoint(_lastInteractionLocation) };
>         [self doAfterPositionInformationUpdate:[assistant = WeakObjCPtr<WKActionSheetAssistant>(_actionSheetAssistant.get())] (WebKit::InteractionInformationAtPosition information) {
>             [assistant interactionDidStartWithPositionInformation:information];
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 3bcf0d785fff52da17b106ebac8f6fd0dd2a4b72..3a73eb827b93d48b544bc8644a319b9793f5e0a1 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -2916,6 +2916,11 @@ void WebPage::touchEventSync(const WebTouchEvent& touchEvent, CompletionHandler<
>         reply(handled);
> }
> 
>+void WebPage::resetPotentialTapSecurityOrigin()
>+{
>+    m_potentialTapSecurityOrigin = nullptr;
>+}
>+
> void WebPage::updatePotentialTapSecurityOrigin(const WebTouchEvent& touchEvent, bool wasHandled)
> {
>     if (wasHandled)
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index 7d79a78afa38ccf8cca392b1cfc29b730eb6151d..7485534e27be0ca9bcf552449de4b018387f19e1 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -1320,6 +1320,7 @@ private:
> 
> #if ENABLE(IOS_TOUCH_EVENTS)
>     void touchEventSync(const WebTouchEvent&, CompletionHandler<void(bool)>&&);
>+    void resetPotentialTapSecurityOrigin();
>     void updatePotentialTapSecurityOrigin(const WebTouchEvent&, bool wasHandled);
> #elif ENABLE(TOUCH_EVENTS)
>     void touchEvent(const WebTouchEvent&);
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>index dad2d5e0585c0cad906244971089b68bf7a97982..f31ee1d64a3e12d238795618cb9a463eab3c3e3b 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>@@ -126,6 +126,7 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
> #endif
> 
> #if ENABLE(IOS_TOUCH_EVENTS)
>+    ResetPotentialTapSecurityOrigin()
>     TouchEventSync(WebKit::WebTouchEvent event) -> (bool handled) Synchronous
> #endif
> #if !ENABLE(IOS_TOUCH_EVENTS) && ENABLE(TOUCH_EVENTS)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 439554a1246a8d0ca4d224742d7e5f9c9881b17f..266b35916c6ede73a00c57fb744b5c918fb98cd0 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-07-03  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        Click events on outer page are not being dispatched correctly after touch-zooming within an iframe
>+        https://bugs.webkit.org/show_bug.cgi?id=185001
>+        <rdar://problem/40569615>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a new layout test to verify that after interacting with an element with touch event handlers in a cross-
>+        origin subframe, the user is still able to click on elements on the top level document.
>+
>+        * http/tests/events/touch/ios/click-after-handling-touch-in-cross-origin-frame.https-expected.txt: Added.
>+        * http/tests/events/touch/ios/click-after-handling-touch-in-cross-origin-frame.https.html: Added.
>+        * http/tests/events/touch/ios/resources/touch-target.html: Added.
>+
> 2019-07-03  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         REGRESSION (iOS 13): Tapping an element with a click event handler no longer clears the selection
>diff --git a/LayoutTests/http/tests/events/touch/ios/click-after-handling-touch-in-cross-origin-frame.https-expected.txt b/LayoutTests/http/tests/events/touch/ios/click-after-handling-touch-in-cross-origin-frame.https-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..907e67ccafed1fbaf80349df02e4259e62427b85
>--- /dev/null
>+++ b/LayoutTests/http/tests/events/touch/ios/click-after-handling-touch-in-cross-origin-frame.https-expected.txt
>@@ -0,0 +1,14 @@
>+This test verifies that tap in the top document is recognized as a click after touching inside a cross-origin subframe. To manually test, touch anywhere in the top iframe, and then tap the green box on the bottom. The text 'Clicked' should appear.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS touchend
>+PASS Clicked
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>+Then click here.
>+Clicked
>+
>diff --git a/LayoutTests/http/tests/events/touch/ios/click-after-handling-touch-in-cross-origin-frame.https.html b/LayoutTests/http/tests/events/touch/ios/click-after-handling-touch-in-cross-origin-frame.https.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..5462e148a43938599ddb14e2bc9a21a3b11473ae
>--- /dev/null
>+++ b/LayoutTests/http/tests/events/touch/ios/click-after-handling-touch-in-cross-origin-frame.https.html
>@@ -0,0 +1,57 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
>+    <script src="/js-test-resources/ui-helper.js"></script>
>+    <script src="/js-test-resources/js-test.js"></script>
>+    <style>
>+        iframe, #target {
>+            width: 300px;
>+            height: 150px;
>+        }
>+
>+        #target {
>+            background-color: green;
>+            color: white;
>+        }
>+
>+        #result {
>+            color: green;
>+        }
>+    </style>
>+    <script>
>+        jsTestIsAsync = true;
>+        progress = 0;
>+
>+        function checkProgress() {
>+            if (++progress == 2)
>+                finishJSTest();
>+        }
>+
>+        description("This test verifies that tap in the top document is recognized as a click after touching inside a cross-origin subframe. To manually test, touch anywhere in the top iframe, and then tap the green box on the bottom. The text 'Clicked' should appear.")
>+
>+        addEventListener("load", async event => {
>+            addEventListener("message", message => testPassed(message.data));
>+            document.getElementById("target").addEventListener("click", () => {
>+                document.getElementById("result").textContent = "Clicked";
>+                testPassed("Clicked");
>+                checkProgress();
>+            });
>+
>+            if (!window.testRunner)
>+                return;
>+
>+            await UIHelper.activateElement(document.querySelector("iframe"));
>+            await UIHelper.activateElement(document.getElementById("target"));
>+            checkProgress();
>+        });
>+    </script>
>+</head>
>+<body>
>+    <p id="description"></p>
>+    <iframe src="https://localhost:8443/events/touch/ios/resources/touch-target.html"></iframe>
>+    <div id="target">Then click here.</div>
>+    <pre id="result"></pre>
>+    <pre id="console"></pre>
>+</body>
>+</html>
>\ No newline at end of file
>diff --git a/LayoutTests/http/tests/events/touch/ios/resources/touch-target.html b/LayoutTests/http/tests/events/touch/ios/resources/touch-target.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..9452528ada1e02a45fdcf40251212933c86a0a47
>--- /dev/null
>+++ b/LayoutTests/http/tests/events/touch/ios/resources/touch-target.html
>@@ -0,0 +1,15 @@
>+<!DOCTYPE html>
>+<style>
>+    body {
>+        margin: 0;
>+    }
>+
>+    div {
>+        width: 100vw;
>+        height: 100vh;
>+    }
>+</style>
>+<div id="target">Touch this element.</div>
>+<script>
>+document.getElementById("target").addEventListener("touchend", () => parent.postMessage("touchend", "*"));
>+</script>

Actions: View | Formatted Diff | Diff

Attachments on bug 185001: 338799 | 343530 | 343531 | 371254 | 373454 | 373455