REGRESSION (r242801): [iOS] preventDefault() on touchstart in a subframe does not...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2019 22:07:07 +0000 (22:07 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2019 22:07:07 +0000 (22:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195749
<rdar://problem/48892367>

Reviewed by Tim Horton.

Source/WebKit:

r242801 added logic to fetch interaction information at the touch location upon touch start. However this,
combined with an existing behavior where the process of computing InteractionInformationAtPosition in WebPage
moves focus into the frame of the hit-tested node below the touch location, means that we'll always trigger a
blur event on the window and move focus into the subframe when performing a touch inside a subframe, even if the
page prevents default on touchstart.

To fix this, add a "readonly" flag to InteractionInformationRequest, and only change focus when requesting
position information in the case where the request is not readonly. For now, this readonly flag is false by
default; in a future patch, we should identify the (hopefully few) places that rely on position information
requests to move focus, explicitly turn this bit off in those places, and otherwise send readonly position
information requests by default.

* Shared/ios/InteractionInformationRequest.cpp:
(WebKit::InteractionInformationRequest::encode const):
(WebKit::InteractionInformationRequest::decode):
(WebKit::InteractionInformationRequest::isValidForRequest):
(WebKit::InteractionInformationRequest::isApproximatelyValidForRequest):

Ensure that a readonly request is not valid for a non-readonly request.

* Shared/ios/InteractionInformationRequest.h:
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _requestActivatedElementAtPosition:completionBlock:]):

Send a readonly position information request in the case where a WebKit SPI client is querying for element
information at the given location.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _webTouchEventsRecognized:]):

Send a readonly position information request on touchstart.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::positionInformation):

LayoutTests:

Add a test to verify that tapping a subframe doesn't move focus into it subframe if the page prevents default
on touchstart.

* fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt: Added.
* fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242968 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/Shared/ios/InteractionInformationRequest.cpp
Source/WebKit/Shared/ios/InteractionInformationRequest.h
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index d7ddfbf..98364b1 100644 (file)
@@ -1,3 +1,17 @@
+2019-03-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION (r242801): [iOS] preventDefault() on touchstart in a subframe does not prevent focusing the subframe
+        https://bugs.webkit.org/show_bug.cgi?id=195749
+        <rdar://problem/48892367>
+
+        Reviewed by Tim Horton.
+
+        Add a test to verify that tapping a subframe doesn't move focus into it subframe if the page prevents default
+        on touchstart.
+
+        * fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt: Added.
+        * fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html: Added.
+
 2019-03-14  Shawn Roberts  <sroberts@apple.com>
 
         Unreviewed, rolling out r242931.
diff --git a/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt b/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt
new file mode 100644 (file)
index 0000000..3501937
--- /dev/null
@@ -0,0 +1,11 @@
+Verifies that focus doesn't move into the subframe when preventDefault() is invoked on touchstart. To test manually, tap the button and check that the top level frame does not lose focus.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.activeElement is document.body
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
diff --git a/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html b/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html
new file mode 100644 (file)
index 0000000..3dc579c
--- /dev/null
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../../../resources/js-test.js"></script>
+<script src="../../../../resources/ui-helper.js"></script>
+<style>
+html, body {
+    width: 100%;
+    height: 100%;
+    margin: 0;
+}
+
+iframe {
+    width: 320px;
+    height: 320px;
+}
+</style>
+<script>
+    jsTestIsAsync = true;
+
+    description("Verifies that focus doesn't move into the subframe when preventDefault() is invoked on touchstart. To test manually, tap the button and check that the top level frame does not lose focus.");
+
+    addEventListener("blur", () => testFailed("Top level frame lost focus."));
+    addEventListener("load", async () => {
+        document.body.addEventListener("touchstart", e => e.preventDefault(), { passive: false });
+        if (!window.testRunner || !testRunner.runUIScript)
+            return;
+
+        await UIHelper.activateElement(document.querySelector("iframe"));
+        shouldBe("document.activeElement", "document.body");
+        finishJSTest();
+    });
+</script>
+</head>
+<body>
+    <iframe srcdoc="
+        <style>
+            body, html, button {
+                width: 100%;
+                height: 100%;
+            }
+        </style>
+        <body>
+            <button>Tap here</button>
+        </body>
+        <script>
+            document.body.addEventListener('touchstart', e => e.preventDefault(), { passive: false });
+        </script>
+    "></iframe>
+    <pre id="description"></pre>
+    <pre id="console"></pre>
+</body>
+</html>
index 17b4fed..c14d6a1 100644 (file)
@@ -1,3 +1,46 @@
+2019-03-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION (r242801): [iOS] preventDefault() on touchstart in a subframe does not prevent focusing the subframe
+        https://bugs.webkit.org/show_bug.cgi?id=195749
+        <rdar://problem/48892367>
+
+        Reviewed by Tim Horton.
+
+        r242801 added logic to fetch interaction information at the touch location upon touch start. However this,
+        combined with an existing behavior where the process of computing InteractionInformationAtPosition in WebPage
+        moves focus into the frame of the hit-tested node below the touch location, means that we'll always trigger a
+        blur event on the window and move focus into the subframe when performing a touch inside a subframe, even if the
+        page prevents default on touchstart.
+
+        To fix this, add a "readonly" flag to InteractionInformationRequest, and only change focus when requesting
+        position information in the case where the request is not readonly. For now, this readonly flag is false by
+        default; in a future patch, we should identify the (hopefully few) places that rely on position information
+        requests to move focus, explicitly turn this bit off in those places, and otherwise send readonly position
+        information requests by default.
+
+        * Shared/ios/InteractionInformationRequest.cpp:
+        (WebKit::InteractionInformationRequest::encode const):
+        (WebKit::InteractionInformationRequest::decode):
+        (WebKit::InteractionInformationRequest::isValidForRequest):
+        (WebKit::InteractionInformationRequest::isApproximatelyValidForRequest):
+
+        Ensure that a readonly request is not valid for a non-readonly request.
+
+        * Shared/ios/InteractionInformationRequest.h:
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _requestActivatedElementAtPosition:completionBlock:]):
+
+        Send a readonly position information request in the case where a WebKit SPI client is querying for element
+        information at the given location.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _webTouchEventsRecognized:]):
+
+        Send a readonly position information request on touchstart.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::positionInformation):
+
 2019-03-14  Chris Dumez  <cdumez@apple.com>
 
         Add WebsitePolicy for the client to specify the device orientation & motion access policy
index 5fcfc03..7f1063a 100644 (file)
@@ -38,6 +38,7 @@ void InteractionInformationRequest::encode(IPC::Encoder& encoder) const
     encoder << point;
     encoder << includeSnapshot;
     encoder << includeLinkIndicator;
+    encoder << readonly;
 }
 
 bool InteractionInformationRequest::decode(IPC::Decoder& decoder, InteractionInformationRequest& result)
@@ -51,6 +52,9 @@ bool InteractionInformationRequest::decode(IPC::Decoder& decoder, InteractionInf
     if (!decoder.decode(result.includeLinkIndicator))
         return false;
 
+    if (!decoder.decode(result.readonly))
+        return false;
+
     return true;
 }
 
@@ -65,6 +69,9 @@ bool InteractionInformationRequest::isValidForRequest(const InteractionInformati
     if (other.includeLinkIndicator && !includeLinkIndicator)
         return false;
 
+    if (!other.readonly && readonly)
+        return false;
+
     return true;
 }
     
@@ -75,6 +82,9 @@ bool InteractionInformationRequest::isApproximatelyValidForRequest(const Interac
     
     if (other.includeLinkIndicator && !includeLinkIndicator)
         return false;
+
+    if (!other.readonly && readonly)
+        return false;
     
     return (other.point - point).diagonalLengthSquared() <= 4;
 }
index d274bab..15bbe09 100644 (file)
@@ -42,6 +42,11 @@ struct InteractionInformationRequest {
     bool includeSnapshot { false };
     bool includeLinkIndicator { false };
 
+    // FIXME: This readonly flag ought to be true by default, but there are a number of interactions (e.g. selection) that currently
+    // rely on the fact that interaction information requests additionally change the focused frame. We should explicitly turn the
+    // readonly bit off in these scenarios, and make sure that all other position information requests don't move focus around.
+    bool readonly { false };
+
     InteractionInformationRequest() { }
     explicit InteractionInformationRequest(WebCore::IntPoint point)
     {
index f9dac4a..da03812 100644 (file)
@@ -6577,6 +6577,7 @@ static WebCore::UserInterfaceLayoutDirection toUserInterfaceLayoutDirection(UISe
 {
     auto infoRequest = WebKit::InteractionInformationRequest(WebCore::roundedIntPoint(position));
     infoRequest.includeSnapshot = true;
+    infoRequest.readonly = true;
 
     [_contentView doAfterPositionInformationUpdate:[capturedBlock = makeBlockPtr(block)] (WebKit::InteractionInformationAtPosition information) {
         capturedBlock([_WKActivatedElementInfo activatedElementInfoWithInteractionInformationAtPosition:information]);
index 7fc46a6..cbfcc92 100644 (file)
@@ -1221,9 +1221,11 @@ inline static UIKeyModifierFlags gestureRecognizerModifierFlags(UIGestureRecogni
         [self _handleDOMPasteRequestWithResult:WebCore::DOMPasteAccessResponse::DeniedForGesture];
         _layerTreeTransactionIdAtLastTouchStart = downcast<WebKit::RemoteLayerTreeDrawingAreaProxy>(*_page->drawingArea()).lastCommittedLayerTreeTransactionID();
 
+        WebKit::InteractionInformationRequest positionInformationRequest { WebCore::IntPoint(_lastInteractionLocation) };
+        positionInformationRequest.readonly = true;
         [self doAfterPositionInformationUpdate:[assistant = WeakObjCPtr<WKActionSheetAssistant>(_actionSheetAssistant.get())] (WebKit::InteractionInformationAtPosition information) {
             [assistant interactionDidStartWithPositionInformation:information];
-        } forRequest:WebKit::InteractionInformationRequest(WebCore::IntPoint(_lastInteractionLocation))];
+        } forRequest:positionInformationRequest];
     }
 
 #if ENABLE(TOUCH_EVENTS)
index 84dfc3e..4665a94 100644 (file)
@@ -2376,7 +2376,8 @@ InteractionInformationAtPosition WebPage::positionInformation(const InteractionI
         // Hit test could return HTMLHtmlElement that has no renderer, if the body is smaller than the document.
         if (hitNode && hitNode->renderer()) {
             RenderObject* renderer = hitNode->renderer();
-            m_page->focusController().setFocusedFrame(result.innerNodeFrame());
+            if (!request.readonly)
+                m_page->focusController().setFocusedFrame(result.innerNodeFrame());
             info.bounds = renderer->absoluteBoundingBoxRect(true);
             // We don't want to select blocks that are larger than 97% of the visible area of the document.
             if (is<HTMLAttachmentElement>(*hitNode)) {