From f32185420d88b99baec46fa731ac3dcb0d3b4655 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Tue, 2 Jan 2024 21:23:53 +1300 Subject: [PATCH] LibWeb: Use FlyString where possible in NamedNodeMap We cannot port over Optional until the IDL generator supports passing that through as an argument (as opposed to an Optional). Change to FlyString where possible, and resolve any fallout as a result. --- Userland/Libraries/LibWeb/DOM/Element.cpp | 10 ++++---- Userland/Libraries/LibWeb/DOM/Element.h | 20 +++++++--------- .../Libraries/LibWeb/DOM/NamedNodeMap.cpp | 24 +++++++++---------- Userland/Libraries/LibWeb/DOM/NamedNodeMap.h | 23 +++++++++--------- .../Libraries/LibWeb/HTML/DOMStringMap.cpp | 2 +- .../LibWeb/HTML/HTMLHeadingElement.h | 2 +- .../LibWeb/SVG/SVGGradientElement.cpp | 2 +- .../Libraries/LibWeb/SVG/SVGUseElement.cpp | 2 +- 8 files changed, 40 insertions(+), 45 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 9659dde6fc..4eb0398688 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -107,7 +107,7 @@ void Element::visit_edges(Cell::Visitor& visitor) } // https://dom.spec.whatwg.org/#dom-element-getattribute -Optional Element::get_attribute(StringView name) const +Optional Element::get_attribute(FlyString const& name) const { // 1. Let attr be the result of getting an attribute given qualifiedName and this. auto const* attribute = m_attributes->get_attribute(name); @@ -120,7 +120,7 @@ Optional Element::get_attribute(StringView name) const return attribute->value(); } -ByteString Element::deprecated_get_attribute(StringView name) const +ByteString Element::deprecated_get_attribute(FlyString const& name) const { auto maybe_attribute = get_attribute(name); if (!maybe_attribute.has_value()) @@ -283,20 +283,20 @@ WebIDL::ExceptionOr> Element::set_attribute_node_ns(Attr& attr) } // https://dom.spec.whatwg.org/#dom-element-removeattribute -void Element::remove_attribute(StringView name) +void Element::remove_attribute(FlyString const& name) { // The removeAttribute(qualifiedName) method steps are to remove an attribute given qualifiedName and this, and then return undefined. m_attributes->remove_attribute(name); } // https://dom.spec.whatwg.org/#dom-element-hasattribute -bool Element::has_attribute(StringView name) const +bool Element::has_attribute(FlyString const& name) const { return m_attributes->get_attribute(name) != nullptr; } // https://dom.spec.whatwg.org/#dom-element-hasattributens -bool Element::has_attribute_ns(Optional const& namespace_, StringView name) const +bool Element::has_attribute_ns(Optional const& namespace_, FlyString const& name) const { StringView namespace_view; if (namespace_.has_value()) diff --git a/Userland/Libraries/LibWeb/DOM/Element.h b/Userland/Libraries/LibWeb/DOM/Element.h index 6869069193..1ef877d875 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.h +++ b/Userland/Libraries/LibWeb/DOM/Element.h @@ -91,18 +91,16 @@ public: // NOTE: This is for the JS bindings Optional const& namespace_uri() const { return m_qualified_name.namespace_(); } - // FIXME: This should be taking a 'FlyString const&' / 'Optional const&' - bool has_attribute(StringView name) const; - bool has_attribute_ns(Optional const& namespace_, StringView name) const; + bool has_attribute(FlyString const& name) const; + // FIXME: This should be taking a 'Optional const&' + bool has_attribute_ns(Optional const& namespace_, FlyString const& name) const; bool has_attributes() const; - // FIXME: This should be taking a 'FlyString const&' - ByteString deprecated_attribute(StringView name) const { return deprecated_get_attribute(name); } - Optional attribute(StringView name) const { return get_attribute(name); } + ByteString deprecated_attribute(FlyString const& name) const { return deprecated_get_attribute(name); } + Optional attribute(FlyString const& name) const { return get_attribute(name); } - // FIXME: This should be taking a 'FlyString const&' / 'Optional const&' - Optional get_attribute(StringView name) const; - ByteString deprecated_get_attribute(StringView name) const; + Optional get_attribute(FlyString const& name) const; + ByteString deprecated_get_attribute(FlyString const& name) const; String get_attribute_value(FlyString const& local_name, Optional const& namespace_ = {}) const; WebIDL::ExceptionOr set_attribute(FlyString const& name, String const& value); @@ -114,9 +112,7 @@ public: WebIDL::ExceptionOr> set_attribute_node_ns(Attr&); void append_attribute(Attr&); - - // FIXME: This should take a 'FlyString cosnt&' - void remove_attribute(StringView name); + void remove_attribute(FlyString const& name); WebIDL::ExceptionOr toggle_attribute(FlyString const& name, Optional force); size_t attribute_list_size() const; diff --git a/Userland/Libraries/LibWeb/DOM/NamedNodeMap.cpp b/Userland/Libraries/LibWeb/DOM/NamedNodeMap.cpp index d1b74e68fd..e4b65350bf 100644 --- a/Userland/Libraries/LibWeb/DOM/NamedNodeMap.cpp +++ b/Userland/Libraries/LibWeb/DOM/NamedNodeMap.cpp @@ -85,13 +85,13 @@ Attr const* NamedNodeMap::item(u32 index) const } // https://dom.spec.whatwg.org/#dom-namednodemap-getnameditem -Attr const* NamedNodeMap::get_named_item(StringView qualified_name) const +Attr const* NamedNodeMap::get_named_item(FlyString const& qualified_name) const { return get_attribute(qualified_name); } // https://dom.spec.whatwg.org/#dom-namednodemap-getnameditemns -Attr const* NamedNodeMap::get_named_item_ns(Optional const& namespace_, StringView local_name) const +Attr const* NamedNodeMap::get_named_item_ns(Optional const& namespace_, FlyString const& local_name) const { // FIXME: This conversion is quite ugly. StringView namespace_view; @@ -114,7 +114,7 @@ WebIDL::ExceptionOr> NamedNodeMap::set_named_item_ns(Attr& attri } // https://dom.spec.whatwg.org/#dom-namednodemap-removenameditem -WebIDL::ExceptionOr NamedNodeMap::remove_named_item(StringView qualified_name) +WebIDL::ExceptionOr NamedNodeMap::remove_named_item(FlyString const& qualified_name) { // 1. Let attr be the result of removing an attribute given qualifiedName and element. auto const* attribute = remove_attribute(qualified_name); @@ -128,7 +128,7 @@ WebIDL::ExceptionOr NamedNodeMap::remove_named_item(StringView qual } // https://dom.spec.whatwg.org/#dom-namednodemap-removenameditemns -WebIDL::ExceptionOr NamedNodeMap::remove_named_item_ns(Optional const& namespace_, StringView local_name) +WebIDL::ExceptionOr NamedNodeMap::remove_named_item_ns(Optional const& namespace_, FlyString const& local_name) { // FIXME: This conversion is quite ugly. StringView namespace_view; @@ -147,13 +147,13 @@ WebIDL::ExceptionOr NamedNodeMap::remove_named_item_ns(Optional(const_cast(this)->get_attribute(qualified_name, item_index)); } // https://dom.spec.whatwg.org/#concept-element-attributes-get-by-name -Attr const* NamedNodeMap::get_attribute(StringView qualified_name, size_t* item_index) const +Attr const* NamedNodeMap::get_attribute(FlyString const& qualified_name, size_t* item_index) const { if (item_index) *item_index = 0; @@ -166,10 +166,10 @@ Attr const* NamedNodeMap::get_attribute(StringView qualified_name, size_t* item_ for (auto const& attribute : m_attributes) { if (compare_as_lowercase) { if (attribute->name().equals_ignoring_ascii_case(qualified_name)) - return attribute.ptr(); + return attribute; } else { if (attribute->name() == qualified_name) - return attribute.ptr(); + return attribute; } if (item_index) @@ -197,13 +197,13 @@ Attr* NamedNodeMap::get_attribute_ns(Optional const& namespace_, FlyS } // https://dom.spec.whatwg.org/#concept-element-attributes-get-by-namespace -Attr* NamedNodeMap::get_attribute_ns(StringView namespace_, StringView local_name, size_t* item_index) +Attr* NamedNodeMap::get_attribute_ns(StringView namespace_, FlyString const& local_name, size_t* item_index) { return const_cast(const_cast(this)->get_attribute_ns(namespace_, local_name, item_index)); } // https://dom.spec.whatwg.org/#concept-element-attributes-get-by-namespace -Attr const* NamedNodeMap::get_attribute_ns(StringView namespace_, StringView local_name, size_t* item_index) const +Attr const* NamedNodeMap::get_attribute_ns(StringView namespace_, FlyString const& local_name, size_t* item_index) const { if (item_index) *item_index = 0; @@ -304,7 +304,7 @@ void NamedNodeMap::remove_attribute_at_index(size_t attribute_index) } // https://dom.spec.whatwg.org/#concept-element-attributes-remove-by-name -Attr const* NamedNodeMap::remove_attribute(StringView qualified_name) +Attr const* NamedNodeMap::remove_attribute(FlyString const& qualified_name) { size_t item_index = 0; @@ -320,7 +320,7 @@ Attr const* NamedNodeMap::remove_attribute(StringView qualified_name) } // https://dom.spec.whatwg.org/#concept-element-attributes-remove-by-namespace -Attr const* NamedNodeMap::remove_attribute_ns(StringView namespace_, StringView local_name) +Attr const* NamedNodeMap::remove_attribute_ns(StringView namespace_, FlyString const& local_name) { size_t item_index = 0; diff --git a/Userland/Libraries/LibWeb/DOM/NamedNodeMap.h b/Userland/Libraries/LibWeb/DOM/NamedNodeMap.h index a8137a6d12..b3ff85af49 100644 --- a/Userland/Libraries/LibWeb/DOM/NamedNodeMap.h +++ b/Userland/Libraries/LibWeb/DOM/NamedNodeMap.h @@ -35,19 +35,19 @@ public: // Methods defined by the spec for JavaScript: Attr const* item(u32 index) const; - Attr const* get_named_item(StringView qualified_name) const; - Attr const* get_named_item_ns(Optional const& namespace_, StringView local_name) const; + Attr const* get_named_item(FlyString const& qualified_name) const; + Attr const* get_named_item_ns(Optional const& namespace_, FlyString const& local_name) const; WebIDL::ExceptionOr> set_named_item(Attr& attribute); WebIDL::ExceptionOr> set_named_item_ns(Attr& attribute); - WebIDL::ExceptionOr remove_named_item(StringView qualified_name); - WebIDL::ExceptionOr remove_named_item_ns(Optional const& namespace_, StringView local_name); + WebIDL::ExceptionOr remove_named_item(FlyString const& qualified_name); + WebIDL::ExceptionOr remove_named_item_ns(Optional const& namespace_, FlyString const& local_name); // Methods defined by the spec for internal use: - // FIXME: These should be taking `FlyString const&' / 'Optional const&' - Attr* get_attribute(StringView qualified_name, size_t* item_index = nullptr); - Attr* get_attribute_ns(StringView namespace_, StringView local_name, size_t* item_index = nullptr); - Attr const* get_attribute(StringView qualified_name, size_t* item_index = nullptr) const; - Attr const* get_attribute_ns(StringView namespace_, StringView local_name, size_t* item_index = nullptr) const; + // FIXME: These should all be taking 'Optional const&' for namespace instead of a StringView + Attr* get_attribute(FlyString const& qualified_name, size_t* item_index = nullptr); + Attr* get_attribute_ns(StringView namespace_, FlyString const& local_name, size_t* item_index = nullptr); + Attr const* get_attribute(FlyString const& qualified_name, size_t* item_index = nullptr) const; + Attr const* get_attribute_ns(StringView namespace_, FlyString const& local_name, size_t* item_index = nullptr) const; WebIDL::ExceptionOr> set_attribute(Attr& attribute); void replace_attribute(Attr& old_attribute, Attr& new_attribute, size_t old_attribute_index); void append_attribute(Attr& attribute); @@ -55,9 +55,8 @@ public: Attr* get_attribute_ns(Optional const& namespace_, FlyString const& local_name, size_t* item_index = nullptr); Attr const* get_attribute_ns(Optional const& namespace_, FlyString const& local_name, size_t* item_index = nullptr) const; - // FIXME: This should take a 'FlyString cosnt&' - Attr const* remove_attribute(StringView qualified_name); - Attr const* remove_attribute_ns(StringView namespace_, StringView local_name); + Attr const* remove_attribute(FlyString const& qualified_name); + Attr const* remove_attribute_ns(StringView namespace_, FlyString const& local_name); private: explicit NamedNodeMap(Element&); diff --git a/Userland/Libraries/LibWeb/HTML/DOMStringMap.cpp b/Userland/Libraries/LibWeb/HTML/DOMStringMap.cpp index ae06483c9f..97c8a7ca7f 100644 --- a/Userland/Libraries/LibWeb/HTML/DOMStringMap.cpp +++ b/Userland/Libraries/LibWeb/HTML/DOMStringMap.cpp @@ -190,7 +190,7 @@ WebIDL::ExceptionOr DOMStringMa } // Remove an attribute by name given name and the DOMStringMap's associated element. - auto data_name = builder.to_byte_string(); + auto data_name = MUST(builder.to_string()); m_associated_element->remove_attribute(data_name); // The spec doesn't have the step. This indicates that the deletion was successful. diff --git a/Userland/Libraries/LibWeb/HTML/HTMLHeadingElement.h b/Userland/Libraries/LibWeb/HTML/HTMLHeadingElement.h index 8827a05b80..b72decefbd 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLHeadingElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLHeadingElement.h @@ -26,7 +26,7 @@ public: virtual Optional aria_level() const override { // TODO: aria-level = the number in the element's tag name - return get_attribute("aria-level"sv); + return get_attribute("aria-level"_fly_string); } private: diff --git a/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp index 1ab9c7d88d..576dac5c8d 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp @@ -94,7 +94,7 @@ JS::GCPtr SVGGradientElement::linked_gradient() const // FIXME: This entire function is an ad-hoc hack! // It can only resolve # in the same document. - auto link = has_attribute(AttributeNames::href) ? deprecated_get_attribute(AttributeNames::href) : deprecated_get_attribute("xlink:href"sv); + auto link = has_attribute(AttributeNames::href) ? deprecated_get_attribute(AttributeNames::href) : deprecated_get_attribute("xlink:href"_fly_string); if (auto href = link; !href.is_empty()) { auto url = document().parse_url(href); auto id = url.fragment(); diff --git a/Userland/Libraries/LibWeb/SVG/SVGUseElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGUseElement.cpp index 72f0aff047..b3cd0f99b9 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGUseElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGUseElement.cpp @@ -107,7 +107,7 @@ void SVGUseElement::svg_element_removed(SVGElement& svg_element) return; } - if (svg_element.deprecated_attribute("id"sv).matches(m_referenced_id.value())) { + if (svg_element.deprecated_attribute("id"_fly_string).matches(m_referenced_id.value())) { shadow_root()->remove_all_children(); } }