[popover] Prevent hidePopover from causing a recursive loop

https://bugs.webkit.org/show_bug.cgi?id=256379

Reviewed by Tim Nguyen.

Fix hideAllPopoversUntil for the case where hiding an auto popover changes the auto popover
list (through beforetoggle) so the same auto popover is on top, causing a loop. To fix this, when
this is detected, continue the loop without firing events.

See:
whatwg/html#9198

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss.html:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::topmostAutoPopover const):
(WebCore::Document::hideAllPopoversUntil):
* Source/WebCore/dom/PopoverData.h:
(WebCore::PopoverData::ScopedStartShowingOrHiding::ScopedStartShowingOrHiding):
(WebCore::PopoverData::ScopedStartShowingOrHiding::~ScopedStartShowingOrHiding):
(WebCore::PopoverData::ScopedStartShowingOrHiding::wasShowingOrHiding const):
* Source/WebCore/html/HTMLElement.cpp:
(WebCore::HTMLElement::showPopover):
(WebCore::HTMLElement::hidePopoverInternal):

Canonical link: https://commits.webkit.org/264623@main

LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt

@@ -1,4 +1,4 @@
-Popover 1 Popover 1 Popover1 anchor (no action) Outside all popovers  Next control after popover1  Popover 3 - button 3 Popover 6  Popover8 anchor (no action)  Open convoluted popover Popover 1
+Popover 1 Popover 1 Popover1 anchor (no action) Outside all popovers  Next control after popover1  Popover 3 - button 3 Popover 6  Popover8 anchor (no action)  Open convoluted popover  Example 2
 
 PASS Clicking outside a popover will dismiss the popover
 PASS Canceling pointer events should not keep clicks from light dismissing popovers
@@ -24,4 +24,6 @@ PASS Moving focus back to the anchor element should not dismiss the popover
 PASS Ensure circular/convoluted ancestral relationships are functional
 PASS Ensure circular/convoluted ancestral relationships are functional, with a direct showPopover()
 PASS Hide the target popover during "hide all popovers until"
+PASS Show a sibling popover during "hide all popovers until"
+PASS Show an unrelated popover during "hide popover"
 

LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss.html

@@ -13,7 +13,7 @@
 
 <button id=b1t popovertarget='p1'>Popover 1</button>
 <button id=b1s popovertarget='p1' popovertargetaction=show>Popover 1</button>
-<button id=p1anchor tabindex="0">Popover1 anchor (no action)</button>
+<button id=p1anchor  tabindex="0">Popover1 anchor (no action)</button>
 <span id=outside>Outside all popovers</span>
 <div popover id=p1 anchor=p1anchor>
   <span id=inside1>Inside popover 1</span>
@@ -265,12 +265,12 @@
   },'Dragging from an open popover outside an open popover should leave the popover open');
 </script>
 
-<button id=b3 popovertarget=p3>Popover 3 - button 3
+<button id=b3 popovertarget=p3 tabindex="0">Popover 3 - button 3
   <div popover id=p4>Inside popover 4</div>
 </button>
 <div popover id=p3>Inside popover 3</div>
 <div popover id=p5>Inside popover 5
-  <button popovertarget=p3>Popover 3 - button 4 - unused</button>
+  <button popovertarget=p3 tabindex="0">Popover 3 - button 4 - unused</button>
 </div>
 <style>
   #p3 {top:100px;}
@@ -380,7 +380,7 @@
     await sendTab();
     assert_equals(document.activeElement,popover8Anchor,'Focus should move to the anchor element');
     assert_true(popover8.matches(':popover-open'),'popover should stay open');
-    popover8.hidePopover();
+    popover8.hidePopover(); // Cleanup
   },'Moving focus back to the anchor element should not dismiss the popover');
 </script>
 
@@ -534,5 +534,53 @@
   assert_true(p13.matches(':popover-open'),'p13 should still be open');
   assert_false(p14.matches(':popover-open'));
   assert_false(p15.matches(':popover-open'));
+  p13.hidePopover(); // Cleanup
 },'Hide the target popover during "hide all popovers until"');
 </script>
+
+<div id=p16 popover>Popover 16
+    <div id=p17 popover>Popover 17</div>
+    <div id=p18 popover>Popover 18</div>
+</div>
+
+<script>
+promise_test(async () => {
+  p16.showPopover();
+  p18.showPopover();
+  let events = [];
+  const logEvents = (e) => {events.push(`${e.newState==='open' ? 'show' : 'hide'} ${e.target.id}`)};
+  p16.addEventListener('beforetoggle', logEvents);
+  p17.addEventListener('beforetoggle', logEvents);
+  p18.addEventListener('beforetoggle', (e) => {
+    logEvents(e);
+    p17.showPopover();
+  });
+  p16.hidePopover();
+  assert_array_equals(events,['hide p18','show p17','hide p16'],'There should not be a hide event for p17');
+  assert_false(p16.matches(':popover-open'));
+  assert_false(p17.matches(':popover-open'));
+  assert_false(p18.matches(':popover-open'));
+},'Show a sibling popover during "hide all popovers until"');
+</script>
+
+<div id=p19 popover>Popover 19</div>
+<div id=p20 popover>Popover 20</div>
+<button id=example2 tabindex="0">Example 2</button>
+
+<script>
+promise_test(async () => {
+  p19.showPopover();
+  let events = [];
+  const logEvents = (e) => {events.push(`${e.newState==='open' ? 'show' : 'hide'} ${e.target.id}`)};
+  p19.addEventListener('beforetoggle', (e) => {
+    logEvents(e);
+    p20.showPopover();
+  });
+  p20.addEventListener('beforetoggle', logEvents);
+  p19.hidePopover();
+  assert_array_equals(events,['hide p19','show p20'],'There should not be a second hide event for 19');
+  assert_false(p19.matches(':popover-open'));
+  assert_true(p20.matches(':popover-open'));
+  p20.hidePopover(); // Cleanup
+},'Show an unrelated popover during "hide popover"');
+</script>

LayoutTests/platform/glib/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt

@@ -1,4 +1,4 @@
-Popover 1 Popover 1 Popover1 anchor (no action) Outside all popovers  Next control after popover1 Popover 3 - button 3   Popover 6  Popover8 anchor (no action)  Open convoluted popover Popover 1
+Popover 1 Popover 1 Popover1 anchor (no action) Outside all popovers  Next control after popover1 Popover 3 - button 3   Popover 6  Popover8 anchor (no action)  Open convoluted popover Example 2
 
 PASS Clicking outside a popover will dismiss the popover
 PASS Canceling pointer events should not keep clicks from light dismissing popovers
@@ -24,4 +24,6 @@ PASS Moving focus back to the anchor element should not dismiss the popover
 PASS Ensure circular/convoluted ancestral relationships are functional
 PASS Ensure circular/convoluted ancestral relationships are functional, with a direct showPopover()
 PASS Hide the target popover during "hide all popovers until"
+PASS Show a sibling popover during "hide all popovers until"
+PASS Show an unrelated popover during "hide popover"
 

LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt

@@ -1,4 +1,4 @@
-Popover 1 Popover 1 Popover1 anchor (no action) Outside all popovers  Next control after popover1  Popover 3 - button 3 Popover 6  Popover8 anchor (no action)  Open convoluted popover Popover 1
+Popover 1 Popover 1 Popover1 anchor (no action) Outside all popovers  Next control after popover1  Popover 3 - button 3 Popover 6  Popover8 anchor (no action)  Open convoluted popover  Example 2
 
 FAIL Clicking outside a popover will dismiss the popover assert_false: expected false got true
 FAIL Canceling pointer events should not keep clicks from light dismissing popovers assert_false: expected false got true
@@ -24,4 +24,6 @@ FAIL Moving focus back to the anchor element should not dismiss the popover asse
 PASS Ensure circular/convoluted ancestral relationships are functional
 PASS Ensure circular/convoluted ancestral relationships are functional, with a direct showPopover()
 PASS Hide the target popover during "hide all popovers until"
+PASS Show a sibling popover during "hide all popovers until"
+PASS Show an unrelated popover during "hide popover"
 

Source/WebCore/dom/Document.cpp

@@ -9009,8 +9009,14 @@ HTMLDialogElement* Document::activeModalDialog() const
 HTMLElement* Document::topmostAutoPopover() const
 {
     for (auto& element : makeReversedRange(m_topLayerElements)) {
-        if (auto* candidate = dynamicDowncast<HTMLElement>(element.get()); candidate && candidate->popoverState() == PopoverState::Auto)
-            return candidate;
+#if ENABLE(FULLSCREEN_API)
+        if (element->hasFullscreenFlag())
+            continue;
+#endif
+        if (auto* candidate = dynamicDowncast<HTMLElement>(element.get()); candidate && candidate->popoverState() == PopoverState::Auto) {
+            if (!is<HTMLDialogElement>(candidate) || !downcast<HTMLDialogElement>(candidate)->isOpen())
+                return candidate;
+        }
     }
 
     return nullptr;
@@ -9019,20 +9025,45 @@ HTMLElement* Document::topmostAutoPopover() const
 // https://html.spec.whatwg.org/#hide-all-popovers-until
 void Document::hideAllPopoversUntil(Element* endpoint, FocusPreviousElement focusPreviousElement, FireEvents fireEvents)
 {
-    Vector<Ref<HTMLElement>> popoversToHide;
-    for (auto& item : makeReversedRange(m_topLayerElements)) {
-        if (!is<HTMLElement>(item))
-            continue;
-        auto& element = downcast<HTMLElement>(item.get());
-        if (element.popoverState() == PopoverState::Auto) {
-            if (&element == endpoint)
-                break;
-            popoversToHide.append(element);
+    auto closeAllOpenPopovers = [&]() {
+        while (RefPtr popover = topmostAutoPopover())
+            popover->hidePopoverInternal(focusPreviousElement, fireEvents);
+    };
+    if (!endpoint)
+        return closeAllOpenPopovers();
+    auto autoPopoverList = [&]()  {
+        Vector<Ref<HTMLElement>> popovers;
+        for (auto& item : m_topLayerElements) {
+            if (auto* candidate = dynamicDowncast<HTMLElement>(item.get()); candidate && candidate->popoverState() == PopoverState::Auto)
+                popovers.append(*candidate);
         }
-    }
+        return popovers;
+    };
 
-    for (auto& popover : popoversToHide)
-        popover->hidePopoverInternal(focusPreviousElement, fireEvents);
+    bool repeatingHide = false;
+    do {
+        RefPtr<Element> lastToHide;
+        bool foundEndPoint = false;
+        for (auto& popover : autoPopoverList()) {
+            if (popover.ptr() == endpoint)
+                foundEndPoint = true;
+            else if (foundEndPoint) {
+                lastToHide = popover.ptr();
+                break;
+            }
+        }
+        if (!foundEndPoint)
+            return closeAllOpenPopovers();
+        while (lastToHide && lastToHide->popoverData() && lastToHide->popoverData()->visibilityState() == PopoverVisibilityState::Showing) {
+            auto* topmostAutoPopover = this->topmostAutoPopover();
+            if (!topmostAutoPopover)
+                break;
+            topmostAutoPopover->hidePopoverInternal(focusPreviousElement, fireEvents);
+        }
+        repeatingHide = m_topLayerElements.contains(*endpoint) && topmostAutoPopover() != endpoint;
+        if (repeatingHide)
+            fireEvents = FireEvents::No;
+    } while (repeatingHide);
 }
 
 // https://html.spec.whatwg.org/#popover-light-dismiss

Source/WebCore/dom/PopoverData.h

@@ -62,12 +62,33 @@ class PopoverData {
     HTMLFormControlElement* invoker() const { return m_invoker.get(); }
     void setInvoker(const HTMLFormControlElement* element) { m_invoker = element; }
 
+    class ScopedStartShowingOrHiding {
+    public:
+    explicit ScopedStartShowingOrHiding(Element& popover)
+        : m_popover(popover)
+        , m_wasSet(popover.popoverData()->m_isHidingOrShowingPopover)
+    {
+        m_popover->popoverData()->m_isHidingOrShowingPopover = true;
+    }
+    ~ScopedStartShowingOrHiding()
+    {
+        if (!m_wasSet && m_popover->popoverData())
+            m_popover->popoverData()->m_isHidingOrShowingPopover = false;
+    }
+    bool wasShowingOrHiding() const { return m_wasSet; }
+
+    private:
+        const Ref<Element> m_popover;
+        bool m_wasSet;
+    };
+
 private:
     PopoverState m_popoverState;
     PopoverVisibilityState m_visibilityState;
     std::optional<PopoverToggleEventData> m_queuedToggleEventData;
     WeakPtr<Element, WeakPtrImplWithEventTargetData> m_previouslyFocusedElement;
     WeakPtr<HTMLFormControlElement, WeakPtrImplWithEventTargetData> m_invoker;
+    bool m_isHidingOrShowingPopover = false;
 };
 
 }

Source/WebCore/html/HTMLElement.cpp

@@ -1362,6 +1362,9 @@ ExceptionOr<void> HTMLElement::showPopover(const HTMLFormControlElement* invoker
 
     ASSERT(!isInTopLayer());
 
+    PopoverData::ScopedStartShowingOrHiding showOrHidingPopoverScope(*this);
+    auto fireEvents = showOrHidingPopoverScope.wasShowingOrHiding() ? FireEvents::No : FireEvents::Yes;
+
     Ref document = this->document();
     auto event = ToggleEvent::create(eventNames().beforetoggleEvent, { EventInit { }, "closed"_s, "open"_s }, Event::IsCancelable::Yes);
     dispatchEvent(event);
@@ -1376,10 +1379,12 @@ ExceptionOr<void> HTMLElement::showPopover(const HTMLFormControlElement* invoker
 
     ASSERT(popoverData());
 
+    bool shouldRestoreFocus = false;
+
     if (popoverState() == PopoverState::Auto) {
         auto originalState = popoverState();
         RefPtr ancestor = topmostPopoverAncestor(*this);
-        document->hideAllPopoversUntil(ancestor.get(), FocusPreviousElement::No, FireEvents::Yes);
+        document->hideAllPopoversUntil(ancestor.get(), FocusPreviousElement::No, fireEvents);
 
         if (popoverState() != originalState)
             return Exception { InvalidStateError, "The value of the popover attribute was changed while hiding the popover."_s };
@@ -1389,9 +1394,10 @@ ExceptionOr<void> HTMLElement::showPopover(const HTMLFormControlElement* invoker
             return check.releaseException();
         if (!check.returnValue())
             return { };
+
+        shouldRestoreFocus = !document->topmostAutoPopover();
     }
 
-    bool shouldRestoreFocus = !document->topmostAutoPopover();
     RefPtr previouslyFocusedElement = document->focusedElement();
 
     addToTopLayer();
@@ -1403,8 +1409,10 @@ ExceptionOr<void> HTMLElement::showPopover(const HTMLFormControlElement* invoker
 
     runPopoverFocusingSteps(*this);
 
-    if (shouldRestoreFocus && popoverState() == PopoverState::Auto)
+    if (shouldRestoreFocus) {
+        ASSERT(popoverState() == PopoverState::Auto);
         popoverData()->setPreviouslyFocusedElement(previouslyFocusedElement.get());
+    }
 
     queuePopoverToggleEventTask(PopoverVisibilityState::Hidden, PopoverVisibilityState::Showing);
 
@@ -1419,9 +1427,12 @@ ExceptionOr<void> HTMLElement::hidePopoverInternal(FocusPreviousElement focusPre
     if (!check.returnValue())
         return { };
 
-
     ASSERT(popoverData());
 
+    PopoverData::ScopedStartShowingOrHiding showOrHidingPopoverScope(*this);
+    if (showOrHidingPopoverScope.wasShowingOrHiding())
+        fireEvents = FireEvents::No;
+
     if (popoverState() == PopoverState::Auto) {
         document().hideAllPopoversUntil(this, focusPreviousElement, fireEvents);