commit 6f16921de968c5da990eeabdd43fb1eb2c4d0339
parent e0d620ec7e5b58dbb0d516859bb932a1560d3745
Author: Keith Cirkel <keithamus@users.noreply.github.com>
Date: Mon, 15 Dec 2025 20:09:11 +0000
Bug 1867743 - Part 5: Tidy up ShowPopover code, aligning to spec r=dom-core,smaug
Some of the steps in ShowPopover were already implemented, but in slightly
different ways compared to the current state of the spec. This commit moves
those particular lines of code around to be more aligned to the spec, in
preparation for popover=hint.
There are no functional changes in this commit, behaviour should be
identical.
Differential Revision: https://phabricator.services.mozilla.com/D276371
Diffstat:
2 files changed, 86 insertions(+), 72 deletions(-)
diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp
@@ -16083,6 +16083,7 @@ void Document::HideAllPopoversUntil(nsINode& aEndpoint,
}
// 2. Let document be endpoint's node document.
+ MOZ_ASSERT(aEndpoint.OwnerDoc() == this);
// 3. Assert: endpoint is a Document or endpoint's popover visibility state is
// showing.
// 4. Assert: endpoint is a Document or endpoint's popover attribute is in the
diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp
@@ -3423,9 +3423,10 @@ void nsGenericHTMLElement::ShowPopoverInternal(Element* aSource,
MOZ_ASSERT(!OwnerDoc()->TopLayerContains(*this));
// 5. Let nestedShow be element's popover showing or hiding.
- bool wasShowingOrHiding = GetPopoverData()->IsShowingOrHiding();
+ bool nestedShow = GetPopoverData()->IsShowingOrHiding();
// 6. Let fireEvents be the boolean negation of nestedShow.
+ bool fireEvents = !nestedShow;
// 7. Set element's popover showing or hiding to true.
GetPopoverData()->SetIsShowingOrHiding(true);
@@ -3435,7 +3436,7 @@ void nsGenericHTMLElement::ShowPopoverInternal(Element* aSource,
// 8.1. If nestedShow is false, then set element's popover showing or hiding
// to false.
if (auto* popoverData = GetPopoverData()) {
- popoverData->SetIsShowingOrHiding(wasShowingOrHiding);
+ popoverData->SetIsShowingOrHiding(nestedShow);
}
});
@@ -3459,10 +3460,16 @@ void nsGenericHTMLElement::ShowPopoverInternal(Element* aSource,
bool shouldRestoreFocus = false;
// 12. Let originalType be the current state of element's popover attribute.
+ auto originalType = GetPopoverAttributeState();
+
// 13. Let stackToAppendTo be null.
+ PopoverAttributeState stackToAppendTo = PopoverAttributeState::None;
+
// 14. Let autoAncestor be the result of running the topmost popover ancestor
// algorithm given element, document's showing auto popover list, source, and
// true.
+ // todo(keithamus) ^ popover hint
+
// 15. Let hintAncestor be the result of running the topmost popover ancestor
// algorithm given element, document's showing hint popover list, source, and
// true.
@@ -3471,7 +3478,7 @@ void nsGenericHTMLElement::ShowPopoverInternal(Element* aSource,
nsWeakPtr originallyFocusedElement;
// 16. If originalType is the Auto state, then:
- if (IsAutoPopover()) {
+ if (originalType == PopoverAttributeState::Auto) {
// 16.1. Run close entire popover list given document's showing hint popover
// list, shouldRestoreFocus, and fireEvents.
// todo(keithamus): ^ popover hint
@@ -3479,7 +3486,6 @@ void nsGenericHTMLElement::ShowPopoverInternal(Element* aSource,
// 16.2. Let ancestor be the result of running the topmost popover ancestor
// algorithm given element, document's showing auto popover list, source,
// and true.
- auto originalState = GetPopoverAttributeState();
RefPtr<nsINode> ancestor = GetTopmostPopoverAncestor(aSource, true);
// 16.3. If ancestor is null, then set ancestor to document.
@@ -3489,40 +3495,10 @@ void nsGenericHTMLElement::ShowPopoverInternal(Element* aSource,
// 16.4. Run hide all popovers until given ancestor, shouldRestoreFocus, and
// fireEvents.
- document->HideAllPopoversUntil(*ancestor, false,
- /* aFireEvents = */ !wasShowingOrHiding);
+ document->HideAllPopoversUntil(*ancestor, false, fireEvents);
// 16.5. Set stackToAppendTo to "auto".
- // todo(keithamus): ^ popover hint
-
- if (GetPopoverAttributeState() != originalState) {
- aRv.ThrowInvalidStateError(
- "The value of the popover attribute was changed while hiding the "
- "popover.");
- return;
- }
-
- // XXX: Roughly steps 18.5.-18.6, 19, 20.
-
- // TODO: Handle if document changes, see
- // https://github.com/whatwg/html/issues/9177
- if (!IsAutoPopover() ||
- !CheckPopoverValidity(PopoverVisibilityState::Hidden, document, aRv)) {
- return;
- }
-
- shouldRestoreFocus = !document->GetTopmostAutoPopover();
- // Let originallyFocusedElement be document's focused area of the document's
- // DOM anchor.
- if (nsIContent* unretargetedFocus =
- document->GetUnretargetedFocusedContent()) {
- originallyFocusedElement =
- do_GetWeakReference(unretargetedFocus->AsElement());
- }
-
- if (StaticPrefs::dom_closewatcher_enabled()) {
- GetPopoverData()->EnsureCloseWatcher(this);
- }
+ stackToAppendTo = PopoverAttributeState::Auto;
}
// 17. If originalType is the Hint state, then:
@@ -3539,36 +3515,73 @@ void nsGenericHTMLElement::ShowPopoverInternal(Element* aSource,
// shouldRestoreFocus, and fireEvents.
// 17.2.2.2 Set stackToAppendTo to "auto".
// 17.2.3. Otherwise, set stackToAppendTo to "hint".
+ // todo(keithamus): ^ popover hint
+
// 18. If originalType is Auto or Hint, then:
- // 18.1. Assert: stackToAppendTo is not null.
- // 18.2. If originalType is not equal to the value of element's popover
- // attribute, then:
- // 18.2.1. If throwExceptions is true, then throw an
- // "InvalidStateError" DOMException.
- // 18.2.2. Return.
- // 18.3. If the result of
- // running check popover validity given element, false, throwExceptions, and
- // document is false, then run cleanupShowingFlag and return.
- // 18.4. If the
- // result of running topmost auto or hint popover on document is null, then
- // set shouldRestoreFocus to true.
- // 18.5. If stackToAppendTo is "auto":
- // 18.5.1.
- // Assert: document's showing auto popover list does not contain element.
- // 18.5.2. Set element's opened in popover mode to "auto".
- // 18.5.- Otherwise:
- // 18.5.-.1. Assert: stackToAppendTo is "hint".
- // 18.5.-.2. Assert: document's showing hint popover list does not contain
- // element.
- // 18.5.-.3. Set element's opened in popover mode to "hint".
- // 18.6.
- // Set element's popover close watcher to the result of establishing a close
- // watcher given element's relevant global object, with:
- // - cancelAction being to return true.
- // - closeAction being to hide a popover given element, true, true,
- // false, and null.
- // - getEnabledState being to return true.
// todo(keithamus): ^ popover hint
+ if (originalType == PopoverAttributeState::Auto) {
+ // 18.1. Assert: stackToAppendTo is not null.
+ MOZ_ASSERT(stackToAppendTo != PopoverAttributeState::None);
+
+ // 18.2. If originalType is not equal to the value of element's popover
+ // attribute, then:
+ if (originalType != GetPopoverAttributeState()) {
+ // 18.2.1. If throwExceptions is true, then throw an
+ // "InvalidStateError" DOMException.
+ aRv.ThrowInvalidStateError(
+ "The value of the popover attribute was changed while hiding the "
+ "popover.");
+ // 18.2.2. Return.
+ return;
+ }
+
+ // 18.3. If the result of running check popover validity given element,
+ // false, throwExceptions, and document is false, then run
+ // cleanupShowingFlag and return.
+ if (!CheckPopoverValidity(PopoverVisibilityState::Hidden, document, aRv)) {
+ return;
+ }
+
+ // 18.4. If the result of running topmost
+ // auto or hint popover on document is null, then set shouldRestoreFocus to
+ // true.
+ shouldRestoreFocus = !document->GetTopmostAutoPopover();
+
+ // 18.5. If stackToAppendTo is "auto":
+ if (stackToAppendTo == PopoverAttributeState::Auto) {
+ // 18.5.1. Assert: document's showing auto popover list does not contain
+ // element.
+ MOZ_ASSERT(!document->AutoPopoverList().Contains(this));
+
+ // 18.5.2. Set element's opened in popover mode to "auto".
+ GetPopoverData()->SetOpenedInMode(PopoverAttributeState::Auto);
+ } else {
+ // 18.5.- Otherwise:
+ // 18.5.-.1. Assert: stackToAppendTo is "hint".
+ // 18.5.-.2. Assert: document's showing hint popover list does not contain
+ // element.
+ // 18.5.-.3. Set element's opened in popover mode to "hint".
+ // todo(keithamus): ^ popover hint
+ MOZ_ASSERT_UNREACHABLE("stackToAppendTo was not Auto!");
+ }
+ // 18.6. Set element's popover close watcher to the result of establishing a
+ // close watcher given element's relevant global object, with:
+ // - cancelAction being to return true.
+ // - closeAction being to hide a popover given element, true, true,
+ // false, and null.
+ // - getEnabledState being to return true.
+ if (StaticPrefs::dom_closewatcher_enabled()) {
+ GetPopoverData()->EnsureCloseWatcher(this);
+ }
+ }
+
+ // 20. Let originallyFocusedElement be document's focused area of the
+ // document's DOM anchor.
+ if (nsIContent* unretargetedFocus =
+ document->GetUnretargetedFocusedContent()) {
+ originallyFocusedElement =
+ do_GetWeakReference(unretargetedFocus->AsElement());
+ }
// 21. Add an element to the top layer given element.
document->AddPopoverToTopLayer(*this);
@@ -3591,16 +3604,16 @@ void nsGenericHTMLElement::ShowPopoverInternal(Element* aSource,
// 25. Run the popover focusing steps given element.
FocusPopover();
- // 26. If shouldRestoreFocus is true and element's popover attribute is not in
- // the No Popover state, then set element's previously focused element to
+ // 26. If shouldRestoreFocus is true and element's popover attribute is not
+ // in the No Popover state, then set element's previously focused element to
// originallyFocusedElement.
if (shouldRestoreFocus &&
GetPopoverAttributeState() != PopoverAttributeState::None) {
GetPopoverData()->SetPreviouslyFocusedElement(originallyFocusedElement);
}
- // 27. Queue a popover toggle event task given element, "closed", "open", and
- // source.
+ // 27. Queue a popover toggle event task given element, "closed", "open",
+ // and source.
QueuePopoverEventTask(PopoverVisibilityState::Hidden, aSource);
// 28. Run cleanupShowingFlag.
@@ -3749,10 +3762,10 @@ void nsGenericHTMLElement::FocusCandidate(Element* aControl,
already_AddRefed<ElementInternals> nsGenericHTMLElement::AttachInternals(
ErrorResult& aRv) {
- // ElementInternals is only available on autonomous custom element, so throws
- // an error by default. The spec steps are implemented in HTMLElement because
- // ElementInternals needs to hold a pointer to HTMLElement in order to forward
- // form operation to it.
+ // ElementInternals is only available on autonomous custom element, so
+ // throws an error by default. The spec steps are implemented in HTMLElement
+ // because ElementInternals needs to hold a pointer to HTMLElement in order
+ // to forward form operation to it.
aRv.ThrowNotSupportedError(nsPrintfCString(
"Cannot attach ElementInternals to a customized built-in or non-custom "
"element "
