[popover] Using shadow DOM wrongly auto-hides popover by light dismiss

https://bugs.webkit.org/show_bug.cgi?id=259261 rdar://112410375 Reviewed by Ryosuke Niwa and Darin Adler. Use the flat tree as mandated by the spec when calculating which popover to light dismiss: - https://html.spec.whatwg.org/#topmost-clicked-popover - https://html.spec.whatwg.org/#nearest-inclusive-open-popover - https://html.spec.whatwg.org/#nearest-inclusive-target-popover-for-invoker * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-flat-tree-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-flat-tree.html: Added. * Source/WebCore/dom/Document.cpp: (WebCore::Document::handlePopoverLightDismiss): Canonical link: https://commits.webkit.org/266457@main
WebKit · Jul 31, 2023 · 9336846 · 9336846
1 parent 37a31af
commit 9336846
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 10 deletions.
diff --git a/...c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-flat-tree-expected.txt b/...c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-flat-tree-expected.txt
@@ -0,0 +1,6 @@
+Test passes if the inner popover opens after clicking the inner toggle.
+
+Toggle
+
+PASS Popover light dismiss uses the flat tree
+
diff --git a/...orted/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-flat-tree.html b/...orted/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-flat-tree.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Test that popover light dismiss uses the flat tree.</title>
+    <link rel="author" title="Tim Nguyen" href="https://github.com/nt1m">
+</head>
+<body>
+    <p>Test passes if the inner popover opens after clicking the inner toggle.</p>
+    <button popovertarget="outerPopover" popovertargetaction="toggle" id="outerPopoverToggle">Toggle</button>
+    <div id="outerPopover" popover>
+        <template shadowrootmode="open">
+            Outer
+            <button popovertarget="innerPopover" popovertargetaction="toggle" id="innerPopoverToggle">Toggle</button>
+            <div id="innerPopover" popover>
+                Inner
+            </div>
+        </template>
+    </div>
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+    <script src="/resources/testdriver.js"></script>
+    <script src="/resources/testdriver-actions.js"></script>
+    <script src="/resources/testdriver-vendor.js"></script>
+    <script src="/resources/declarative-shadow-dom-polyfill.js"></script>
+    <script src="resources/popover-utils.js"></script>
+    <script>
+        promise_test(async () => {
+            const innerPopoverToggle = outerPopover.shadowRoot.querySelector("#innerPopoverToggle");
+            const innerPopover = outerPopover.shadowRoot.querySelector("#innerPopover");
+
+            assert_false(outerPopover.matches(":popover-open"), "outer popover is initially hidden");
+            assert_false(innerPopover.matches(":popover-open"), "inner popover is initially hidden");
+
+            await clickOn(outerPopoverToggle);
+
+            assert_true(outerPopover.matches(":popover-open"), "outer popover is open after clicking the toggle");
+            assert_false(innerPopover.matches(":popover-open"), "inner popover is initially hidden");
+
+            await clickOn(innerPopoverToggle);
+
+            assert_true(outerPopover.matches(":popover-open"), "outer popover is not dismissed after clicking the second toggle");
+            assert_true(innerPopover.matches(":popover-open"), "inner popover is open after clicking the second toggle");
+        }, "Popover light dismiss uses the flat tree");
+    </script>
+</body>
+</html>
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
@@ -8906,18 +8906,20 @@ void Document::handlePopoverLightDismiss(const PointerEvent& event, Node& target
             auto isShowingAutoPopover = [](HTMLElement& element) -> bool {
                 return element.popoverState() == PopoverState::Auto && element.popoverData()->visibilityState() == PopoverVisibilityState::Showing;
             };
-            for (auto& element : lineageOfType<HTMLElement>(*startElement)) {
-                if (!clickedPopover && isShowingAutoPopover(element))
-                    clickedPopover = &element;
-
-                if (!invokerPopover) {
-                    if (auto* button = dynamicDowncast<HTMLFormControlElement>(element)) {
-                        if (auto* popover = button->popoverTargetElement(); popover && isShowingAutoPopover(*popover))
-                            invokerPopover = popover;
+            for (RefPtr element = startElement; element; element = element->parentElementInComposedTree()) {
+                if (auto* htmlElement = dynamicDowncast<HTMLElement>(element.get())) {
+                    if (!clickedPopover && isShowingAutoPopover(*htmlElement))
+                        clickedPopover = htmlElement;
+
+                    if (!invokerPopover) {
+                        if (auto* button = dynamicDowncast<HTMLFormControlElement>(*htmlElement)) {
+                            if (auto* popover = button->popoverTargetElement(); popover && isShowingAutoPopover(*popover))
+                                invokerPopover = popover;
+                        }
                     }
+                    if (clickedPopover && invokerPopover)
+                        break;
                 }
-                if (clickedPopover && invokerPopover)
-                    break;
             }
             return std::tuple { WTFMove(clickedPopover), WTFMove(invokerPopover) };
         }();