Title: [287671] trunk
Revision
287671
Author
wenson_hs...@apple.com
Date
2022-01-05 19:08:12 -0800 (Wed, 05 Jan 2022)

Log Message

Modal container observer should detect and suppress elements that prevent user interaction
https://bugs.webkit.org/show_bug.cgi?id=234695

Reviewed by Tim Horton.

Source/WebCore:

Add support for additionally detecting and hiding viewport-constrained elements whose sole (apparent) purpose
is to prevent the user from interacting with the rest of the page, after suppressing a detected modal
container. See comments below for more details.

Test: ModalContainerObservation.HideUserInteractionBlockingElement

* page/ModalContainerObserver.cpp:
(WebCore::ModalContainerObserver::setContainer):

When setting (and hiding) a detected modal container, queue an internal task to try and identify an element
underneath the modal container that may be preventing user interaction with the rest of the page (note that this
must be queued as an async task because it may trigger layout, unlike `updateModalContainerIfNeeded`). See below
for more information.

(WebCore::ModalContainerObserver::hideUserInteractionBlockingElementIfNeeded):

Implement a simple heuristic to detect elements that block user interaction after the modal container has been
hidden. To do this, we hit-test five locations relative to the absolute rect for fixed-position elements: one
hit-test in the center of the rect, and 4 more near all of the corners (shifted by 1px towards the center).

If all 5 hit-tests find the same viewport-constrained element with no rendered children that isn't also the
document or body element, we flag it as the `m_userInteractionBlockingElement` and hide it during style
adjustment in the same way as the modal container (see below).

(WebCore::ModalContainerObserver::revealModalContainer):
* page/ModalContainerObserver.h:
(WebCore::ModalContainerObserver::shouldHide const):

Add `m_userInteractionBlockingElement` and hide both the modal container as well as this new element during
style adjustment.

Tools:

Add an API test (as well as a new test page) to exercise the fix.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit/modal-container-with-overlay.html: Added.
* TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm:
(TestWebKitAPI::TEST):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (287670 => 287671)


--- trunk/Source/WebCore/ChangeLog	2022-01-06 02:57:46 UTC (rev 287670)
+++ trunk/Source/WebCore/ChangeLog	2022-01-06 03:08:12 UTC (rev 287671)
@@ -1,3 +1,41 @@
+2022-01-05  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Modal container observer should detect and suppress elements that prevent user interaction
+        https://bugs.webkit.org/show_bug.cgi?id=234695
+
+        Reviewed by Tim Horton.
+
+        Add support for additionally detecting and hiding viewport-constrained elements whose sole (apparent) purpose
+        is to prevent the user from interacting with the rest of the page, after suppressing a detected modal
+        container. See comments below for more details.
+
+        Test: ModalContainerObservation.HideUserInteractionBlockingElement
+
+        * page/ModalContainerObserver.cpp:
+        (WebCore::ModalContainerObserver::setContainer):
+
+        When setting (and hiding) a detected modal container, queue an internal task to try and identify an element
+        underneath the modal container that may be preventing user interaction with the rest of the page (note that this
+        must be queued as an async task because it may trigger layout, unlike `updateModalContainerIfNeeded`). See below
+        for more information.
+
+        (WebCore::ModalContainerObserver::hideUserInteractionBlockingElementIfNeeded):
+
+        Implement a simple heuristic to detect elements that block user interaction after the modal container has been
+        hidden. To do this, we hit-test five locations relative to the absolute rect for fixed-position elements: one
+        hit-test in the center of the rect, and 4 more near all of the corners (shifted by 1px towards the center).
+
+        If all 5 hit-tests find the same viewport-constrained element with no rendered children that isn't also the
+        document or body element, we flag it as the `m_userInteractionBlockingElement` and hide it during style
+        adjustment in the same way as the modal container (see below).
+
+        (WebCore::ModalContainerObserver::revealModalContainer):
+        * page/ModalContainerObserver.h:
+        (WebCore::ModalContainerObserver::shouldHide const):
+
+        Add `m_userInteractionBlockingElement` and hide both the modal container as well as this new element during
+        style adjustment.
+
 2022-01-05  Antoine Quint  <grao...@webkit.org>
 
         Refactor computed style code for transition-property and the transition shorthand

Modified: trunk/Source/WebCore/page/ModalContainerObserver.cpp (287670 => 287671)


--- trunk/Source/WebCore/page/ModalContainerObserver.cpp	2022-01-06 02:57:46 UTC (rev 287670)
+++ trunk/Source/WebCore/page/ModalContainerObserver.cpp	2022-01-06 03:08:12 UTC (rev 287671)
@@ -32,6 +32,7 @@
 #include "Document.h"
 #include "DocumentLoader.h"
 #include "ElementIterator.h"
+#include "EventHandler.h"
 #include "EventLoop.h"
 #include "EventNames.h"
 #include "Frame.h"
@@ -42,6 +43,7 @@
 #include "HTMLElement.h"
 #include "HTMLImageElement.h"
 #include "HTMLInputElement.h"
+#include "HitTestResult.h"
 #include "ModalContainerTypes.h"
 #include "Page.h"
 #include "RenderDescendantIterator.h"
@@ -246,10 +248,23 @@
     if (container())
         container()->invalidateStyle();
 
+    if (m_userInteractionBlockingElement)
+        m_userInteractionBlockingElement->invalidateStyle();
+
+    m_userInteractionBlockingElement = { };
     m_containerAndFrameOwnerForControls = { { newContainer }, { frameOwner } };
 
     newContainer.invalidateStyle();
     scheduleClickableElementCollection();
+
+    newContainer.document().eventLoop().queueTask(TaskSource::InternalAsyncTask, [weakContainer = WeakPtr { newContainer }]() mutable {
+        RefPtr container = weakContainer.get();
+        if (!container)
+            return;
+
+        if (auto observer = container->document().modalContainerObserverIfExists(); observer && container == observer->container())
+            observer->hideUserInteractionBlockingElementIfNeeded();
+    });
 }
 
 Element* ModalContainerObserver::container() const
@@ -532,11 +547,67 @@
     });
 }
 
+void ModalContainerObserver::hideUserInteractionBlockingElementIfNeeded()
+{
+    if (m_userInteractionBlockingElement) {
+        ASSERT_NOT_REACHED();
+        return;
+    }
+
+    RefPtr container = this->container();
+    if (!container) {
+        ASSERT_NOT_REACHED();
+        return;
+    }
+
+    RefPtr view = container->document().view();
+    if (!view)
+        return;
+
+    auto fixedPositionRect = view->rectForFixedPositionLayout();
+    if (fixedPositionRect.isEmpty())
+        return;
+
+    FixedVector locationsToHitTest {
+        fixedPositionRect.center(),
+        fixedPositionRect.minXMinYCorner() + LayoutSize { 1, 1 },
+        fixedPositionRect.maxXMinYCorner() + LayoutSize { -1, 1 },
+        fixedPositionRect.minXMaxYCorner() + LayoutSize { 1, -1 },
+        fixedPositionRect.maxXMaxYCorner() + LayoutSize { -1, -1 }
+    };
+
+    constexpr OptionSet hitTestTypes { HitTestRequest::Type::ReadOnly, HitTestRequest::Type::DisallowUserAgentShadowContent };
+
+    RefPtr<Element> foundElement;
+    for (auto& location : locationsToHitTest) {
+        auto hitTestResult = view->frame().eventHandler().hitTestResultAtPoint(location, hitTestTypes);
+        auto target = hitTestResult.targetElement();
+        if (!target || is<HTMLBodyElement>(*target) || target->isDocumentNode())
+            return;
+
+        if (foundElement && foundElement != target)
+            return;
+
+        auto renderer = target->renderer();
+        if (!renderer || renderer->firstChild() || !renderer->style().hasViewportConstrainedPosition() || renderer->isDocumentElementRenderer())
+            return;
+
+        if (!foundElement)
+            foundElement = WTFMove(target);
+    }
+
+    m_userInteractionBlockingElement = foundElement.get();
+    foundElement->invalidateStyle();
+}
+
 void ModalContainerObserver::revealModalContainer()
 {
     auto [container, frameOwner] = std::exchange(m_containerAndFrameOwnerForControls, { });
     if (container)
         container->invalidateStyle();
+
+    if (auto element = std::exchange(m_userInteractionBlockingElement, { }))
+        element->invalidateStyle();
 }
 
 std::pair<Vector<WeakPtr<HTMLElement>>, Vector<String>> ModalContainerObserver::collectClickableElements()

Modified: trunk/Source/WebCore/page/ModalContainerObserver.h (287670 => 287671)


--- trunk/Source/WebCore/page/ModalContainerObserver.h	2022-01-06 02:57:46 UTC (rev 287670)
+++ trunk/Source/WebCore/page/ModalContainerObserver.h	2022-01-06 03:08:12 UTC (rev 287671)
@@ -68,10 +68,12 @@
     HTMLFrameOwnerElement* frameOwnerForControls() const;
 
     std::pair<Vector<WeakPtr<HTMLElement>>, Vector<String>> collectClickableElements();
+    void hideUserInteractionBlockingElementIfNeeded();
 
     WeakHashSet<Element> m_elementsToIgnoreWhenSearching;
     std::pair<WeakPtr<Element>, WeakPtr<HTMLFrameOwnerElement>> m_containerAndFrameOwnerForControls;
     WeakHashMap<HTMLFrameOwnerElement, WeakPtr<Element>> m_frameOwnersAndContainersToSearchAgain;
+    WeakPtr<Element> m_userInteractionBlockingElement;
     AtomString m_overrideSearchTermForTesting;
     Timer m_collectClickableElementsTimer;
     bool m_collectingClickableElements { false };
@@ -85,7 +87,10 @@
 
 inline bool ModalContainerObserver::shouldHide(const Element& element) const
 {
-    return container() == &element && !m_collectingClickableElements;
+    if (m_collectingClickableElements)
+        return false;
+
+    return m_userInteractionBlockingElement == &element || container() == &element;
 }
 
 } // namespace WebCore

Modified: trunk/Tools/ChangeLog (287670 => 287671)


--- trunk/Tools/ChangeLog	2022-01-06 02:57:46 UTC (rev 287670)
+++ trunk/Tools/ChangeLog	2022-01-06 03:08:12 UTC (rev 287671)
@@ -1,3 +1,17 @@
+2022-01-05  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Modal container observer should detect and suppress elements that prevent user interaction
+        https://bugs.webkit.org/show_bug.cgi?id=234695
+
+        Reviewed by Tim Horton.
+
+        Add an API test (as well as a new test page) to exercise the fix.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKit/modal-container-with-overlay.html: Added.
+        * TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm:
+        (TestWebKitAPI::TEST):
+
 2022-01-05  Jonathan Bedard  <jbed...@apple.com>
 
         Running layout tests sometimes throws an AssertionError in python2.7/multiprocessing

Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (287670 => 287671)


--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2022-01-06 02:57:46 UTC (rev 287670)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2022-01-06 03:08:12 UTC (rev 287671)
@@ -1070,6 +1070,7 @@
 		F472874727816BCE003EBE7F /* NSResponderTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F43C3823278133190099ABCE /* NSResponderTests.mm */; };
 		F47728991E4AE3C1007ABF6A /* full-page-contenteditable.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F47728981E4AE3AD007ABF6A /* full-page-contenteditable.html */; };
 		F47DFB2621A878DF00021FB6 /* data-detectors.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F47DFB2421A8704A00021FB6 /* data-detectors.html */; };
+		F481C028277945B800E470CF /* modal-container-with-overlay.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F481C0272779459700E470CF /* modal-container-with-overlay.html */; };
 		F4856CA31E649EA8009D7EE7 /* attachment-element.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4856CA21E6498A8009D7EE7 /* attachment-element.html */; };
 		F486B1D01F67952300F34BDD /* DataTransfer-setDragImage.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F486B1CF1F6794FF00F34BDD /* DataTransfer-setDragImage.html */; };
 		F48D6C10224B377000E3E2FB /* PreferredContentMode.mm in Sources */ = {isa = PBXBuildFile; fileRef = F48D6C0F224B377000E3E2FB /* PreferredContentMode.mm */; };
@@ -1478,6 +1479,7 @@
 				4971B1202453A88C0096994D /* missingTopFrameUniqueRedirectSameSiteStrictTableSchema.db in Copy Resources */,
 				51CD1C721B38D48400142CA5 /* modal-alerts-in-new-about-blank-window.html in Copy Resources */,
 				F458286027750525003ECCF3 /* modal-container-custom.html in Copy Resources */,
+				F481C028277945B800E470CF /* modal-container-with-overlay.html in Copy Resources */,
 				F4E86F8A2773DC1B003859A6 /* modal-container.html in Copy Resources */,
 				7A1458FC1AD5C07000E06772 /* mouse-button-listener.html in Copy Resources */,
 				33E79E06137B5FD900E32D99 /* mouse-move-listener.html in Copy Resources */,
@@ -3040,6 +3042,7 @@
 		F47DFB2421A8704A00021FB6 /* data-detectors.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "data-detectors.html"; sourceTree = "<group>"; };
 		F47DFB2721A885E700021FB6 /* DataDetectorsCoreSPI.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = DataDetectorsCoreSPI.h; sourceTree = "<group>"; };
 		F4811E5821940B4400A5E0FD /* WKWebViewEditActions.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebViewEditActions.mm; sourceTree = "<group>"; };
+		F481C0272779459700E470CF /* modal-container-with-overlay.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "modal-container-with-overlay.html"; sourceTree = "<group>"; };
 		F4856CA21E6498A8009D7EE7 /* attachment-element.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "attachment-element.html"; sourceTree = "<group>"; };
 		F486B1CF1F6794FF00F34BDD /* DataTransfer-setDragImage.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "DataTransfer-setDragImage.html"; sourceTree = "<group>"; };
 		F48D6C0F224B377000E3E2FB /* PreferredContentMode.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = PreferredContentMode.mm; sourceTree = "<group>"; };
@@ -4672,6 +4675,7 @@
 				4971B11F2453A87F0096994D /* missingTopFrameUniqueRedirectSameSiteStrictTableSchema.db */,
 				51CD1C711B38D48400142CA5 /* modal-alerts-in-new-about-blank-window.html */,
 				F458285F2775051B003ECCF3 /* modal-container-custom.html */,
+				F481C0272779459700E470CF /* modal-container-with-overlay.html */,
 				F4E86F892773DC06003859A6 /* modal-container.html */,
 				7A1458FB1AD5C03500E06772 /* mouse-button-listener.html */,
 				33E79E05137B5FCE00E32D99 /* mouse-move-listener.html */,

Added: trunk/Tools/TestWebKitAPI/Tests/WebKit/modal-container-with-overlay.html (0 => 287671)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit/modal-container-with-overlay.html	                        (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit/modal-container-with-overlay.html	2022-01-06 03:08:12 UTC (rev 287671)
@@ -0,0 +1,75 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<style>
+body, html {
+    margin: 0;
+    background-color: #EFEFEF;
+    font-family: system-ui;
+}
+
+button {
+    padding: 6px;
+    border-radius: 6px;
+    appearance: none;
+    white-space: normal;
+    width: auto;
+    margin: 4px;
+    border: none;
+    font-size: 1em;
+    cursor: pointer;
+    color: rgb(76, 217, 100);
+    background-color: rgba(76, 217, 100, 0.1);
+}
+
+#fixed {
+    width: 300px;
+    height: 300px;
+    top: calc(50% - 150px);
+    left: calc(50% - 150px);
+    position: fixed;
+    background-color: white;
+    border-radius: 8px;
+    padding: 16px;
+    z-index: 1001;
+    font-size: 14px;
+}
+
+#overlay {
+    position: fixed;
+    top: 0;
+    left: 0;
+    width: 100%;
+    height: 100%;
+    z-index: 1000;
+    background-color: rgba(100, 100, 100, 0.5);
+}
+
+#content {
+    width: 200px;
+    border: 1px solid black;
+    margin: 0;
+    box-sizing: border-box;
+}
+</style>
+<script>
+if (window.internals)
+    internals.overrideModalContainerSearchTermForTesting("hello world");
+</script>
+</head>
+<body>
+<p id="content">Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua.</p>
+<div id="overlay"></div>
+<div id="fixed">
+    <p>Hello world</p>
+    <button id="button">Yes</button>
+</div>
+<script>
+button.addEventListener("click", () => {
+    fixed.remove();
+    overlay.remove();
+});
+</script>
+</body>
+</html>

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm (287670 => 287671)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm	2022-01-06 02:57:46 UTC (rev 287670)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm	2022-01-06 03:08:12 UTC (rev 287671)
@@ -256,6 +256,18 @@
     EXPECT_EQ([webView lastModalContainerInfo].availableTypes, _WKModalContainerControlTypePositive | _WKModalContainerControlTypeNegative);
 }
 
+TEST(ModalContainerObservation, HideUserInteractionBlockingElement)
+{
+    auto webView = createModalContainerWebView();
+    [webView loadBundlePage:@"modal-container-with-overlay" andDecidePolicy:_WKModalContainerDecisionHideAndIgnore];
+
+    EXPECT_FALSE([[webView contentsAsString] containsString:@"Hello world"]);
+    EXPECT_EQ([webView lastModalContainerInfo].availableTypes, _WKModalContainerControlTypePositive);
+
+    NSString *hitTestedText = [webView stringByEvaluatingJavaScript:@"document.elementFromPoint(50, 50).textContent"];
+    EXPECT_TRUE([hitTestedText containsString:@"Lorem"]);
+}
+
 TEST(ModalContainerObservation, IgnoreFixedDocumentElement)
 {
     auto webView = createModalContainerWebView();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to