- Revision
- 256821
- Author
- [email protected]
- Date
- 2020-02-17 23:38:10 -0800 (Mon, 17 Feb 2020)
Log Message
Selection cannot be modified via text interaction in some areas of the compose body field in Gmail
https://bugs.webkit.org/show_bug.cgi?id=207854
<rdar://problem/59218824>
Reviewed by Tim Horton.
Source/WebCore:
Currently, several codepaths that are consulted when performing text interaction gestures on iOS to make and
modify text selections (e.g. double taps and long presses) rely on Frame::visiblePositionForPoint to map the
touch location to a visible editing position on the page. visiblePositionForPoint uses the hit-testing option
AllowChildFrameContent, which allows it to find and select text within subframes. However, this option also
includes content in hidden ("visibility: hidden;") frames. This means that text selection gestures on iOS are
unable to find otherwise selectable text, if a hidden child frame overlaps the text.
To fix this, we instead use the AllowVisibleChildFrameContentOnly hit-testing option, which will ignore such
hidden subframes.
Test: editing/selection/ios/select-text-under-hidden-subframe.html
* page/Frame.cpp:
(WebCore::Frame::visiblePositionForPoint const):
Source/WebKit:
Adjusts more call sites in WebKit to ignore child frames that are hidden when hit-testing. See WebCore/ChangeLog
for more details.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::rectForElementAtInteractionLocation const):
(WebKit::WebPage::handleStylusSingleTapAtPoint):
(WebKit::rangeForPointInRootViewCoordinates):
(WebKit::WebPage::setFocusedFrameBeforeSelectingTextAtLocation):
(WebKit::selectionPositionInformation):
(WebKit::textInteractionPositionInformation):
(WebKit::WebPage::positionInformation):
LayoutTests:
Add a new layout test to verify that the hit-testing codepaths exercised by text interaction gestures on iOS
successfully ignore a hidden child frame that covers the page.
* editing/selection/ios/select-text-under-hidden-subframe-expected.txt: Added.
* editing/selection/ios/select-text-under-hidden-subframe.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (256820 => 256821)
--- trunk/LayoutTests/ChangeLog 2020-02-18 06:32:42 UTC (rev 256820)
+++ trunk/LayoutTests/ChangeLog 2020-02-18 07:38:10 UTC (rev 256821)
@@ -1,3 +1,17 @@
+2020-02-17 Wenson Hsieh <[email protected]>
+
+ Selection cannot be modified via text interaction in some areas of the compose body field in Gmail
+ https://bugs.webkit.org/show_bug.cgi?id=207854
+ <rdar://problem/59218824>
+
+ Reviewed by Tim Horton.
+
+ Add a new layout test to verify that the hit-testing codepaths exercised by text interaction gestures on iOS
+ successfully ignore a hidden child frame that covers the page.
+
+ * editing/selection/ios/select-text-under-hidden-subframe-expected.txt: Added.
+ * editing/selection/ios/select-text-under-hidden-subframe.html: Added.
+
2020-02-17 Chris Dumez <[email protected]>
[WK2][Cocoa] Implement in-WebProcess cookie cache to avoid sync IPC for document.cookie in most cases
Added: trunk/LayoutTests/editing/selection/ios/select-text-under-hidden-subframe-expected.txt (0 => 256821)
--- trunk/LayoutTests/editing/selection/ios/select-text-under-hidden-subframe-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/select-text-under-hidden-subframe-expected.txt 2020-02-18 07:38:10 UTC (rev 256821)
@@ -0,0 +1,13 @@
+Verifies that text can be selected using text interaction gestures, even when it is fully covered by a hidden subframe. To manually run the test, tap to focus and double tap again to select 'Banana'; then long press to select 'Apple'.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getSelection().toString() became 'Banana'
+PASS getSelection().toString() became 'Apple'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Apple
+Banana
+
Added: trunk/LayoutTests/editing/selection/ios/select-text-under-hidden-subframe.html (0 => 256821)
--- trunk/LayoutTests/editing/selection/ios/select-text-under-hidden-subframe.html (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/select-text-under-hidden-subframe.html 2020-02-18 07:38:10 UTC (rev 256821)
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <meta name="viewport" content="width=device-width, initial-scale=1">
+ <script src=""
+ <script src=""
+ <style>
+ .target {
+ font-size: 32px;
+ width: 300px;
+ text-align: center;
+ }
+
+ .target-container {
+ margin-bottom: 1em;
+ }
+
+ iframe {
+ visibility: hidden;
+ position: fixed;
+ top: 0;
+ left: 0;
+ width: 100%;
+ height: 100%;
+ z-index: 100;
+ }
+ </style>
+ <script>
+ jsTestIsAsync = true;
+ description("Verifies that text can be selected using text interaction gestures, even when it is fully covered by a hidden subframe. To manually run the test, tap to focus and double tap again to select 'Banana'; then long press to select 'Apple'.");
+
+ addEventListener("load", async () => {
+ const topTarget = document.getElementById("top");
+ const bottomTarget = document.getElementById("bottom");
+ const bottomTargetRect = bottomTarget.getBoundingClientRect();
+
+ await UIHelper.activateElementAndWaitForInputSession(bottomTarget);
+ await UIHelper.doubleTapAt(bottomTargetRect.left + bottomTargetRect.width / 2, bottomTargetRect.top + bottomTargetRect.height / 2);
+ await new Promise(resolve => shouldBecomeEqual("getSelection().toString()", "'Banana'", resolve));
+
+ document.activeElement.blur();
+ await UIHelper.waitForKeyboardToHide();
+ await UIHelper.waitForDoubleTapDelay();
+ await UIHelper.longPressElement(topTarget);
+ await new Promise(resolve => shouldBecomeEqual("getSelection().toString()", "'Apple'", resolve));
+
+ finishJSTest();
+ });
+ </script>
+</head>
+<body>
+ <div class="target-container"><span class="target" id="top">Apple</span></div>
+ <div class="target-container" contenteditable><span class="target" id="bottom">Banana</span></div>
+ <iframe srcdoc="<body style='font-size: 100px; -webkit-user-select: none;'>The quick brown fox jumped over the lazy dog!</body>"></iframe>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (256820 => 256821)
--- trunk/Source/WebCore/ChangeLog 2020-02-18 06:32:42 UTC (rev 256820)
+++ trunk/Source/WebCore/ChangeLog 2020-02-18 07:38:10 UTC (rev 256821)
@@ -1,3 +1,26 @@
+2020-02-17 Wenson Hsieh <[email protected]>
+
+ Selection cannot be modified via text interaction in some areas of the compose body field in Gmail
+ https://bugs.webkit.org/show_bug.cgi?id=207854
+ <rdar://problem/59218824>
+
+ Reviewed by Tim Horton.
+
+ Currently, several codepaths that are consulted when performing text interaction gestures on iOS to make and
+ modify text selections (e.g. double taps and long presses) rely on Frame::visiblePositionForPoint to map the
+ touch location to a visible editing position on the page. visiblePositionForPoint uses the hit-testing option
+ AllowChildFrameContent, which allows it to find and select text within subframes. However, this option also
+ includes content in hidden ("visibility: hidden;") frames. This means that text selection gestures on iOS are
+ unable to find otherwise selectable text, if a hidden child frame overlaps the text.
+
+ To fix this, we instead use the AllowVisibleChildFrameContentOnly hit-testing option, which will ignore such
+ hidden subframes.
+
+ Test: editing/selection/ios/select-text-under-hidden-subframe.html
+
+ * page/Frame.cpp:
+ (WebCore::Frame::visiblePositionForPoint const):
+
2020-02-17 Chris Dumez <[email protected]>
[WK2][Cocoa] Implement in-WebProcess cookie cache to avoid sync IPC for document.cookie in most cases
Modified: trunk/Source/WebCore/page/Frame.cpp (256820 => 256821)
--- trunk/Source/WebCore/page/Frame.cpp 2020-02-18 06:32:42 UTC (rev 256820)
+++ trunk/Source/WebCore/page/Frame.cpp 2020-02-18 07:38:10 UTC (rev 256821)
@@ -736,7 +736,7 @@
VisiblePosition Frame::visiblePositionForPoint(const IntPoint& framePoint) const
{
- HitTestResult result = eventHandler().hitTestResultAtPoint(framePoint, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent);
+ HitTestResult result = eventHandler().hitTestResultAtPoint(framePoint, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowVisibleChildFrameContentOnly);
Node* node = result.innerNonSharedNode();
if (!node)
return VisiblePosition();
Modified: trunk/Source/WebKit/ChangeLog (256820 => 256821)
--- trunk/Source/WebKit/ChangeLog 2020-02-18 06:32:42 UTC (rev 256820)
+++ trunk/Source/WebKit/ChangeLog 2020-02-18 07:38:10 UTC (rev 256821)
@@ -1,3 +1,23 @@
+2020-02-17 Wenson Hsieh <[email protected]>
+
+ Selection cannot be modified via text interaction in some areas of the compose body field in Gmail
+ https://bugs.webkit.org/show_bug.cgi?id=207854
+ <rdar://problem/59218824>
+
+ Reviewed by Tim Horton.
+
+ Adjusts more call sites in WebKit to ignore child frames that are hidden when hit-testing. See WebCore/ChangeLog
+ for more details.
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::rectForElementAtInteractionLocation const):
+ (WebKit::WebPage::handleStylusSingleTapAtPoint):
+ (WebKit::rangeForPointInRootViewCoordinates):
+ (WebKit::WebPage::setFocusedFrameBeforeSelectingTextAtLocation):
+ (WebKit::selectionPositionInformation):
+ (WebKit::textInteractionPositionInformation):
+ (WebKit::WebPage::positionInformation):
+
2020-02-17 Chris Dumez <[email protected]>
[WK2][Cocoa] Implement in-WebProcess cookie cache to avoid sync IPC for document.cookie in most cases
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (256820 => 256821)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2020-02-18 06:32:42 UTC (rev 256820)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2020-02-18 07:38:10 UTC (rev 256821)
@@ -576,7 +576,7 @@
IntRect WebPage::rectForElementAtInteractionLocation() const
{
- HitTestResult result = m_page->mainFrame().eventHandler().hitTestResultAtPoint(m_lastInteractionLocation, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent);
+ HitTestResult result = m_page->mainFrame().eventHandler().hitTestResultAtPoint(m_lastInteractionLocation, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowVisibleChildFrameContentOnly);
Node* hitNode = result.innerNode();
if (!hitNode || !hitNode->renderer())
return IntRect();
@@ -1031,7 +1031,7 @@
auto& frame = m_page->focusController().focusedOrMainFrame();
auto pointInDocument = frame.view()->rootViewToContents(point);
- HitTestResult hitTest = frame.eventHandler().hitTestResultAtPoint(pointInDocument, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent);
+ HitTestResult hitTest = frame.eventHandler().hitTestResultAtPoint(pointInDocument, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowVisibleChildFrameContentOnly);
Node* node = hitTest.innerNonSharedNode();
if (!node)
@@ -1473,7 +1473,7 @@
VisiblePosition result;
RefPtr<Range> range;
- HitTestResult hitTest = frame.eventHandler().hitTestResultAtPoint(pointInDocument, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent);
+ HitTestResult hitTest = frame.eventHandler().hitTestResultAtPoint(pointInDocument, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowVisibleChildFrameContentOnly);
if (hitTest.targetNode())
result = frame.eventHandler().selectionExtentRespectingEditingBoundary(frame.selection().selection(), hitTest.localPoint(), hitTest.targetNode()).deepEquivalent();
else
@@ -2081,7 +2081,7 @@
void WebPage::setFocusedFrameBeforeSelectingTextAtLocation(const IntPoint& point)
{
- auto result = m_page->mainFrame().eventHandler().hitTestResultAtPoint(point, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowUserAgentShadowContent | HitTestRequest::AllowChildFrameContent);
+ auto result = m_page->mainFrame().eventHandler().hitTestResultAtPoint(point, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowUserAgentShadowContent | HitTestRequest::AllowVisibleChildFrameContentOnly);
auto* hitNode = result.innerNode();
if (hitNode && hitNode->renderer())
m_page->focusController().setFocusedFrame(result.innerNodeFrame());
@@ -2714,7 +2714,7 @@
static void selectionPositionInformation(WebPage& page, const InteractionInformationRequest& request, InteractionInformationAtPosition& info)
{
- HitTestResult result = page.corePage()->mainFrame().eventHandler().hitTestResultAtPoint(request.point, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowUserAgentShadowContent | HitTestRequest::AllowChildFrameContent);
+ HitTestResult result = page.corePage()->mainFrame().eventHandler().hitTestResultAtPoint(request.point, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowUserAgentShadowContent | HitTestRequest::AllowVisibleChildFrameContentOnly);
Node* hitNode = result.innerNode();
// Hit test could return HTMLHtmlElement that has no renderer, if the body is smaller than the document.
@@ -2761,7 +2761,7 @@
if (!input.list())
return;
- HitTestResult result = page.corePage()->mainFrame().eventHandler().hitTestResultAtPoint(request.point, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent);
+ HitTestResult result = page.corePage()->mainFrame().eventHandler().hitTestResultAtPoint(request.point, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowVisibleChildFrameContentOnly);
if (result.innerNode() == input.dataListButtonElement())
info.preventTextInteraction = true;
}
@@ -2876,7 +2876,7 @@
info.nodeAtPositionHasDoubleClickHandler = m_page->mainFrame().nodeRespondingToDoubleClickEvent(request.point, adjustedPoint);
auto& eventHandler = m_page->mainFrame().eventHandler();
- HitTestResult hitTestResultForCursor = eventHandler.hitTestResultAtPoint(request.point, HitTestRequest::ReadOnly | HitTestRequest::AllowFrameScrollbars | HitTestRequest::AllowChildFrameContent);
+ HitTestResult hitTestResultForCursor = eventHandler.hitTestResultAtPoint(request.point, HitTestRequest::ReadOnly | HitTestRequest::AllowFrameScrollbars | HitTestRequest::AllowVisibleChildFrameContentOnly);
if (auto* hitFrame = hitTestResultForCursor.innerNodeFrame()) {
info.cursor = hitFrame->eventHandler().selectCursor(hitTestResultForCursor, false);
if (request.includeCaretContext)
