Title: [287860] trunk
- Revision
- 287860
- Author
- [email protected]
- Date
- 2022-01-10 14:58:49 -0800 (Mon, 10 Jan 2022)
Log Message
Modal container observer should classify elements that are styled like clickable controls
https://bugs.webkit.org/show_bug.cgi?id=235022
Reviewed by Tim Horton.
Source/WebCore:
Broaden the criteria when considering whether or not an element inside of a detected modal container is a
"clickable control". In the case where there are event listeners on the modal container, an element inside of
the modal container that has `cursor: pointer;` may trigger an action on the modal container when clicked, even
if it does not have event listeners itself. Handle this scenario by considering the element to be a "clickable
control", and extract text from the element for the purposes of control classification.
Test: ModalContainerObservation.DetectControlsWithEventListenersOnModalContainer
* page/ModalContainerObserver.cpp:
(WebCore::listensToUserActivation):
Factor out this logic into a separate helper function.
(WebCore::isClickableControl):
(WebCore::ModalContainerObserver::collectClickableElements):
Tools:
Add a new API test to exercise the change, and adjust the test harness to allow tests using the harness to
additionally add an event listener on the modal container.
* TestWebKitAPI/Tests/WebKit/modal-container-custom.html:
* TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (287859 => 287860)
--- trunk/Source/WebCore/ChangeLog 2022-01-10 22:36:41 UTC (rev 287859)
+++ trunk/Source/WebCore/ChangeLog 2022-01-10 22:58:49 UTC (rev 287860)
@@ -1,3 +1,26 @@
+2022-01-10 Wenson Hsieh <[email protected]>
+
+ Modal container observer should classify elements that are styled like clickable controls
+ https://bugs.webkit.org/show_bug.cgi?id=235022
+
+ Reviewed by Tim Horton.
+
+ Broaden the criteria when considering whether or not an element inside of a detected modal container is a
+ "clickable control". In the case where there are event listeners on the modal container, an element inside of
+ the modal container that has `cursor: pointer;` may trigger an action on the modal container when clicked, even
+ if it does not have event listeners itself. Handle this scenario by considering the element to be a "clickable
+ control", and extract text from the element for the purposes of control classification.
+
+ Test: ModalContainerObservation.DetectControlsWithEventListenersOnModalContainer
+
+ * page/ModalContainerObserver.cpp:
+ (WebCore::listensToUserActivation):
+
+ Factor out this logic into a separate helper function.
+
+ (WebCore::isClickableControl):
+ (WebCore::ModalContainerObserver::collectClickableElements):
+
2022-01-10 Eric Carlson <[email protected]>
hasBrokenEncryptedMediaAPISupportQuirk and needsPreloadAutoQuirk have overly
Modified: trunk/Source/WebCore/page/ModalContainerObserver.cpp (287859 => 287860)
--- trunk/Source/WebCore/page/ModalContainerObserver.cpp 2022-01-10 22:36:41 UTC (rev 287859)
+++ trunk/Source/WebCore/page/ModalContainerObserver.cpp 2022-01-10 22:58:49 UTC (rev 287860)
@@ -59,6 +59,8 @@
namespace WebCore {
static constexpr size_t maxLengthForClickableElementText = 100;
+static constexpr double maxWidthForElementsThatLookClickable = 200;
+static constexpr double maxHeightForElementsThatLookClickable = 100;
bool ModalContainerObserver::isNeededFor(const Document& document)
{
@@ -295,8 +297,16 @@
return m_containerAndFrameOwnerForControls.second.get();
}
-static bool isClickableControl(const HTMLElement& element)
+static bool listensForUserActivation(const Element& element)
{
+ return element.hasEventListeners(eventNames().clickEvent) || element.hasEventListeners(eventNames().mousedownEvent) || element.hasEventListeners(eventNames().mouseupEvent)
+ || element.hasEventListeners(eventNames().touchstartEvent) || element.hasEventListeners(eventNames().touchendEvent)
+ || element.hasEventListeners(eventNames().pointerdownEvent) || element.hasEventListeners(eventNames().pointerupEvent);
+}
+
+enum class ContainerListensForUserActivation : bool { No, Yes };
+static bool isClickableControl(const HTMLElement& element, ContainerListensForUserActivation containerListensForUserActivation)
+{
if (element.isActuallyDisabled())
return false;
@@ -326,9 +336,28 @@
return equalIgnoringFragmentIdentifier(element.document().url(), href) || !href.protocolIsInHTTPFamily();
}
- return element.hasEventListeners(eventNames().clickEvent) || element.hasEventListeners(eventNames().mousedownEvent) || element.hasEventListeners(eventNames().mouseupEvent)
- || element.hasEventListeners(eventNames().touchstartEvent) || element.hasEventListeners(eventNames().touchendEvent)
- || element.hasEventListeners(eventNames().pointerdownEvent) || element.hasEventListeners(eventNames().pointerupEvent);
+ if (listensForUserActivation(element))
+ return true;
+
+ if (containerListensForUserActivation == ContainerListensForUserActivation::No)
+ return false;
+
+ auto rendererAndRect = element.boundingAbsoluteRectWithoutLayout();
+ if (!rendererAndRect)
+ return false;
+
+ auto [renderer, rect] = *rendererAndRect;
+ if (!renderer || rect.isEmpty())
+ return false;
+
+ // If the modal container itself has event listeners for user activation, continue looking for elements that look like
+ // clickable elements (e.g. small nodes with pointer-style cursor).
+ if (renderer->style().cursor() == CursorType::Pointer) {
+ if (rect.width() <= maxWidthForElementsThatLookClickable && rect.height() <= maxHeightForElementsThatLookClickable)
+ return true;
+ }
+
+ return false;
}
static void removeParentOrChildElements(Vector<Ref<HTMLElement>>& elements)
@@ -709,9 +738,10 @@
if (!containerForControls)
return { };
+ auto containerListensForUserActivation = listensForUserActivation(*containerForControls) ? ContainerListensForUserActivation::Yes : ContainerListensForUserActivation::No;
Vector<Ref<HTMLElement>> clickableControls;
for (auto& child : descendantsOfType<HTMLElement>(*containerForControls)) {
- if (isClickableControl(child))
+ if (isClickableControl(child, containerListensForUserActivation))
clickableControls.append(child);
}
Modified: trunk/Tools/ChangeLog (287859 => 287860)
--- trunk/Tools/ChangeLog 2022-01-10 22:36:41 UTC (rev 287859)
+++ trunk/Tools/ChangeLog 2022-01-10 22:58:49 UTC (rev 287860)
@@ -1,3 +1,17 @@
+2022-01-10 Wenson Hsieh <[email protected]>
+
+ Modal container observer should classify elements that are styled like clickable controls
+ https://bugs.webkit.org/show_bug.cgi?id=235022
+
+ Reviewed by Tim Horton.
+
+ Add a new API test to exercise the change, and adjust the test harness to allow tests using the harness to
+ additionally add an event listener on the modal container.
+
+ * TestWebKitAPI/Tests/WebKit/modal-container-custom.html:
+ * TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm:
+ (TestWebKitAPI::TEST):
+
2022-01-10 Jonathan Bedard <[email protected]>
[git-webkit] Retain old commits in pull-request (Follow-up fix)
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit/modal-container-custom.html (287859 => 287860)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit/modal-container-custom.html 2022-01-10 22:36:41 UTC (rev 287859)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit/modal-container-custom.html 2022-01-10 22:58:49 UTC (rev 287860)
@@ -29,6 +29,13 @@
fixedContainer.innerHTML = markup;
fixedContainer.style.display = "block";
}
+
+function showWithEventListener(markup, eventType, callback) {
+ const fixedContainer = document.getElementById("fixed");
+ fixedContainer.addEventListener(eventType, callback);
+ fixedContainer.innerHTML = markup;
+ fixedContainer.style.display = "block";
+}
</script>
</head>
<body>
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm (287859 => 287860)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm 2022-01-10 22:36:41 UTC (rev 287859)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm 2022-01-10 22:58:49 UTC (rev 287860)
@@ -328,4 +328,16 @@
EXPECT_EQ([webView lastModalContainerInfo].availableTypes, _WKModalContainerControlTypeNegative);
}
+TEST(ModalContainerObservation, DetectControlsWithEventListenersOnModalContainer)
+{
+ auto webView = createModalContainerWebView();
+ [webView loadBundlePage:@"modal-container-custom"];
+ auto script = @"showWithEventListener(`<div>Hello world <span style='cursor: pointer;'>yes</span></div>`, 'click', () => window.testPassed = true)";
+ [webView evaluate:script andDecidePolicy:_WKModalContainerDecisionHideAndAllow];
+ [webView waitForNextPresentationUpdate];
+ EXPECT_FALSE([[webView contentsAsString] containsString:@"Hello world"]);
+ EXPECT_EQ([webView lastModalContainerInfo].availableTypes, _WKModalContainerControlTypePositive);
+ EXPECT_TRUE([[webView objectByEvaluatingJavaScript:@"window.testPassed"] boolValue]);
+}
+
} // namespace TestWebKitAPI
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
