LibWeb: Reimplement Element attribute related methods with NamedNodeMap

Userland/Libraries/LibWeb/DOM/Element.cpp

@@ -38,6 +38,7 @@ namespace Web::DOM {
 Element::Element(Document& document, QualifiedName qualified_name)
     : ParentNode(document, NodeType::ELEMENT_NODE)
     , m_qualified_name(move(qualified_name))
+    , m_attributes(NamedNodeMap::create(*this))
 {
     make_html_uppercased_qualified_name();
 }
@@ -46,53 +47,60 @@ Element::~Element()
 {
 }
 
-Attribute* Element::find_attribute(const FlyString& name)
+// https://dom.spec.whatwg.org/#dom-element-getattribute
+String Element::get_attribute(const FlyString& name) const
 {
-    for (auto& attribute : m_attributes) {
-        if (attribute->name() == name)
-            return attribute;
-    }
-    return nullptr;
-}
+    // 1. Let attr be the result of getting an attribute given qualifiedName and this.
+    auto const* attribute = m_attributes->get_attribute(name);
 
-const Attribute* Element::find_attribute(const FlyString& name) const
-{
-    for (auto& attribute : m_attributes) {
-        if (attribute->name() == name)
-            return attribute;
-    }
-    return nullptr;
-}
+    // 2. If attr is null, return null.
+    if (!attribute)
+        return {};
 
-String Element::attribute(const FlyString& name) const
-{
-    if (auto* attribute = find_attribute(name))
-        return attribute->value();
-    return {};
+    // 3. Return attr’s value.
+    return attribute->value();
 }
 
+// https://dom.spec.whatwg.org/#dom-element-setattribute
 ExceptionOr<void> Element::set_attribute(const FlyString& name, const String& value)
 {
+    // 1. If qualifiedName does not match the Name production in XML, then throw an "InvalidCharacterError" DOMException.
     // FIXME: Proper name validation
     if (name.is_empty())
         return InvalidCharacterError::create("Attribute name must not be empty");
 
     CSS::StyleInvalidator style_invalidator(document());
 
-    if (auto* attribute = find_attribute(name))
+    // 2. If this is in the HTML namespace and its node document is an HTML document, then set qualifiedName to qualifiedName in ASCII lowercase.
+    // 3. Let attribute be the first attribute in this’s attribute list whose qualified name is qualifiedName, and null otherwise.
+    auto* attribute = m_attributes->get_attribute(name);
+
+    // 4. If attribute is null, create an attribute whose local name is qualifiedName, value is value, and node document is this’s node document, then append this attribute to this, and then return.
+    if (!attribute) {
+        auto new_attribute = Attribute::create(document(), name, value);
+        m_attributes->append_attribute(new_attribute);
+    }
+
+    // 5. Change attribute to value.
+    else {
         attribute->set_value(value);
-    else
-        m_attributes.append(Attribute::create(document(), name, value, this));
+    }
 
     parse_attribute(name, value);
     return {};
 }
 
+// https://dom.spec.whatwg.org/#dom-element-removeattribute
 void Element::remove_attribute(const FlyString& name)
 {
     CSS::StyleInvalidator style_invalidator(document());
+    m_attributes->remove_attribute(name);
+}
 
-    m_attributes.remove_first_matching([&](auto& attribute) { return attribute->name() == name; });
+// https://dom.spec.whatwg.org/#dom-element-hasattribute
+bool Element::has_attribute(const FlyString& name) const
+{
+    return m_attributes->get_attribute(name) != nullptr;
 }
 
 bool Element::has_class(const FlyString& class_name, CaseSensitivity case_sensitivity) const

Userland/Libraries/LibWeb/DOM/Element.h

@@ -13,6 +13,7 @@
 #include <LibWeb/DOM/Attribute.h>
 #include <LibWeb/DOM/ChildNode.h>
 #include <LibWeb/DOM/ExceptionOr.h>
+#include <LibWeb/DOM/NamedNodeMap.h>
 #include <LibWeb/DOM/NonDocumentTypeChildNode.h>
 #include <LibWeb/DOM/ParentNode.h>
 #include <LibWeb/HTML/AttributeNames.h>
@@ -48,13 +49,13 @@ public:
     // NOTE: This is for the JS bindings
     const FlyString& namespace_uri() const { return namespace_(); }
 
-    bool has_attribute(const FlyString& name) const { return find_attribute(name) != nullptr; }
-    bool has_attributes() const { return !m_attributes.is_empty(); }
-    String attribute(const FlyString& name) const;
-    String get_attribute(const FlyString& name) const { return attribute(name); }
+    bool has_attribute(const FlyString& name) const;
+    bool has_attributes() const { return !m_attributes->is_empty(); }
+    String attribute(const FlyString& name) const { return get_attribute(name); }
+    String get_attribute(const FlyString& name) const;
     ExceptionOr<void> set_attribute(const FlyString& name, const String& value);
     void remove_attribute(const FlyString& name);
-    size_t attribute_list_size() const { return m_attributes.size(); }
+    size_t attribute_list_size() const { return m_attributes->length(); }
 
     DOM::ExceptionOr<bool> matches(StringView selectors) const;
 
@@ -66,8 +67,10 @@ public:
     template<typename Callback>
     void for_each_attribute(Callback callback) const
     {
-        for (auto& attribute : m_attributes)
+        for (size_t i = 0; i < m_attributes->length(); ++i) {
+            auto const* attribute = m_attributes->item(i);
             callback(attribute->name(), attribute->value());
+        }
     }
 
     bool has_class(const FlyString&, CaseSensitivity = CaseSensitivity::CaseSensitive) const;
@@ -125,14 +128,11 @@ protected:
     virtual void children_changed() override;
 
 private:
-    Attribute* find_attribute(const FlyString& name);
-    const Attribute* find_attribute(const FlyString& name) const;
-
     void make_html_uppercased_qualified_name();
 
     QualifiedName m_qualified_name;
     String m_html_uppercased_qualified_name;
-    Vector<NonnullRefPtr<Attribute>> m_attributes;
+    NonnullRefPtr<NamedNodeMap> m_attributes;
 
     RefPtr<CSS::CSSStyleDeclaration> m_inline_style;