LibWeb: Implement ariaActiveDescendantElement spiritually closer to spec

We are meant to store a weak reference to the element indicated by this
attribute, rather than a GC-protected strong reference. This also hoists
the "get the attr-associated element" AO into its own function, rather
than being hidden in IDL, to match "get the attr-associated elements".

Libraries/LibWeb/ARIA/ARIAMixin.cpp

@@ -14,11 +14,6 @@ namespace Web::ARIA {
 ARIAMixin::ARIAMixin() = default;
 ARIAMixin::~ARIAMixin() = default;
 
-void ARIAMixin::visit_edges(GC::Cell::Visitor& visitor)
-{
-    visitor.visit(m_aria_active_descendant_element);
-}
-
 // https://www.w3.org/TR/wai-aria-1.2/#introroles
 Optional<Role> ARIAMixin::role_from_role_attribute_value() const
 {
@@ -230,6 +225,19 @@ Vector<String> ARIAMixin::parse_id_reference_list(Optional<String> const& id_lis
     return result;
 }
 
+#define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute) \
+    GC::Ptr<DOM::Element> ARIAMixin::attribute() const               \
+    {                                                                \
+        return m_##attribute.ptr();                                  \
+    }                                                                \
+                                                                     \
+    void ARIAMixin::set_##attribute(GC::Ptr<DOM::Element> value)     \
+    {                                                                \
+        m_##attribute = value.ptr();                                 \
+    }
+ENUMERATE_ARIA_ELEMENT_REFERENCING_ATTRIBUTES
+#undef __ENUMERATE_ARIA_ATTRIBUTE
+
 #define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute)               \
     Optional<Vector<WeakPtr<DOM::Element>>> const& ARIAMixin::attribute() const    \
     {                                                                              \

Libraries/LibWeb/ARIA/ARIAMixin.h

@@ -15,6 +15,9 @@
 
 namespace Web::ARIA {
 
+#define ENUMERATE_ARIA_ELEMENT_REFERENCING_ATTRIBUTES \
+    __ENUMERATE_ARIA_ATTRIBUTE(aria_active_descendant_element, aria_active_descendant)
+
 #define ENUMERATE_ARIA_ELEMENT_LIST_REFERENCING_ATTRIBUTES                      \
     __ENUMERATE_ARIA_ATTRIBUTE(aria_controls_elements, aria_controls)           \
     __ENUMERATE_ARIA_ATTRIBUTE(aria_described_by_elements, aria_described_by)   \
@@ -57,8 +60,11 @@ public:
     // https://www.w3.org/TR/wai-aria-1.2/#valuetype_idref_list
     Vector<String> parse_id_reference_list(Optional<String> const&) const;
 
-    GC::Ptr<DOM::Element> aria_active_descendant_element() { return m_aria_active_descendant_element; }
+#define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute) \
-    void set_aria_active_descendant_element(GC::Ptr<DOM::Element> value) { m_aria_active_descendant_element = value; }
+    GC::Ptr<DOM::Element> attribute() const;                         \
+    void set_##attribute(GC::Ptr<DOM::Element> value);
+    ENUMERATE_ARIA_ELEMENT_REFERENCING_ATTRIBUTES
+#undef __ENUMERATE_ARIA_ATTRIBUTE
 
 #define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute)  \
     Optional<Vector<WeakPtr<DOM::Element>>> const& attribute() const; \
@@ -69,12 +75,13 @@ public:
 protected:
     ARIAMixin();
 
-    void visit_edges(GC::Cell::Visitor&);
-
     virtual bool id_reference_exists(String const&) const = 0;
 
 private:
-    GC::Ptr<DOM::Element> m_aria_active_descendant_element;
+#define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute) \
+    WeakPtr<DOM::Element> m_##attribute;
+    ENUMERATE_ARIA_ELEMENT_REFERENCING_ATTRIBUTES
+#undef __ENUMERATE_ARIA_ATTRIBUTE
 
 #define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute) \
     Optional<Vector<WeakPtr<DOM::Element>>> m_##attribute;

Libraries/LibWeb/DOM/Element.cpp

@@ -105,7 +105,6 @@ void Element::visit_edges(Cell::Visitor& visitor)
     Base::visit_edges(visitor);
     SlottableMixin::visit_edges(visitor);
     Animatable::visit_edges(visitor);
-    ARIAMixin::visit_edges(visitor);
 
     visitor.visit(m_attributes);
     visitor.visit(m_inline_style);
@@ -430,6 +429,39 @@ Vector<String> Element::get_attribute_names() const
     return names;
 }
 
+// https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#attr-associated-element
+GC::Ptr<DOM::Element> Element::get_the_attribute_associated_element(FlyString const& content_attribute, GC::Ptr<DOM::Element> explicitly_set_attribute_element) const
+{
+    // 1. Let element be the result of running reflectedTarget's get the element.
+    auto const& element = *this;
+
+    // 2. Let contentAttributeValue be the result of running reflectedTarget's get the content attribute.
+    auto content_attribute_value = element.get_attribute(content_attribute);
+
+    // 3. If reflectedTarget's explicitly set attr-element is not null:
+    if (explicitly_set_attribute_element) {
+        // 1. If reflectedTarget's explicitly set attr-element is a descendant of any of element's shadow-including
+        //    ancestors, then return reflectedTarget's explicitly set attr-element.
+        if (&explicitly_set_attribute_element->root() == &element.shadow_including_root())
+            return *explicitly_set_attribute_element;
+
+        // 2. Return null.
+        return {};
+    }
+
+    // 4. Otherwise, if contentAttributeValue is not null, return the first element candidate, in tree order, that meets
+    //    the following criteria:
+    //     * candidate's root is the same as element's root;
+    //     * candidate's ID is contentAttributeValue; and
+    //     * candidate implements T.
+    if (content_attribute_value.has_value())
+        return element.document().get_element_by_id(*content_attribute_value);
+
+    // 5. If no such element exists, then return null.
+    // 6. Return null.
+    return {};
+}
+
 // https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#attr-associated-elements
 Optional<GC::RootVector<GC::Ref<DOM::Element>>> Element::get_the_attribute_associated_elements(FlyString const& content_attribute, Optional<Vector<WeakPtr<DOM::Element>>> const& explicitly_set_attribute_elements) const
 {
@@ -3658,12 +3690,19 @@ void Element::attribute_changed(FlyString const& local_name, Optional<String> co
             m_dir = Dir::Auto;
         else
             m_dir = {};
-    } else if (local_name == ARIA::AttributeNames::aria_active_descendant) {
-        // https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes:concept-element-attributes-change-ext
-        // Set element's explicitly set attr-element to null.
-        set_aria_active_descendant_element({});
     }
 
+    // https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes:concept-element-attributes-change-ext
+    // 1. If localName is not attr or namespace is not null, then return.
+    // 2. Set element's explicitly set attr-element to null.
+#define __ENUMERATE_ARIA_ATTRIBUTE(attribute, referencing_attribute)                               \
+    else if (local_name == ARIA::AttributeNames::referencing_attribute && !namespace_.has_value()) \
+    {                                                                                              \
+        set_##attribute({});                                                                       \
+    }
+    ENUMERATE_ARIA_ELEMENT_REFERENCING_ATTRIBUTES
+#undef __ENUMERATE_ARIA_ATTRIBUTE
+
     // https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes:concept-element-attributes-change-ext-2
     // 1. If localName is not attr or namespace is not null, then return.
     // 2. Set element's explicitly set attr-elements to null.

Libraries/LibWeb/DOM/Element.h

@@ -163,6 +163,7 @@ public:
     GC::Ptr<Attr> get_attribute_node(FlyString const& name) const;
     GC::Ptr<Attr> get_attribute_node_ns(Optional<FlyString> const& namespace_, FlyString const& name) const;
 
+    GC::Ptr<DOM::Element> get_the_attribute_associated_element(FlyString const& content_attribute, GC::Ptr<DOM::Element> explicitly_set_attribute_element) const;
     Optional<GC::RootVector<GC::Ref<DOM::Element>>> get_the_attribute_associated_elements(FlyString const& content_attribute, Optional<Vector<WeakPtr<DOM::Element>>> const& explicitly_set_attribute_elements) const;
 
     DOMTokenList* class_list();

Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp

@@ -4025,53 +4025,16 @@ JS_DEFINE_NATIVE_FUNCTION(@class_name@::@attribute.getter_callback@)
         retval = MUST(Infra::convert_to_scalar_value_string(*content_attribute_value));
 )~~~");
             }
-            // If a reflected IDL attribute has the type T?, where T is either Element
+            // If a reflected IDL attribute has the type T?, where T is either Element or an interface that inherits
-            // FIXME: or an interface that inherits from Element,
+            // from Element, then with attr being the reflected content attribute name:
-            // then with attr being the reflected content attribute name:
+            // FIXME: Handle "an interface that inherits from Element".
             else if (attribute.type->is_nullable() && attribute.type->name() == "Element") {
                 // The getter steps are to return the result of running this's get the attr-associated element.
                 attribute_generator.append(R"~~~(
-    auto retval = GC::Ptr<Element> {};
+    static auto content_attribute = "@attribute.reflect_name@"_fly_string;
-)~~~");
 
-                // 1. Let element be the result of running reflectedTarget's get the element.
+    auto retval = impl->get_the_attribute_associated_element(content_attribute, TRY(throw_dom_exception_if_needed(vm, [&] { return impl->@attribute.cpp_name@(); })));
-                // 2. Let contentAttributeValue be the result of running reflectedTarget's get the content attribute.
-                attribute_generator.append(R"~~~(
-    auto contentAttributeValue = impl->attribute("@attribute.reflect_name@"_fly_string);
 )~~~");
-                // 3. If reflectedTarget's explicitly set attr-element is not null:
-                //      1. If reflectedTarget's explicitly set attr-element is a descendant of any of element's shadow-including ancestors, then return reflectedTarget's explicitly set attr-element.
-                //      2. Return null.
-                attribute_generator.append(R"~~~(
-    auto const explicitly_set_attr = TRY(throw_dom_exception_if_needed(vm, [&] { return impl->@attribute.cpp_name@(); }));
-    if (explicitly_set_attr) {
-        if (&impl->shadow_including_root() == &explicitly_set_attr->root()) {
-            retval = explicitly_set_attr;
-        } else {
-            retval = GC::Ptr<Element> {};
-        }
-    }
-)~~~");
-                // 4. Otherwise, if contentAttributeValue is not null, return the first element candidate, in tree order, that meets the following criteria:
-                //      candidate's root is the same as element's root;
-                //      candidate's ID is contentAttributeValue; and
-                //      FIXME: candidate implements T.
-                // If no such element exists, then return null.
-                // 5. Return null.
-
-                // FIXME: This works when T is Element but will need adjustment when we handle subtypes too
-                attribute_generator.append(R"~~~(
-    else if (contentAttributeValue.has_value()) {
-        impl->root().for_each_in_inclusive_subtree_of_type<DOM::Element>([&](auto& candidate) {
-            if (candidate.attribute(HTML::AttributeNames::id) == contentAttributeValue.value()) {
-                retval = &candidate;
-                return TraversalDecision::Break;
-            }
-            return TraversalDecision::Continue;
-        });
-    }
-)~~~");
-
             }
             // If a reflected IDL attribute has the type FrozenArray<T>?, where T is either Element or an interface that
             // inherits from Element, then with attr being the reflected content attribute name:
@@ -4209,9 +4172,9 @@ JS_DEFINE_NATIVE_FUNCTION(@class_name@::@attribute.setter_callback@)
     MUST(impl->set_attribute("@attribute.reflect_name@"_fly_string, String::number(cpp_value)));
 )~~~");
                 }
-                // If a reflected IDL attribute has the type T?, where T is either Element
+                // If a reflected IDL attribute has the type T?, where T is either Element or an interface that inherits
-                // FIXME: or an interface that inherits from Element,
+                // from Element, then with attr being the reflected content attribute name:
-                // then with attr being the reflected content attribute name:
+                // FIXME: Handle "an interface that inherits from Element".
                 else if (attribute.type->is_nullable() && attribute.type->name() == "Element") {
                     // The setter steps are:
                     // 1. If the given value is null, then:
@@ -4219,19 +4182,21 @@ JS_DEFINE_NATIVE_FUNCTION(@class_name@::@attribute.setter_callback@)
                     //     2. Run this's delete the content attribute.
                     //     3. Return.
                     attribute_generator.append(R"~~~(
+    static auto content_attribute = "@attribute.reflect_name@"_fly_string;
+
     if (!cpp_value) {
-        TRY(throw_dom_exception_if_needed(vm, [&] { return impl->set_@attribute.cpp_name@(nullptr); }));
+        TRY(throw_dom_exception_if_needed(vm, [&] { return impl->set_@attribute.cpp_name@({}); }));
-        impl->remove_attribute("@attribute.reflect_name@"_fly_string);
+        impl->remove_attribute(content_attribute);
         return JS::js_undefined();
     }
 )~~~");
                     // 2. Run this's set the content attribute with the empty string.
                     attribute_generator.append(R"~~~(
-    MUST(impl->set_attribute("@attribute.reflect_name@"_fly_string, String {}));
+    MUST(impl->set_attribute(content_attribute, String {}));
 )~~~");
                     // 3. Set this's explicitly set attr-element to a weak reference to the given value.
                     attribute_generator.append(R"~~~(
-    TRY(throw_dom_exception_if_needed(vm, [&] { return impl->set_@attribute.cpp_name@(cpp_value); }));
+    TRY(throw_dom_exception_if_needed(vm, [&] { return impl->set_@attribute.cpp_name@(*cpp_value); }));
 )~~~");
                 }
                 // If a reflected IDL attribute has the type FrozenArray<T>?, where T is either Element or an interface