Bug 1825808 - Popover: Complete ShowPopover and HidePopover, r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D175395

dom/base/Document.cpp

@@ -14977,6 +14977,14 @@ nsTArray<Element*> Document::GetTopLayer() const {
   return elements;
 }
 
+bool Document::TopLayerContains(Element& aElement) const {
+  if (mTopLayer.IsEmpty()) {
+    return false;
+  }
+  nsWeakPtr weakElement = do_GetWeakReference(&aElement);
+  return mTopLayer.Contains(weakElement);
+}
+
 void Document::HideAllPopoversUntil(nsINode& aEndpoint,
                                     bool aFocusPreviousElement,
                                     bool aFireEvents) {
@@ -15029,7 +15037,19 @@ void Document::HidePopover(Element& aPopover, bool aFocusPreviousElement,
     return;
   }
 
-  // TODO: Run auto popover steps.
+  if (popoverHTMLEl->IsAutoPopover()) {
+    // TODO: There might be a circle if show other auto popover while hidding
+    // See, https://github.com/whatwg/html/issues/9196
+    HideAllPopoversUntil(*popoverHTMLEl, aFocusPreviousElement, aFireEvents);
+    if (!popoverHTMLEl->CheckPopoverValidity(PopoverVisibilityState::Showing,
+                                             nullptr, aRv)) {
+      return;
+    }
+    // TODO: we can't always guarantee:
+    // The last item in document's auto popover list is popoverHTMLEl.
+    // See, https://github.com/whatwg/html/issues/9197
+    MOZ_ASSERT(GetTopmostAutoPopover() == popoverHTMLEl);
+  }
 
   aPopover.SetHasPopoverInvoker(false);
 
@@ -15046,7 +15066,7 @@ void Document::HidePopover(Element& aPopover, bool aFocusPreviousElement,
     }
   }
 
-  // TODO: Remove from Top Layer.
+  RemovePopoverFromTopLayer(aPopover);
 
   popoverHTMLEl->PopoverPseudoStateUpdate(false, true);
   popoverHTMLEl->GetPopoverData()->SetPopoverVisibilityState(
@@ -15096,6 +15116,16 @@ void Document::RemoveFromAutoPopoverList(Element& aElement) {
   TopLayerPop(aElement);
 }
 
+void Document::AddPopoverToTopLayer(Element& aElement) {
+  MOZ_ASSERT(aElement.GetPopoverData());
+  TopLayerPush(aElement);
+}
+
+void Document::RemovePopoverFromTopLayer(Element& aElement) {
+  MOZ_ASSERT(aElement.GetPopoverData());
+  TopLayerPop(aElement);
+}
+
 // Returns true if aDoc browsing context is focused.
 bool IsInFocusedTab(Document* aDoc) {
   BrowsingContext* bc = aDoc->GetBrowsingContext();

dom/base/Document.h

@@ -1891,6 +1891,8 @@ class Document : public nsINode,
    */
   nsTArray<Element*> GetTopLayer() const;
 
+  bool TopLayerContains(Element&) const;
+
   // Do the "fullscreen element ready check" from the fullscreen spec.
   // It returns true if the given element is allowed to go into fullscreen.
   // It is responsive to dispatch "fullscreenerror" event when necessary.
@@ -3477,7 +3479,7 @@ class Document : public nsINode,
   // See https://html.spec.whatwg.org/multipage/popover.html#auto-popover-list
   nsTArray<Element*> AutoPopoverList() const;
 
-  // Teturn document's auto popover list's last element.
+  // Return document's auto popover list's last element.
   // See
   // https://html.spec.whatwg.org/multipage/popover.html#topmost-auto-popover
   Element* GetTopmostAutoPopover() const;
@@ -3486,6 +3488,9 @@ class Document : public nsINode,
   void AddToAutoPopoverList(Element&);
   void RemoveFromAutoPopoverList(Element&);
 
+  void AddPopoverToTopLayer(Element&);
+  void RemovePopoverFromTopLayer(Element&);
+
   Element* GetTopLayerTop();
   // Return the fullscreen element in the top layer
   Element* GetUnretargetedFullscreenElement() const;

dom/html/nsGenericHTMLElement.cpp

@@ -3291,6 +3291,7 @@ void nsGenericHTMLElement::ShowPopover(ErrorResult& aRv) {
     return;
   }
   RefPtr<Document> document = OwnerDoc();
+  MOZ_ASSERT(!OwnerDoc()->TopLayerContains(*this));
 
   // Fire beforetoggle event and re-check popover validity.
   if (FireToggleEvent(PopoverVisibilityState::Hidden,
@@ -3300,7 +3301,21 @@ void nsGenericHTMLElement::ShowPopover(ErrorResult& aRv) {
   if (!CheckPopoverValidity(PopoverVisibilityState::Hidden, document, aRv)) {
     return;
   }
-  // TODO: Run auto popover steps.
+
+  if (IsAutoPopover()) {
+    RefPtr<Element> ancestor = GetTopmostPopoverAncestor();
+    if (!ancestor) {
+      ancestor = document->GetDocumentElement();
+    }
+    document->HideAllPopoversUntil(*ancestor, false, true);
+
+    // TODO: Handle if document changes, see
+    // https://github.com/whatwg/html/issues/9177
+    if (!IsAutoPopover() ||
+        !CheckPopoverValidity(PopoverVisibilityState::Hidden, document, aRv)) {
+      return;
+    }
+  }
 
   const bool shouldRestoreFocus = !document->GetTopmostAutoPopover();
   // Let originallyFocusedElement be document's focused area of the document's
@@ -3312,7 +3327,7 @@ void nsGenericHTMLElement::ShowPopover(ErrorResult& aRv) {
         do_GetWeakReference(unretargetedFocus->AsElement());
   }
 
-  // TODO: Add to Top Layer.
+  document->AddPopoverToTopLayer(*this);
 
   PopoverPseudoStateUpdate(true, true);
   GetPopoverData()->SetPopoverVisibilityState(PopoverVisibilityState::Showing);

layout/style/res/html.css

@@ -927,6 +927,10 @@ slot {
     background-color: Canvas;
   }
 
+  :popover-open {
+    -moz-top-layer: top;
+  }
+
   :popover-open::backdrop {
     position: fixed;
     inset: 0;

testing/web-platform/meta/html/semantics/popovers/hide-other-popover-side-effects.html.ini

@@ -1,3 +0,0 @@
-[hide-other-popover-side-effects.html]
-  [Removing a popover while it is opening and force closing another popover should throw an exception.]
-    expected: FAIL

testing/web-platform/meta/html/semantics/popovers/popover-attribute-basic.html.ini

@@ -4,7 +4,7 @@
     if not debug and (os == "linux") and not fission: [ERROR, OK]
     if not debug and (os == "mac"): [ERROR, OK]
     if not debug and (os == "android"): [ERROR, OK]
-    ERROR
+  max-asserts: 15
   [The element <div popover="auto">Pop up</div> should behave as a popover.]
     expected: FAIL
 
@@ -44,9 +44,6 @@
   [Changing the popover type in a "beforetoggle" event handler should throw an exception (during showPopover())]
     expected: FAIL
 
-  [Changing the popover type in a "beforetoggle" event handler should throw an exception (during hidePopover())]
-    expected: FAIL
-
   [The element <div popover="hint">Pop up</div> should behave as a popover.]
     expected: FAIL
 
@@ -112,3 +109,177 @@
       if not debug and (os == "linux"): [PASS, FAIL]
       if not debug and (os == "mac"): [PASS, FAIL]
       if not debug and (os == "android"): [PASS, FAIL]
+
+  [Changing a popover from auto to manual (via attr), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to manual (via attr), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to manual (via attr), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to manual (via attr), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to invalid (via attr), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to invalid (via attr), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to invalid (via attr), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to invalid (via attr), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to null (via attr), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to null (via attr), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to null (via attr), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to null (via attr), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to undefined (via attr), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to undefined (via attr), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to undefined (via attr), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to undefined (via attr), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via attr), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via attr), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via attr), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via attr), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via attr), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to undefined (via attr), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to undefined (via attr), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to undefined (via attr), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to undefined (via attr), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to manual (via idl), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to manual (via idl), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to manual (via idl), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to manual (via idl), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to invalid (via idl), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to invalid (via idl), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to invalid (via idl), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to invalid (via idl), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to null (via idl), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to null (via idl), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to null (via idl), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to null (via idl), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to null (via idl), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to undefined (via idl), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to undefined (via idl), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to undefined (via idl), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to undefined (via idl), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from auto to undefined (via idl), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via idl), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via idl), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via idl), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via idl), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to auto (via idl), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to null (via idl), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to null (via idl), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to null (via idl), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to null (via idl), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to null (via idl), and then undefined during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to undefined (via idl), and then auto during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to undefined (via idl), and then manual during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to undefined (via idl), and then invalid during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to undefined (via idl), and then null during 'beforetoggle' works]
+    expected: FAIL
+
+  [Changing a popover from manual to undefined (via idl), and then undefined during 'beforetoggle' works]
+    expected: FAIL

testing/web-platform/meta/html/semantics/popovers/popover-backdrop-appearance.html.ini

@@ -1,2 +0,0 @@
-[popover-backdrop-appearance.html]
-  expected: FAIL

testing/web-platform/meta/html/semantics/popovers/popover-move-documents.html.ini

@@ -1,6 +1,3 @@
 [popover-move-documents.html]
   [Moving popovers between documents while hiding should not throw an exception.]
     expected: FAIL
-
-  [Moving popovers between documents during light dismiss should throw an exception.]
-    expected: FAIL

testing/web-platform/meta/html/semantics/popovers/popover-open-display.html.ini

@@ -1,2 +0,0 @@
-[popover-open-display.html]
-  expected: FAIL

testing/web-platform/meta/html/semantics/popovers/popover-open-overflow-display.html.ini

@@ -1,2 +1,4 @@
 [popover-open-overflow-display.html]
-  expected: FAIL
+  expected:
+    if os == "win": FAIL
+  max-asserts: 3

testing/web-platform/meta/html/semantics/popovers/popover-target-element-disabled.html.ini

@@ -1,7 +1,4 @@
 [popover-target-element-disabled.html]
-  [Disabled popover*target buttons should not affect the popover heirarchy.]
-    expected: FAIL
-
   [Modifying popovertarget on a button which doesn't break the chain shouldn't close any popovers.]
     expected:
       if not debug and (os == "linux"): [PASS, FAIL]

testing/web-platform/meta/html/semantics/popovers/popover-top-layer-interactions.html.ini

@@ -1,4 +1,7 @@
 [popover-top-layer-interactions.html]
+  [A Modal Dialog should close a Popover.]
+    expected: FAIL
+
   [A Fullscreen Element should close a Popover.]
     expected: FAIL
 
@@ -8,9 +11,6 @@
   [A Modal Dialog should close a Popover.]
     expected: FAIL
 
-  [A Popover should close a Popover.]
-    expected: FAIL
-
   [A Popover should *not* close a Modal Dialog.]
     expected:
       if not debug and (os == "linux"): [PASS, FAIL]