Bug 1838450 - Refine attribute handling. r=emilio

whatwg/dom#1176 https://dom.spec.whatwg.org/#concept-element-attributes-change-ext "attribute change steps" at https://html.spec.whatwg.org/multipage/popover.html#attr-popover Differential Revision: https://phabricator.services.mozilla.com/D181880 UltraBlame original commit: 00b027478170756439b780282f9cfdf15edea985
marco-c · Oct 13, 2023 · 251f85c · 251f85c
1 parent 866140c
commit 251f85c
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 121 deletions.
diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp
@@ -652,45 +652,47 @@ constexpr PopoverAttributeState ToPopoverAttributeState(
 }  
 
 void nsGenericHTMLElement::AfterSetPopoverAttr() {
-  const nsAttrValue* newValue = GetParsedAttr(nsGkAtoms::popover);
-
-  const PopoverAttributeState newState = [&newValue]() {
-    if (newValue) {
-      MOZ_ASSERT(newValue->Type() == nsAttrValue::eEnum);
+  auto mapPopoverState = [](const nsAttrValue* value) -> PopoverAttributeState {
+    if (value) {
+      MOZ_ASSERT(value->Type() == nsAttrValue::eEnum);
       const auto popoverAttributeKeyword =
-          static_cast<PopoverAttributeKeyword>(newValue->GetEnumValue());
+          static_cast<PopoverAttributeKeyword>(value->GetEnumValue());
       return ToPopoverAttributeState(popoverAttributeKeyword);
     }
 
 
 
     return PopoverAttributeState::None;
-  }();
+  };
+
+  PopoverAttributeState newState =
+      mapPopoverState(GetParsedAttr(nsGkAtoms::popover));
 
   const PopoverAttributeState oldState = GetPopoverAttributeState();
 
   if (newState != oldState) {
-    EnsurePopoverData().SetPopoverAttributeState(newState);
+    PopoverPseudoStateUpdate(false, true);
 
-    HidePopoverInternal( true,
-                         true, IgnoreErrors());
+    if (IsPopoverOpen()) {
+      HidePopoverInternal( true,
+                           true, IgnoreErrors());
+
+
+
+      newState = mapPopoverState(GetParsedAttr(nsGkAtoms::popover));
+    }
 
-
-
-    if (newState == GetPopoverAttributeState()) {
-      if (newState == PopoverAttributeState::None) {
-
-
-
-
+    if (newState == PopoverAttributeState::None) {
+
+
+      if (GetPopoverData()) {
         OwnerDoc()->RemovePopoverFromTopLayer(*this);
-
-        ClearPopoverData();
-        RemoveStates(ElementState::POPOVER_OPEN);
-      } else {
-
-        PopoverPseudoStateUpdate(false, true);
       }
+      ClearPopoverData();
+      RemoveStates(ElementState::POPOVER_OPEN);
+    } else {
+
+      EnsurePopoverData().SetPopoverAttributeState(newState);
     }
   }
 }
@@ -3165,17 +3167,12 @@ bool nsGenericHTMLElement::PopoverOpen() const {
 bool nsGenericHTMLElement::CheckPopoverValidity(
     PopoverVisibilityState aExpectedState, Document* aExpectedDocument,
     ErrorResult& aRv) {
-  const PopoverData* data = GetPopoverData();
-  if (!data ||
-      data->GetPopoverAttributeState() == PopoverAttributeState::None) {
-    MOZ_ASSERT(!HasAttr(nsGkAtoms::popover));
+  if (GetPopoverAttributeState() == PopoverAttributeState::None) {
     aRv.ThrowNotSupportedError("Element is in the no popover state");
     return false;
   }
 
-  MOZ_ASSERT(HasAttr(nsGkAtoms::popover));
-
-  if (data->GetPopoverVisibilityState() != aExpectedState) {
+  if (GetPopoverData()->GetPopoverVisibilityState() != aExpectedState) {
     return false;
   }
 

diff --git a/testing/web-platform/meta/html/semantics/popovers/popover-attribute-basic.html.ini b/testing/web-platform/meta/html/semantics/popovers/popover-attribute-basic.html.ini
@@ -1,94 +1,4 @@
 [popover-attribute-basic.html]
-  [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 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 null (via idl), and then auto 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 manual to null (via idl), and then auto 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 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 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 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 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 auto to undefined (via attr), and then undefined during 'beforetoggle' works]
-    expected: FAIL
-
-  [Changing a popover from manual to undefined (via attr), and then undefined 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 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 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 null during 'beforetoggle' works]
-    expected: FAIL
-
-  [Changing a popover from manual to undefined (via idl), and then undefined during 'beforetoggle' works]
-    expected: FAIL
-
   [Changing a popover from manual to auto (via attr), and then auto during 'beforetoggle' works]
     expected:
       if (os == "mac") and not debug: [PASS, FAIL]