From 50350fb79c6c807fb55bc17254d8910cd0d841ff Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sun, 1 Oct 2023 17:46:26 +1300 Subject: [PATCH] LibWeb: Add a non-DeprecatedString version of Element::get_attribute Renaming the DeprecatedString version of this function to Element::get_deprecated_attribute. While performing this rename, port over functions where it is trivial to do so to the Optional version of this function. --- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 4 ++-- .../Libraries/LibWeb/DOM/DOMTokenList.cpp | 2 +- Userland/Libraries/LibWeb/DOM/Element.cpp | 13 ++++++++-- Userland/Libraries/LibWeb/DOM/Element.h | 15 ++++-------- Userland/Libraries/LibWeb/DOM/Element.idl | 2 +- Userland/Libraries/LibWeb/DOM/Node.cpp | 6 ++--- .../Libraries/LibWeb/DOM/RadioNodeList.cpp | 24 +++++++------------ .../Libraries/LibWeb/HTML/HTMLFormElement.cpp | 2 +- .../LibWeb/HTML/HTMLHeadingElement.h | 2 +- .../LibWeb/HTML/HTMLIFrameElement.cpp | 2 +- .../LibWeb/HTML/HTMLImageElement.cpp | 4 ++-- .../LibWeb/HTML/HTMLInputElement.cpp | 6 ++--- .../Libraries/LibWeb/HTML/HTMLLinkElement.cpp | 16 ++++++------- Userland/Libraries/LibWeb/Layout/Node.cpp | 4 ++-- .../LibWeb/SVG/SVGGradientElement.cpp | 2 +- .../WebContent/ConnectionFromClient.cpp | 2 +- .../WebContent/WebDriverConnection.cpp | 2 +- 17 files changed, 52 insertions(+), 56 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index a111bc1923..013e65913b 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -310,8 +310,8 @@ Vector StyleComputer::collect_matching_rules(DOM::Element const& e if (auto it = rule_cache.rules_by_class.find(class_name); it != rule_cache.rules_by_class.end()) add_rules_to_run(it->value); } - if (auto id = element.get_attribute(HTML::AttributeNames::id); !id.is_null()) { - if (auto it = rule_cache.rules_by_id.find(FlyString::from_deprecated_fly_string(id).release_value_but_fixme_should_propagate_errors()); it != rule_cache.rules_by_id.end()) + if (auto id = element.get_attribute(HTML::AttributeNames::id); id.has_value()) { + if (auto it = rule_cache.rules_by_id.find(id.value()); it != rule_cache.rules_by_id.end()) add_rules_to_run(it->value); } if (auto it = rule_cache.rules_by_tag_name.find(FlyString::from_deprecated_fly_string(element.local_name()).release_value_but_fixme_should_propagate_errors()); it != rule_cache.rules_by_tag_name.end()) diff --git a/Userland/Libraries/LibWeb/DOM/DOMTokenList.cpp b/Userland/Libraries/LibWeb/DOM/DOMTokenList.cpp index b702cee154..d742301264 100644 --- a/Userland/Libraries/LibWeb/DOM/DOMTokenList.cpp +++ b/Userland/Libraries/LibWeb/DOM/DOMTokenList.cpp @@ -64,7 +64,7 @@ DOMTokenList::DOMTokenList(Element& associated_element, FlyString associated_att , m_associated_element(associated_element) , m_associated_attribute(move(associated_attribute)) { - auto value = associated_element.get_attribute(m_associated_attribute.to_deprecated_fly_string()); + auto value = associated_element.deprecated_get_attribute(m_associated_attribute); associated_attribute_changed(value); } diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index a8f2a4b4c4..9877983c95 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -133,7 +133,7 @@ void Element::visit_edges(Cell::Visitor& visitor) } // https://dom.spec.whatwg.org/#dom-element-getattribute -DeprecatedString Element::get_attribute(StringView name) const +Optional Element::get_attribute(StringView name) const { // 1. Let attr be the result of getting an attribute given qualifiedName and this. auto const* attribute = m_attributes->get_attribute(name); @@ -143,7 +143,16 @@ DeprecatedString Element::get_attribute(StringView name) const return {}; // 3. Return attr’s value. - return attribute->value().to_deprecated_string(); + return attribute->value(); +} + +DeprecatedString Element::deprecated_get_attribute(StringView name) const +{ + auto maybe_attribute = get_attribute(name); + if (!maybe_attribute.has_value()) + return {}; + + return maybe_attribute->to_deprecated_string(); } // https://dom.spec.whatwg.org/#concept-element-attributes-get-value diff --git a/Userland/Libraries/LibWeb/DOM/Element.h b/Userland/Libraries/LibWeb/DOM/Element.h index 9a4d764ae2..4efe314114 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.h +++ b/Userland/Libraries/LibWeb/DOM/Element.h @@ -99,17 +99,12 @@ public: bool has_attributes() const; // FIXME: This should be taking a 'FlyString const&' - DeprecatedString deprecated_attribute(StringView name) const { return get_attribute(name); } - Optional attribute(StringView name) const - { - auto ret = deprecated_attribute(name); - if (ret.is_null()) - return {}; - return String::from_deprecated_string(ret).release_value(); - } + DeprecatedString deprecated_attribute(StringView name) const { return deprecated_get_attribute(name); } + Optional attribute(StringView name) const { return get_attribute(name); } // FIXME: This should be taking a 'FlyString const&' / 'Optional const&' - DeprecatedString get_attribute(StringView name) const; + Optional get_attribute(StringView name) const; + DeprecatedString deprecated_get_attribute(StringView name) const; DeprecatedString get_attribute_value(StringView local_name, DeprecatedFlyString const& namespace_ = {}) const; WebIDL::ExceptionOr set_attribute(DeprecatedFlyString const& name, DeprecatedString const& value); @@ -257,7 +252,7 @@ public: #define ARIA_IMPL(name, attribute) \ DeprecatedString name() const override \ { \ - return get_attribute(attribute); \ + return deprecated_get_attribute(attribute); \ } \ \ WebIDL::ExceptionOr set_##name(DeprecatedString const& value) override \ diff --git a/Userland/Libraries/LibWeb/DOM/Element.idl b/Userland/Libraries/LibWeb/DOM/Element.idl index 7c8b6fa1f3..00580bbf22 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.idl +++ b/Userland/Libraries/LibWeb/DOM/Element.idl @@ -28,7 +28,7 @@ interface Element : Node { readonly attribute DOMString localName; readonly attribute DOMString tagName; - DOMString? getAttribute(DOMString qualifiedName); + [ImplementedAs=deprecated_get_attribute] DOMString? getAttribute(DOMString qualifiedName); [CEReactions] undefined setAttribute(DOMString qualifiedName, DOMString value); [CEReactions] undefined setAttributeNS(DOMString? namespace , DOMString qualifiedName , DOMString value); [CEReactions] Attr? setAttributeNode(Attr attr); diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index 2ae3cb8624..407015de45 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -1387,7 +1387,7 @@ bool Node::is_equal_node(Node const* other_node) const // If A is an element, each attribute in its attribute list has an attribute that equals an attribute in B’s attribute list. bool has_same_attributes = true; this_element.for_each_attribute([&](auto& name, auto& value) { - if (other_element.get_attribute(name) != value) + if (other_element.deprecated_get_attribute(name) != value) has_same_attributes = false; }); if (!has_same_attributes) @@ -1473,8 +1473,8 @@ DeprecatedString Node::debug_description() const builder.append(node_name().to_deprecated_fly_string().to_lowercase()); if (is_element()) { auto& element = static_cast(*this); - if (auto id = element.get_attribute(HTML::AttributeNames::id); !id.is_null()) - builder.appendff("#{}", id); + if (auto id = element.get_attribute(HTML::AttributeNames::id); id.has_value()) + builder.appendff("#{}", id.value()); for (auto const& class_name : element.class_names()) builder.appendff(".{}", class_name); } diff --git a/Userland/Libraries/LibWeb/DOM/RadioNodeList.cpp b/Userland/Libraries/LibWeb/DOM/RadioNodeList.cpp index 92db6fef50..f923cc1e52 100644 --- a/Userland/Libraries/LibWeb/DOM/RadioNodeList.cpp +++ b/Userland/Libraries/LibWeb/DOM/RadioNodeList.cpp @@ -60,44 +60,38 @@ FlyString RadioNodeList::value() const return String {}; // 3. If element is an element with no value attribute, return the string "on". - auto const value = element->get_attribute(HTML::AttributeNames::value); - if (value.is_null()) - return "on"_fly_string; - // 4. Otherwise, return the value of element's value attribute. - return MUST(FlyString::from_deprecated_fly_string(value)); + return element->get_attribute(HTML::AttributeNames::value).value_or("on"_string); } void RadioNodeList::set_value(FlyString const& value) { HTML::HTMLInputElement* element = nullptr; - auto deprecated_value = value.to_deprecated_fly_string(); - // 1. If the new value is the string "on": let element be the first element in tree order represented by the RadioNodeList object // that is an input element whose type attribute is in the Radio Button state and whose value content attribute is either absent, // or present and equal to the new value, if any. If no such element exists, then instead let element be null. if (value == "on"sv) { - element = verify_cast(first_matching([&deprecated_value](auto const& node) { + element = verify_cast(first_matching([&value](auto const& node) { auto const* button = radio_button(node); if (!button) return false; - auto const value = button->get_attribute(HTML::AttributeNames::value); - return value.is_null() || value == deprecated_value; + auto const maybe_value = button->get_attribute(HTML::AttributeNames::value); + return !maybe_value.has_value() || maybe_value.value() == value; })); } // 2. Otherwise: let element be the first element in tree order represented by the RadioNodeList object that is an input element whose - // type attribute is in the Radio Button state and whose value content attribute is present and equal to the new value, if any. If - // no such element exists, then instead let element be null. + // type attribute is in the Radio Button state and whose value content attribute is present and equal to the new value, if any. If + // no such element exists, then instead let element be null. else { - element = verify_cast(first_matching([&deprecated_value](auto const& node) { + element = verify_cast(first_matching([&value](auto const& node) { auto const* button = radio_button(node); if (!button) return false; - auto const value = button->get_attribute(HTML::AttributeNames::value); - return !value.is_null() && value == deprecated_value; + auto const maybe_value = button->get_attribute(HTML::AttributeNames::value); + return maybe_value.has_value() && maybe_value.value() == value; })); } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp index b6a77df03d..d5de63e007 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp @@ -394,7 +394,7 @@ static bool is_form_control(DOM::Element const& element) } if (is(element) - && !element.get_attribute(HTML::AttributeNames::type).equals_ignoring_ascii_case("image"sv)) { + && !element.deprecated_get_attribute(HTML::AttributeNames::type).equals_ignoring_ascii_case("image"sv)) { return true; } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLHeadingElement.h b/Userland/Libraries/LibWeb/HTML/HTMLHeadingElement.h index 4452bbd03f..a880505dfd 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLHeadingElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLHeadingElement.h @@ -25,7 +25,7 @@ public: virtual DeprecatedString aria_level() const override { // TODO: aria-level = the number in the element's tag name - return get_attribute("aria-level"sv); + return deprecated_get_attribute("aria-level"sv); } private: diff --git a/Userland/Libraries/LibWeb/HTML/HTMLIFrameElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLIFrameElement.cpp index 8b42743026..26305d5385 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLIFrameElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLIFrameElement.cpp @@ -90,7 +90,7 @@ void HTMLIFrameElement::process_the_iframe_attributes(bool initial_insertion) } // 3. Navigate to the srcdoc resource: navigate an iframe or frame given element, about:srcdoc, the empty string, and the value of element's srcdoc attribute. - navigate_an_iframe_or_frame(AK::URL("about:srcdoc"sv), ReferrerPolicy::ReferrerPolicy::EmptyString, String::from_deprecated_string(get_attribute(HTML::AttributeNames::srcdoc)).release_value_but_fixme_should_propagate_errors()); + navigate_an_iframe_or_frame(AK::URL("about:srcdoc"sv), ReferrerPolicy::ReferrerPolicy::EmptyString, get_attribute(HTML::AttributeNames::srcdoc)); // FIXME: The resulting Document must be considered an iframe srcdoc document. diff --git a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp index ecb97145a5..ddb0ac8d90 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp @@ -175,7 +175,7 @@ unsigned HTMLImageElement::width() const return paintable_box->content_width().to_int(); // NOTE: This step seems to not be in the spec, but all browsers do it. - auto width_attr = get_attribute(HTML::AttributeNames::width); + auto width_attr = deprecated_get_attribute(HTML::AttributeNames::width); if (auto converted = width_attr.to_uint(); converted.has_value()) return *converted; @@ -203,7 +203,7 @@ unsigned HTMLImageElement::height() const return paintable_box->content_height().to_int(); // NOTE: This step seems to not be in the spec, but all browsers do it. - auto height_attr = get_attribute(HTML::AttributeNames::height); + auto height_attr = deprecated_get_attribute(HTML::AttributeNames::height); if (auto converted = height_attr.to_uint(); converted.has_value()) return *converted; diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp index 828fc26dff..edaca5cf90 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp @@ -352,7 +352,7 @@ DeprecatedString HTMLInputElement::value() const // https://html.spec.whatwg.org/multipage/input.html#dom-input-value-default-on if (type_state() == TypeAttributeState::Checkbox || type_state() == TypeAttributeState::RadioButton) { // On getting, if the element has a value content attribute, return that attribute's value; otherwise, return the string "on". - return has_attribute(AttributeNames::value) ? get_attribute(AttributeNames::value) : "on"; + return get_attribute(AttributeNames::value).value_or("on"_string).to_deprecated_string(); } // https://html.spec.whatwg.org/multipage/input.html#dom-input-value-default @@ -362,7 +362,7 @@ DeprecatedString HTMLInputElement::value() const || type_state() == TypeAttributeState::ResetButton || type_state() == TypeAttributeState::Button) { // On getting, if the element has a value content attribute, return that attribute's value; otherwise, return the empty string. - return has_attribute(AttributeNames::value) ? get_attribute(AttributeNames::value) : DeprecatedString::empty(); + return get_attribute(AttributeNames::value).value_or(String {}).to_deprecated_string(); } // https://html.spec.whatwg.org/multipage/input.html#dom-input-value-value @@ -934,7 +934,7 @@ void HTMLInputElement::reset_algorithm() m_dirty_checkedness = false; // set the value of the element to the value of the value content attribute, if there is one, or the empty string otherwise, - m_value = has_attribute(AttributeNames::value) ? get_attribute(AttributeNames::value) : DeprecatedString::empty(); + m_value = get_attribute(AttributeNames::value).value_or(String {}).to_deprecated_string(); // set the checkedness of the element to true if the element has a checked content attribute and false if it does not, m_checked = has_attribute(AttributeNames::checked); diff --git a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp index b824305a7d..697f9507c3 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp @@ -160,9 +160,7 @@ HTMLLinkElement::LinkProcessingOptions HTMLLinkElement::create_link_options() LinkProcessingOptions options; // FIXME: destination the result of translating the state of el's as attribute // crossorigin the state of el's crossorigin content attribute - options.crossorigin = cors_setting_attribute_from_keyword( - has_attribute(AttributeNames::crossorigin) ? String::from_deprecated_string(get_attribute(AttributeNames::crossorigin)).release_value_but_fixme_should_propagate_errors() - : Optional {}); + options.crossorigin = cors_setting_attribute_from_keyword(get_attribute(AttributeNames::crossorigin)); // FIXME: referrer policy the state of el's referrerpolicy content attribute // FIXME: source set el's source set // base URL document's URL @@ -178,16 +176,16 @@ HTMLLinkElement::LinkProcessingOptions HTMLLinkElement::create_link_options() // FIXME: cryptographic nonce metadata The current value of el's [[CryptographicNonce]] internal slot // 3. If el has an href attribute, then set options's href to the value of el's href attribute. - if (has_attribute(AttributeNames::href)) - options.href = String::from_deprecated_string(get_attribute(AttributeNames::href)).release_value_but_fixme_should_propagate_errors(); + if (auto maybe_href = get_attribute(AttributeNames::href); maybe_href.has_value()) + options.href = maybe_href.value(); // 4. If el has an integrity attribute, then set options's integrity to the value of el's integrity content attribute. - if (has_attribute(AttributeNames::integrity)) - options.integrity = String::from_deprecated_string(get_attribute(AttributeNames::integrity)).release_value_but_fixme_should_propagate_errors(); + if (auto maybe_integrity = get_attribute(AttributeNames::integrity); maybe_integrity.has_value()) + options.integrity = maybe_integrity.value(); // 5. If el has a type attribute, then set options's type to the value of el's type attribute. - if (has_attribute(AttributeNames::type)) - options.type = String::from_deprecated_string(get_attribute(AttributeNames::type)).release_value_but_fixme_should_propagate_errors(); + if (auto maybe_type = get_attribute(AttributeNames::type); maybe_type.has_value()) + options.type = maybe_type.value(); // FIXME: 6. Assert: options's href is not the empty string, or options's source set is not null. // A link element with neither an href or an imagesrcset does not represent a link. diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp index 3ba61180f0..c30b52f539 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.cpp +++ b/Userland/Libraries/LibWeb/Layout/Node.cpp @@ -858,8 +858,8 @@ DeprecatedString Node::debug_description() const builder.appendff("<{}>", dom_node()->node_name()); if (dom_node()->is_element()) { auto& element = static_cast(*dom_node()); - if (auto id = element.get_attribute(HTML::AttributeNames::id); !id.is_null()) - builder.appendff("#{}", id); + if (auto id = element.get_attribute(HTML::AttributeNames::id); id.has_value()) + builder.appendff("#{}", id.value()); for (auto const& class_name : element.class_names()) builder.appendff(".{}", class_name); } diff --git a/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp index 96678977b7..265216f77b 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) ? get_attribute(AttributeNames::href) : get_attribute("xlink:href"sv); + auto link = has_attribute(AttributeNames::href) ? deprecated_get_attribute(AttributeNames::href) : deprecated_get_attribute("xlink:href"sv); if (auto href = link; !href.is_empty()) { auto url = document().parse_url(href); auto id = url.fragment(); diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp index f858714484..00d7a04206 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.cpp +++ b/Userland/Services/WebContent/ConnectionFromClient.cpp @@ -484,7 +484,7 @@ void ConnectionFromClient::debug_request(DeprecatedString const& request, Deprec load_html("

Failed to find <link rel="match" /> in ref test page!

Make sure you added it."); } else { auto link = maybe_link.release_value(); - auto url = document->parse_url(link->get_attribute(Web::HTML::AttributeNames::href)); + auto url = document->parse_url(link->deprecated_get_attribute(Web::HTML::AttributeNames::href)); load_url(url); } } diff --git a/Userland/Services/WebContent/WebDriverConnection.cpp b/Userland/Services/WebContent/WebDriverConnection.cpp index 9527df963b..fc2e075df2 100644 --- a/Userland/Services/WebContent/WebDriverConnection.cpp +++ b/Userland/Services/WebContent/WebDriverConnection.cpp @@ -1075,7 +1075,7 @@ Messages::WebDriverClient::GetElementAttributeResponse WebDriverConnection::get_ // -> Otherwise else { // The result of getting an attribute by name name. - result = element->get_attribute(deprecated_name); + result = element->deprecated_get_attribute(deprecated_name); } // 5. Return success with data result.