- Revision
- 278633
- Author
- [email protected]
- Date
- 2021-06-08 16:30:21 -0700 (Tue, 08 Jun 2021)
Log Message
[iOS] Safari tab pill should toggle visibility when tapping on article text on adventure.com
https://bugs.webkit.org/show_bug.cgi?id=226775
rdar://78826820
Reviewed by Tim Horton and Devin Rousso.
Source/WebKit:
Adjust the meaningful click heuristic to account for click event listeners added to the document node. See below
for more details.
Test: fast/events/ios/non-meaningful-click-when-tapping-document.html
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::getPlatformEditorState const):
(WebKit::isProbablyMeaningfulClick):
Remove a check for whether or not the clicked node is an Element; this existed because the call to
`rootViewBoundsForElement` below takes an Element rather than just a Node; however, this method doesn't do
anything that requires an Element instead of a Node, so we can just remove the check and refactor these static
methods to accept Nodes. This allows us to bail early if the clicked node is *either* the body or the document
node, instead of just the body.
(WebKit::WebPage::insertDroppedImagePlaceholders):
(WebKit::elementBoundsInFrame):
(WebKit::WebPage::rootViewBounds):
(WebKit::WebPage::absoluteInteractionBounds):
(WebKit::WebPage::rootViewInteractionBounds):
Drive-by refactoring: drop the `-ForElement` suffixes on these helper methods, and additionally make them accept
a Node instead of requiring an Element. This allows us to remove the `is<Element>()` check from the meaningful
click heuristic above.
Also deploy RefPtr in a few more places.
(WebKit::WebPage::dispatchSyntheticMouseEventsForSelectionGesture):
(WebKit::WebPage::focusedElementInformation):
(WebKit::WebPage::rootViewBoundsForElement): Deleted.
(WebKit::WebPage::absoluteInteractionBoundsForElement): Deleted.
(WebKit::WebPage::rootViewInteractionBoundsForElement): Deleted.
LayoutTests:
* fast/events/ios/non-meaningful-click-when-tapping-document-expected.txt: Added.
* fast/events/ios/non-meaningful-click-when-tapping-document.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (278632 => 278633)
--- trunk/LayoutTests/ChangeLog 2021-06-08 22:48:59 UTC (rev 278632)
+++ trunk/LayoutTests/ChangeLog 2021-06-08 23:30:21 UTC (rev 278633)
@@ -1,3 +1,14 @@
+2021-06-08 Wenson Hsieh <[email protected]>
+
+ [iOS] Safari tab pill should toggle visibility when tapping on article text on adventure.com
+ https://bugs.webkit.org/show_bug.cgi?id=226775
+ rdar://78826820
+
+ Reviewed by Tim Horton and Devin Rousso.
+
+ * fast/events/ios/non-meaningful-click-when-tapping-document-expected.txt: Added.
+ * fast/events/ios/non-meaningful-click-when-tapping-document.html: Added.
+
2021-06-08 Diego Pino Garcia <[email protected]>
[GTK] Unreviewed test gardening. Update GTK baselines of several ARIA tests.
Added: trunk/LayoutTests/fast/events/ios/non-meaningful-click-when-tapping-document-expected.txt (0 => 278633)
--- trunk/LayoutTests/fast/events/ios/non-meaningful-click-when-tapping-document-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/ios/non-meaningful-click-when-tapping-document-expected.txt 2021-06-08 23:30:21 UTC (rev 278633)
@@ -0,0 +1,10 @@
+This test exercises the 'meaningful click' heuristic when dispatching clicks on the document node, and requires WebKitTestRunner.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS didDispatchClick became true
+PASS didDispatchNonMeaningfulClickCallback became true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/events/ios/non-meaningful-click-when-tapping-document.html (0 => 278633)
--- trunk/LayoutTests/fast/events/ios/non-meaningful-click-when-tapping-document.html (rev 0)
+++ trunk/LayoutTests/fast/events/ios/non-meaningful-click-when-tapping-document.html 2021-06-08 23:30:21 UTC (rev 278633)
@@ -0,0 +1,45 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<style>
+body, html {
+ width: 100%;
+ height: 100%;
+}
+</style>
+<script src=""
+<script src=""
+<script>
+jsTestIsAsync = true;
+
+addEventListener("load", async () => {
+ description("This test exercises the 'meaningful click' heuristic when dispatching clicks on the document node, and requires WebKitTestRunner.");
+ if (!window.testRunner)
+ return;
+
+ didDispatchNonMeaningfulClickCallback = false;
+ testRunner.installDidNotHandleTapAsMeaningfulClickCallback(() => {
+ didDispatchNonMeaningfulClickCallback = true;
+ });
+
+ didDispatchClick = false;
+ document.addEventListener("click", () => {
+ didDispatchClick = true;
+ });
+
+ await UIHelper.activateAt(100, 100);
+ await UIHelper.waitForDoubleTapDelay();
+ await new Promise(resolve => shouldBecomeEqual("didDispatchClick", "true", resolve));
+ await new Promise(resolve => shouldBecomeEqual("didDispatchNonMeaningfulClickCallback", "true", resolve));
+
+ testRunner.clearTestRunnerCallbacks();
+ finishJSTest();
+});
+</script>
+</head>
+<body>
+ <div id="description"></div>
+ <div id="console"></div>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebKit/ChangeLog (278632 => 278633)
--- trunk/Source/WebKit/ChangeLog 2021-06-08 22:48:59 UTC (rev 278632)
+++ trunk/Source/WebKit/ChangeLog 2021-06-08 23:30:21 UTC (rev 278633)
@@ -1,3 +1,45 @@
+2021-06-08 Wenson Hsieh <[email protected]>
+
+ [iOS] Safari tab pill should toggle visibility when tapping on article text on adventure.com
+ https://bugs.webkit.org/show_bug.cgi?id=226775
+ rdar://78826820
+
+ Reviewed by Tim Horton and Devin Rousso.
+
+ Adjust the meaningful click heuristic to account for click event listeners added to the document node. See below
+ for more details.
+
+ Test: fast/events/ios/non-meaningful-click-when-tapping-document.html
+
+ * WebProcess/WebPage/WebPage.h:
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::getPlatformEditorState const):
+ (WebKit::isProbablyMeaningfulClick):
+
+ Remove a check for whether or not the clicked node is an Element; this existed because the call to
+ `rootViewBoundsForElement` below takes an Element rather than just a Node; however, this method doesn't do
+ anything that requires an Element instead of a Node, so we can just remove the check and refactor these static
+ methods to accept Nodes. This allows us to bail early if the clicked node is *either* the body or the document
+ node, instead of just the body.
+
+ (WebKit::WebPage::insertDroppedImagePlaceholders):
+ (WebKit::elementBoundsInFrame):
+ (WebKit::WebPage::rootViewBounds):
+ (WebKit::WebPage::absoluteInteractionBounds):
+ (WebKit::WebPage::rootViewInteractionBounds):
+
+ Drive-by refactoring: drop the `-ForElement` suffixes on these helper methods, and additionally make them accept
+ a Node instead of requiring an Element. This allows us to remove the `is<Element>()` check from the meaningful
+ click heuristic above.
+
+ Also deploy RefPtr in a few more places.
+
+ (WebKit::WebPage::dispatchSyntheticMouseEventsForSelectionGesture):
+ (WebKit::WebPage::focusedElementInformation):
+ (WebKit::WebPage::rootViewBoundsForElement): Deleted.
+ (WebKit::WebPage::absoluteInteractionBoundsForElement): Deleted.
+ (WebKit::WebPage::rootViewInteractionBoundsForElement): Deleted.
+
2021-06-08 Devin Rousso <[email protected]>
[Payment Request] upstream new features
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (278632 => 278633)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-06-08 22:48:59 UTC (rev 278632)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-06-08 23:30:21 UTC (rev 278633)
@@ -1356,10 +1356,10 @@
#if PLATFORM(IOS_FAMILY)
// This excludes layout overflow, includes borders.
- static WebCore::IntRect rootViewBoundsForElement(const WebCore::Element&);
+ static WebCore::IntRect rootViewBounds(const WebCore::Node&);
// These include layout overflow for overflow:visible elements, but exclude borders.
- static WebCore::IntRect absoluteInteractionBoundsForElement(const WebCore::Element&);
- static WebCore::IntRect rootViewInteractionBoundsForElement(const WebCore::Element&);
+ static WebCore::IntRect absoluteInteractionBounds(const WebCore::Node&);
+ static WebCore::IntRect rootViewInteractionBounds(const WebCore::Node&);
InteractionInformationAtPosition positionInformation(const InteractionInformationRequest&);
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (278632 => 278633)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-06-08 22:48:59 UTC (rev 278632)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-06-08 23:30:21 UTC (rev 278633)
@@ -344,7 +344,7 @@
}
if (auto editableRootOrFormControl = makeRefPtr(enclosingTextFormControl(selection.start()) ?: selection.rootEditableElement())) {
- postLayoutData.selectionClipRect = rootViewInteractionBoundsForElement(*editableRootOrFormControl);
+ postLayoutData.selectionClipRect = rootViewInteractionBounds(*editableRootOrFormControl);
postLayoutData.editableRootIsTransparentOrFullyClipped = result.isContentEditable && isTransparentOrFullyClipped(*editableRootOrFormControl);
}
computeEditableRootHasContentAndPlainText(selection, postLayoutData);
@@ -839,14 +839,14 @@
static bool isProbablyMeaningfulClick(Node& clickNode)
{
auto frame = makeRefPtr(clickNode.document().frame());
- if (!is<Element>(clickNode) || !clickNode.isConnected() || !frame)
+ if (!frame || !clickNode.isConnected())
return true;
- if (is<HTMLBodyElement>(clickNode))
+ if (is<HTMLBodyElement>(clickNode) || is<Document>(clickNode) || clickNode.document().documentElement() == &clickNode)
return false;
if (auto view = makeRefPtr(frame->mainFrame().view())) {
- auto elementBounds = WebPage::rootViewBoundsForElement(downcast<Element>(clickNode));
+ auto elementBounds = WebPage::rootViewInteractionBounds(clickNode);
auto unobscuredRect = view->unobscuredContentRect();
if (elementBounds.width() >= unobscuredRect.width() / 2 && elementBounds.height() >= unobscuredRect.height() / 2)
return false;
@@ -1005,7 +1005,7 @@
{
m_page->dragController().insertDroppedImagePlaceholdersAtCaret(imageSizes);
auto placeholderRects = m_page->dragController().droppedImagePlaceholders().map([&] (auto& element) {
- return rootViewBoundsForElement(element);
+ return rootViewBounds(element);
});
auto imagePlaceholderRange = m_page->dragController().droppedImagePlaceholderRange();
@@ -1360,10 +1360,10 @@
frame.document()->updateLayoutIgnorePendingStylesheets();
if (focusedElement.hasTagName(HTMLNames::textareaTag) || focusedElement.hasTagName(HTMLNames::inputTag) || focusedElement.hasTagName(HTMLNames::selectTag))
- return WebPage::absoluteInteractionBoundsForElement(focusedElement);
+ return WebPage::absoluteInteractionBounds(focusedElement);
if (auto* rootEditableElement = focusedElement.rootEditableElement())
- return WebPage::absoluteInteractionBoundsForElement(*rootEditableElement);
+ return WebPage::absoluteInteractionBounds(*rootEditableElement);
return { };
}
@@ -1678,17 +1678,17 @@
return makeSimpleRange(base, extent);
}
-IntRect WebPage::rootViewBoundsForElement(const Element& element)
+IntRect WebPage::rootViewBounds(const Node& node)
{
- auto* frame = element.document().frame();
+ auto frame = makeRefPtr(node.document().frame());
if (!frame)
return { };
- auto* view = frame->view();
+ auto view = makeRefPtr(frame->view());
if (!view)
return { };
- auto* renderer = element.renderer();
+ auto* renderer = node.renderer();
if (!renderer)
return { };
@@ -1695,17 +1695,17 @@
return view->contentsToRootView(renderer->absoluteBoundingBoxRect());
}
-IntRect WebPage::absoluteInteractionBoundsForElement(const Element& element)
+IntRect WebPage::absoluteInteractionBounds(const Node& node)
{
- auto* frame = element.document().frame();
+ auto frame = makeRefPtr(node.document().frame());
if (!frame)
return { };
- auto* view = frame->view();
+ auto view = makeRefPtr(frame->view());
if (!view)
return { };
- auto* renderer = element.renderer();
+ auto* renderer = node.renderer();
if (!renderer)
return { };
@@ -1730,17 +1730,17 @@
return enclosingIntRect(boundingBox);
}
-IntRect WebPage::rootViewInteractionBoundsForElement(const Element& element)
+IntRect WebPage::rootViewInteractionBounds(const Node& node)
{
- auto* frame = element.document().frame();
+ auto frame = makeRefPtr(node.document().frame());
if (!frame)
return { };
- auto* view = frame->view();
+ auto view = makeRefPtr(frame->view());
if (!view)
return { };
- return view->contentsToRootView(absoluteInteractionBoundsForElement(element));
+ return view->contentsToRootView(absoluteInteractionBounds(node));
}
void WebPage::clearSelection()
@@ -1757,7 +1757,7 @@
IntRect focusedElementRect;
if (m_focusedElement)
- focusedElementRect = rootViewInteractionBoundsForElement(*m_focusedElement);
+ focusedElementRect = rootViewInteractionBounds(*m_focusedElement);
if (focusedElementRect.isEmpty())
return;
@@ -3242,7 +3242,7 @@
information.elementContext = WTFMove(*elementContext);
if (auto* renderer = focusedElement->renderer()) {
- information.interactionRect = rootViewInteractionBoundsForElement(*focusedElement);
+ information.interactionRect = rootViewInteractionBounds(*focusedElement);
information.nodeFontSize = renderer->style().fontDescription().computedSize();
bool inFixed = false;
@@ -3269,11 +3269,11 @@
information.allowsUserScaling = m_viewportConfiguration.allowsUserScaling();
information.allowsUserScalingIgnoringAlwaysScalable = m_viewportConfiguration.allowsUserScalingIgnoringAlwaysScalable();
if (auto* nextElement = nextAssistableElement(focusedElement.get(), *m_page, true)) {
- information.nextNodeRect = rootViewBoundsForElement(*nextElement);
+ information.nextNodeRect = rootViewBounds(*nextElement);
information.hasNextNode = true;
}
if (auto* previousElement = nextAssistableElement(focusedElement.get(), *m_page, false)) {
- information.previousNodeRect = rootViewBoundsForElement(*previousElement);
+ information.previousNodeRect = rootViewBounds(*previousElement);
information.hasPreviousNode = true;
}
information.focusedElementIdentifier = m_currentFocusedElementIdentifier;
