From 0695236408f2d5c85b4a5ae4d9de309d73dfdde8 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sat, 13 Jan 2024 20:12:25 +1300 Subject: [PATCH] LibWeb: Use cached element name and id where possible Instead of looking up in the named node map, we can simply use the cached name and ID on the element. --- Userland/Libraries/LibWeb/DOM/Document.cpp | 4 ++-- Userland/Libraries/LibWeb/DOM/HTMLCollection.cpp | 10 ++++------ Userland/Libraries/LibWeb/DOM/Node.cpp | 4 ++-- Userland/Libraries/LibWeb/Dump.cpp | 5 ++--- .../LibWeb/HTML/FormAssociatedElement.cpp | 2 +- Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp | 14 ++++++-------- Userland/Libraries/LibWeb/HTML/HTMLMetaElement.cpp | 3 +-- .../Libraries/LibWeb/HTML/NavigableContainer.cpp | 4 ++-- Userland/Libraries/LibWeb/HTML/Window.cpp | 12 ++++++------ Userland/Libraries/LibWeb/Layout/Label.cpp | 4 ++-- Userland/Libraries/LibWeb/Layout/Node.cpp | 4 ++-- 11 files changed, 30 insertions(+), 36 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index c1f8a34ee0..17e5fe44ed 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -1285,7 +1285,7 @@ JS::NonnullGCPtr Document::anchors() { if (!m_anchors) { m_anchors = HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const& element) { - return is(element) && element.has_attribute(HTML::AttributeNames::name); + return is(element) && element.name().has_value(); }); } return *m_anchors; @@ -1828,7 +1828,7 @@ Element* Document::find_a_potential_indicated_element(FlyString const& fragment) // whose value is equal to fragment, then return the first such element in tree order. Element* element_with_name = nullptr; root().for_each_in_subtree_of_type([&](Element const& element) { - if (element.attribute(HTML::AttributeNames::name) == fragment) { + if (element.name() == fragment) { element_with_name = const_cast(&element); return IterationDecision::Break; } diff --git a/Userland/Libraries/LibWeb/DOM/HTMLCollection.cpp b/Userland/Libraries/LibWeb/DOM/HTMLCollection.cpp index a0635bf326..e508db0364 100644 --- a/Userland/Libraries/LibWeb/DOM/HTMLCollection.cpp +++ b/Userland/Libraries/LibWeb/DOM/HTMLCollection.cpp @@ -118,12 +118,10 @@ Vector HTMLCollection::supported_property_names() const } // 2. If element is in the HTML namespace and has a name attribute whose value is neither the empty string nor is in result, append element’s name attribute value to result. - if (element->namespace_uri() == Namespace::HTML) { - if (auto maybe_name = element->attribute(HTML::AttributeNames::name); maybe_name.has_value()) { - auto name = maybe_name.release_value(); - if (!name.is_empty() && !result.contains_slow(name)) - result.append(name); - } + if (element->namespace_uri() == Namespace::HTML && element->name().has_value()) { + auto name = element->name().value(); + if (!name.is_empty() && !result.contains_slow(name)) + result.append(move(name)); } } diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index cf31ce0d90..6db39ddb3e 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -1481,8 +1481,8 @@ String Node::debug_description() const builder.append(node_name().to_deprecated_fly_string().to_lowercase()); if (is_element()) { auto const& element = static_cast(*this); - if (auto id = element.get_attribute(HTML::AttributeNames::id); id.has_value()) - builder.appendff("#{}", id.value()); + if (element.id().has_value()) + builder.appendff("#{}", element.id().value()); for (auto const& class_name : element.class_names()) builder.appendff(".{}", class_name); } diff --git a/Userland/Libraries/LibWeb/Dump.cpp b/Userland/Libraries/LibWeb/Dump.cpp index 504505064e..f5a68b9575 100644 --- a/Userland/Libraries/LibWeb/Dump.cpp +++ b/Userland/Libraries/LibWeb/Dump.cpp @@ -150,10 +150,9 @@ void dump_tree(StringBuilder& builder, Layout::Node const& layout_node, bool sho if (layout_node.dom_node() && is(*layout_node.dom_node())) { auto& element = verify_cast(*layout_node.dom_node()); StringBuilder builder; - auto id = element.deprecated_attribute(HTML::AttributeNames::id); - if (!id.is_empty()) { + if (element.id().has_value() && !element.id()->is_empty()) { builder.append('#'); - builder.append(id); + builder.append(*element.id()); } for (auto& class_name : element.class_names()) { builder.append('.'); diff --git a/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.cpp b/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.cpp index 9fe3b371ac..c1b4bfd0df 100644 --- a/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.cpp @@ -104,7 +104,7 @@ void FormAssociatedElement::reset_form_owner() // 1. If the first element in element's tree, in tree order, to have an ID that is identical to element's form content attribute's value, is a form element, then associate the element with that form element. auto form_value = html_element.attribute(HTML::AttributeNames::form); html_element.root().for_each_in_inclusive_subtree_of_type([this, &form_value](HTMLFormElement& form_element) { - if (form_element.attribute(HTML::AttributeNames::id) == form_value) { + if (form_element.id() == form_value) { set_form(&form_element); return IterationDecision::Break; } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp index 3499680196..fc4803d04e 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp @@ -902,8 +902,8 @@ Vector HTMLFormElement::supported_property_names() const // 2. If candidate has a name attribute, add an entry to sourced names with that name attribute's value as the // string, candidate as the element, and name as the source. - if (auto maybe_name = candidate->attribute(HTML::AttributeNames::name); maybe_name.has_value()) - sourced_names.append(SourcedName { maybe_name.value(), candidate, SourcedName::Source::Name, {} }); + if (candidate->name().has_value()) + sourced_names.append(SourcedName { candidate->name().value(), candidate, SourcedName::Source::Name, {} }); } // 3. For each img element candidate whose form owner is the form element: @@ -920,8 +920,8 @@ Vector HTMLFormElement::supported_property_names() const // 2. If candidate has a name attribute, add an entry to sourced names with that name attribute's value as the // string, candidate as the element, and name as the source. - if (auto maybe_name = candidate->attribute(HTML::AttributeNames::name); maybe_name.has_value()) - sourced_names.append(SourcedName { maybe_name.value(), candidate, SourcedName::Source::Name, {} }); + if (candidate->name().has_value()) + sourced_names.append(SourcedName { candidate->name().value(), candidate, SourcedName::Source::Name, {} }); } // 4. For each entry past entry in the past names map add an entry to sourced names with the past entry's name as @@ -984,8 +984,7 @@ WebIDL::ExceptionOr HTMLFormElement::named_item_value(FlyString const if (!is_form_control(element, *this)) return false; - // FIXME: DOM::Element::name() isn't cached - return name == element.id() || name == element.attribute(HTML::AttributeNames::name); + return name == element.id() || name == element.name(); }); // 2. If candidates is empty, let candidates be a live RadioNodeList object containing all the img elements, @@ -1000,8 +999,7 @@ WebIDL::ExceptionOr HTMLFormElement::named_item_value(FlyString const if (element.form() != this) return false; - // FIXME: DOM::Element::name() isn't cached - return name == element.id() || name == element.attribute(HTML::AttributeNames::name); + return name == element.id() || name == element.name(); }); } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLMetaElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLMetaElement.cpp index 477ca2aca6..120c0baa2e 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLMetaElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLMetaElement.cpp @@ -54,9 +54,8 @@ void HTMLMetaElement::inserted() // * The element is in a document tree // * The element has a name attribute, whose value is an ASCII case-insensitive match for theme-color // * The element has a content attribute - auto name = attribute(AttributeNames::name); auto content = attribute(AttributeNames::content); - if (name.has_value() && name->bytes_as_string_view().equals_ignoring_ascii_case("theme-color"sv) && content.has_value()) { + if (name().has_value() && name()->equals_ignoring_ascii_case("theme-color"sv) && content.has_value()) { auto context = CSS::Parser::ParsingContext { document() }; // 2. For each element in candidate elements: diff --git a/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp b/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp index 0f8594db32..61f0d642b5 100644 --- a/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp +++ b/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp @@ -76,8 +76,8 @@ WebIDL::ExceptionOr NavigableContainer::create_new_child_navigable() Optional target_name; // 5. If element has a name content attribute, then set targetName to the value of that attribute. - if (auto value = attribute(HTML::AttributeNames::name); value.has_value()) - target_name = move(value); + if (name().has_value()) + target_name = name().value().to_string(); // 6. Let documentState be a new document state, with // - document: document diff --git a/Userland/Libraries/LibWeb/HTML/Window.cpp b/Userland/Libraries/LibWeb/HTML/Window.cpp index c1f1a73f5d..8f49b15a4e 100644 --- a/Userland/Libraries/LibWeb/HTML/Window.cpp +++ b/Userland/Libraries/LibWeb/HTML/Window.cpp @@ -1587,8 +1587,8 @@ Vector Window::supported_property_names() const // and are in a document tree with window's associated Document as their root. associated_document().for_each_in_subtree_of_type([&property_names](auto& element) -> IterationDecision { if (is(element) || is(element) || is(element) || is(element)) { - if (auto const& name = element.attribute(AttributeNames::name); name.has_value()) - property_names.set(name.value(), AK::HashSetExistingEntryBehavior::Keep); + if (element.name().has_value()) + property_names.set(element.name().value(), AK::HashSetExistingEntryBehavior::Keep); } if (auto const& name = element.id(); name.has_value()) property_names.set(name.value().to_string(), AK::HashSetExistingEntryBehavior::Keep); @@ -1636,9 +1636,9 @@ WebIDL::ExceptionOr Window::named_item_value(FlyString const& name) c // whose filter matches only named objects of window with the name name. (By definition, these will all be elements.) return DOM::HTMLCollection::create(mutable_this.associated_document(), DOM::HTMLCollection::Scope::Descendants, [name](auto& element) -> bool { if ((is(element) || is(element) || is(element) || is(element)) - && (element.attribute(AttributeNames::name) == name)) + && (element.name() == name)) return true; - return element.attribute(AttributeNames::id) == name; + return element.id() == name; }); } @@ -1664,9 +1664,9 @@ Window::NamedObjects Window::named_objects(StringView name) // HTML elements that have an id content attribute whose value is name and are in a document tree with window's associated Document as their root. associated_document().for_each_in_subtree_of_type([&objects, &name](auto& element) -> IterationDecision { if ((is(element) || is(element) || is(element) || is(element)) - && (element.attribute(AttributeNames::name) == name)) + && (element.name() == name)) objects.elements.append(element); - else if (element.attribute(AttributeNames::id) == name) + else if (element.id() == name) objects.elements.append(element); return IterationDecision::Continue; }); diff --git a/Userland/Libraries/LibWeb/Layout/Label.cpp b/Userland/Libraries/LibWeb/Layout/Label.cpp index 74a83a9917..ec68fffe96 100644 --- a/Userland/Libraries/LibWeb/Layout/Label.cpp +++ b/Userland/Libraries/LibWeb/Layout/Label.cpp @@ -93,7 +93,7 @@ Label const* Label::label_for_control_node(LabelableNode const& control) // same tree as the label element. If the attribute is specified and there is an element in the tree // whose ID is equal to the value of the for attribute, and the first such element in tree order is // a labelable element, then that element is the label element's labeled control. - if (auto id = control.dom_node().attribute(HTML::AttributeNames::id); id.has_value() && !id->is_empty()) { + if (auto const& id = control.dom_node().id(); id.has_value() && !id->is_empty()) { Label const* label = nullptr; control.document().layout_node()->for_each_in_inclusive_subtree_of_type