- Revision
- 242968
- Author
- [email protected]
- Date
- 2019-03-14 15:07:07 -0700 (Thu, 14 Mar 2019)
Log Message
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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (242967 => 242968)
--- trunk/LayoutTests/ChangeLog 2019-03-14 22:06:17 UTC (rev 242967)
+++ trunk/LayoutTests/ChangeLog 2019-03-14 22:07:07 UTC (rev 242968)
@@ -1,3 +1,17 @@
+2019-03-14 Wenson Hsieh <[email protected]>
+
+ 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 <[email protected]>
Unreviewed, rolling out r242931.
Added: trunk/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt (0 => 242968)
--- trunk/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt 2019-03-14 22:07:07 UTC (rev 242968)
@@ -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
+
+
Added: trunk/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html (0 => 242968)
--- trunk/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html 2019-03-14 22:07:07 UTC (rev 242968)
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script src=""
+<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>
Modified: trunk/Source/WebKit/ChangeLog (242967 => 242968)
--- trunk/Source/WebKit/ChangeLog 2019-03-14 22:06:17 UTC (rev 242967)
+++ trunk/Source/WebKit/ChangeLog 2019-03-14 22:07:07 UTC (rev 242968)
@@ -1,3 +1,46 @@
+2019-03-14 Wenson Hsieh <[email protected]>
+
+ 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 <[email protected]>
Add WebsitePolicy for the client to specify the device orientation & motion access policy
Modified: trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp (242967 => 242968)
--- trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp 2019-03-14 22:06:17 UTC (rev 242967)
+++ trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp 2019-03-14 22:07:07 UTC (rev 242968)
@@ -38,6 +38,7 @@
encoder << point;
encoder << includeSnapshot;
encoder << includeLinkIndicator;
+ encoder << readonly;
}
bool InteractionInformationRequest::decode(IPC::Decoder& decoder, InteractionInformationRequest& result)
@@ -51,6 +52,9 @@
if (!decoder.decode(result.includeLinkIndicator))
return false;
+ if (!decoder.decode(result.readonly))
+ return false;
+
return true;
}
@@ -65,6 +69,9 @@
if (other.includeLinkIndicator && !includeLinkIndicator)
return false;
+ if (!other.readonly && readonly)
+ return false;
+
return true;
}
@@ -75,6 +82,9 @@
if (other.includeLinkIndicator && !includeLinkIndicator)
return false;
+
+ if (!other.readonly && readonly)
+ return false;
return (other.point - point).diagonalLengthSquared() <= 4;
}
Modified: trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.h (242967 => 242968)
--- trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.h 2019-03-14 22:06:17 UTC (rev 242967)
+++ trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.h 2019-03-14 22:07:07 UTC (rev 242968)
@@ -42,6 +42,11 @@
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)
{
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (242967 => 242968)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2019-03-14 22:06:17 UTC (rev 242967)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2019-03-14 22:07:07 UTC (rev 242968)
@@ -6577,6 +6577,7 @@
{
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]);
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (242967 => 242968)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2019-03-14 22:06:17 UTC (rev 242967)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2019-03-14 22:07:07 UTC (rev 242968)
@@ -1221,9 +1221,11 @@
[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)
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (242967 => 242968)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-03-14 22:06:17 UTC (rev 242967)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-03-14 22:07:07 UTC (rev 242968)
@@ -2376,7 +2376,8 @@
// 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)) {
