From f56ca133405b01a6b775dba20f20edccc3d449ad Mon Sep 17 00:00:00 2001
From: Ziran Sun <zsun@igalia.com>
Date: Fri, 16 Feb 2024 21:51:24 +0000
Subject: [PATCH] Bug 1879001 - Remove the popovertarget attribute or set it to
 an invalid value should clear the explicitly set attr-element. r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D201711
---
 dom/base/Element.cpp                          | 14 +++++---
 dom/base/Element.h                            |  2 ++
 dom/html/nsGenericHTMLElement.cpp             |  2 ++
 .../popovertarget-reflection.html.ini         |  3 --
 .../popovers/popovertarget-reflection.html    | 34 ++++++++++++++++++-
 5 files changed, 46 insertions(+), 9 deletions(-)
 delete mode 100644 testing/web-platform/meta/html/semantics/popovers/popovertarget-reflection.html.ini

diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp
index 186628e8076a..f653363f48fb 100644
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1767,19 +1767,23 @@ Element* Element::GetAttrAssociatedElement(nsAtom* aAttr) const {
   return nullptr;
 }
 
+void Element::ClearExplicitlySetAttrElement(nsAtom* aAttr) {
+  if (auto* slots = GetExistingExtendedDOMSlots()) {
+    slots->mExplicitlySetAttrElements.Remove(aAttr);
+  }
+}
+
 void Element::ExplicitlySetAttrElement(nsAtom* aAttr, Element* aElement) {
   if (aElement) {
+    SetAttr(aAttr, EmptyString(), IgnoreErrors());
     nsExtendedDOMSlots* slots = ExtendedDOMSlots();
     slots->mExplicitlySetAttrElements.InsertOrUpdate(
         aAttr, do_GetWeakReference(aElement));
-    SetAttr(aAttr, EmptyString(), IgnoreErrors());
     return;
   }
 
-  if (auto* slots = GetExistingExtendedDOMSlots()) {
-    slots->mExplicitlySetAttrElements.Remove(aAttr);
-    UnsetAttr(aAttr, IgnoreErrors());
-  }
+  ClearExplicitlySetAttrElement(aAttr);
+  UnsetAttr(aAttr, IgnoreErrors());
 }
 
 void Element::GetElementsWithGrid(nsTArray<RefPtr<Element>>& aElements) {
diff --git a/dom/base/Element.h b/dom/base/Element.h
index dd34dfbf162b..6c717bfc88c7 100644
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1243,6 +1243,8 @@ class Element : public FragmentOrElement {
    */
   void ExplicitlySetAttrElement(nsAtom* aAttr, Element* aElement);
 
+  void ClearExplicitlySetAttrElement(nsAtom*);
+
   PseudoStyleType GetPseudoElementType() const {
     nsresult rv = NS_OK;
     auto raw = GetProperty(nsGkAtoms::pseudoProperty, &rv);
diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp
index d96ddc5fbce8..de29276fdccd 100644
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -762,6 +762,8 @@ void nsGenericHTMLElement::AfterSetAttr(int32_t aNamespaceID, nsAtom* aName,
       nsContentUtils::AddScriptRunner(
           NewRunnableMethod("nsGenericHTMLElement::AfterSetPopoverAttr", this,
                             &nsGenericHTMLElement::AfterSetPopoverAttr));
+    } else if (aName == nsGkAtoms::popovertarget) {
+      ClearExplicitlySetAttrElement(nsGkAtoms::popovertarget);
     } else if (aName == nsGkAtoms::dir) {
       auto dir = Directionality::Ltr;
       // A boolean tracking whether we need to recompute our directionality.
diff --git a/testing/web-platform/meta/html/semantics/popovers/popovertarget-reflection.html.ini b/testing/web-platform/meta/html/semantics/popovers/popovertarget-reflection.html.ini
deleted file mode 100644
index b181bd6a9c8f..000000000000
--- a/testing/web-platform/meta/html/semantics/popovers/popovertarget-reflection.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[popovertarget-reflection.html]
-  [Element attribute reflection of popoverTargetElement/popovertarget should be kept in sync.]
-    expected: FAIL
diff --git a/testing/web-platform/tests/html/semantics/popovers/popovertarget-reflection.html b/testing/web-platform/tests/html/semantics/popovers/popovertarget-reflection.html
index 7f265ae4eb51..d0750fdd4c09 100644
--- a/testing/web-platform/tests/html/semantics/popovers/popovertarget-reflection.html
+++ b/testing/web-platform/tests/html/semantics/popovers/popovertarget-reflection.html
@@ -1,15 +1,25 @@
 <!DOCTYPE html>
 <link rel=author href="mailto:jarhar@chromium.org">
 <link rel=help href="https://bugs.chromium.org/p/chromium/issues/detail?id=1523410">
+<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1879001">
 <link rel=help href="https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes:element">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 
-<button id=mybutton>toggle popover</button>
+<button id=mybutton popovertarget="mypopover">toggle popover</button>
 <div id=mypopover popover=auto>popover</div>
 
 <script>
 test(() => {
+   assert_equals(mybutton.popoverTargetElement.id, "mypopover",
+    'Setting element.popoverTargetElement to a valid element should work');
+
+  mybutton.popoverTargetElement = null;
+  assert_false(mybutton.hasAttribute('popovertarget'),
+    'Setting element.popoverTargetElement to null should unset popovertarget attribute.');
+  assert_equals(mybutton.popoverTargetElement, null,
+    'Setting element.popoverTargetElement to null should remove the existing element from element.popoverTargetElement.');
+
   mybutton.popoverTargetElement = mypopover;
   assert_true(mybutton.hasAttribute('popovertarget'),
     'Assigning to element.popoverTargetElement should set the popovertarget attribute.');
@@ -17,5 +27,27 @@ test(() => {
   mybutton.removeAttribute('popovertarget');
   assert_equals(mybutton.popoverTargetElement, null,
     'Removing the popovertarget attribute should remove the element from element.popoverTargetElement.');
+
+  mybutton.popoverTargetElement = mypopover;
+  assert_true(mybutton.hasAttribute('popovertarget'),
+    'Assigning to element.popoverTargetElement should set the popovertarget attribute.');
+
+  mybutton.setAttribute("popovertarget", 'invalid');
+    assert_equals(mybutton.popoverTargetElement, null,
+    'Setting the popovertarget attribute to a localName that is not attr should remove the existing element from element.popoverTargetElement.');
+
+  mybutton.popoverTargetElement = mypopover;
+  mybutton.setAttribute("popovertarget", "");
+  assert_equals(mybutton.popoverTargetElement.id, "mypopover",
+    'Setting the popovertarget attribute to empty string right after explicitly setting attribute element should have no effect.');
+
+  mybutton.setAttribute("popovertarget", "mypopover");
+  assert_equals(mybutton.popoverTargetElement.id, "mypopover",
+    'Setting the popovertarget attribute to a value should set the popover target element.');
+  mybutton.setAttribute("popovertarget", "");
+  assert_equals(mybutton.getAttribute('popovertarget'), "",
+    'Assigning to element.popoverTargetElement to empty string should update the attribute value to empty string.');
+  assert_equals(mybutton.popoverTargetElement, null,
+    'Setting the popovertarget attribute to empty string should remove the existing element from element.popoverTargetElement.');
 }, 'Element attribute reflection of popoverTargetElement/popovertarget should be kept in sync.');
 </script>