Skip to content

Commit 50350fb

Browse files
shannonboothawesomekling
authored andcommitted
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<String> version of this function.
1 parent ebe01b5 commit 50350fb

17 files changed

+52
-56
lines changed

Userland/Libraries/LibWeb/CSS/StyleComputer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,8 @@ Vector<MatchingRule> StyleComputer::collect_matching_rules(DOM::Element const& e
310310
if (auto it = rule_cache.rules_by_class.find(class_name); it != rule_cache.rules_by_class.end())
311311
add_rules_to_run(it->value);
312312
}
313-
if (auto id = element.get_attribute(HTML::AttributeNames::id); !id.is_null()) {
314-
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())
313+
if (auto id = element.get_attribute(HTML::AttributeNames::id); id.has_value()) {
314+
if (auto it = rule_cache.rules_by_id.find(id.value()); it != rule_cache.rules_by_id.end())
315315
add_rules_to_run(it->value);
316316
}
317317
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())

Userland/Libraries/LibWeb/DOM/DOMTokenList.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ DOMTokenList::DOMTokenList(Element& associated_element, FlyString associated_att
6464
, m_associated_element(associated_element)
6565
, m_associated_attribute(move(associated_attribute))
6666
{
67-
auto value = associated_element.get_attribute(m_associated_attribute.to_deprecated_fly_string());
67+
auto value = associated_element.deprecated_get_attribute(m_associated_attribute);
6868
associated_attribute_changed(value);
6969
}
7070

Userland/Libraries/LibWeb/DOM/Element.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ void Element::visit_edges(Cell::Visitor& visitor)
133133
}
134134

135135
// https://dom.spec.whatwg.org/#dom-element-getattribute
136-
DeprecatedString Element::get_attribute(StringView name) const
136+
Optional<String> Element::get_attribute(StringView name) const
137137
{
138138
// 1. Let attr be the result of getting an attribute given qualifiedName and this.
139139
auto const* attribute = m_attributes->get_attribute(name);
@@ -143,7 +143,16 @@ DeprecatedString Element::get_attribute(StringView name) const
143143
return {};
144144

145145
// 3. Return attr’s value.
146-
return attribute->value().to_deprecated_string();
146+
return attribute->value();
147+
}
148+
149+
DeprecatedString Element::deprecated_get_attribute(StringView name) const
150+
{
151+
auto maybe_attribute = get_attribute(name);
152+
if (!maybe_attribute.has_value())
153+
return {};
154+
155+
return maybe_attribute->to_deprecated_string();
147156
}
148157

149158
// https://dom.spec.whatwg.org/#concept-element-attributes-get-value

Userland/Libraries/LibWeb/DOM/Element.h

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,17 +99,12 @@ class Element
9999
bool has_attributes() const;
100100

101101
// FIXME: This should be taking a 'FlyString const&'
102-
DeprecatedString deprecated_attribute(StringView name) const { return get_attribute(name); }
103-
Optional<String> attribute(StringView name) const
104-
{
105-
auto ret = deprecated_attribute(name);
106-
if (ret.is_null())
107-
return {};
108-
return String::from_deprecated_string(ret).release_value();
109-
}
102+
DeprecatedString deprecated_attribute(StringView name) const { return deprecated_get_attribute(name); }
103+
Optional<String> attribute(StringView name) const { return get_attribute(name); }
110104

111105
// FIXME: This should be taking a 'FlyString const&' / 'Optional<FlyString> const&'
112-
DeprecatedString get_attribute(StringView name) const;
106+
Optional<String> get_attribute(StringView name) const;
107+
DeprecatedString deprecated_get_attribute(StringView name) const;
113108
DeprecatedString get_attribute_value(StringView local_name, DeprecatedFlyString const& namespace_ = {}) const;
114109

115110
WebIDL::ExceptionOr<void> set_attribute(DeprecatedFlyString const& name, DeprecatedString const& value);
@@ -257,7 +252,7 @@ class Element
257252
#define ARIA_IMPL(name, attribute) \
258253
DeprecatedString name() const override \
259254
{ \
260-
return get_attribute(attribute); \
255+
return deprecated_get_attribute(attribute); \
261256
} \
262257
\
263258
WebIDL::ExceptionOr<void> set_##name(DeprecatedString const& value) override \

Userland/Libraries/LibWeb/DOM/Element.idl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ interface Element : Node {
2828
readonly attribute DOMString localName;
2929
readonly attribute DOMString tagName;
3030

31-
DOMString? getAttribute(DOMString qualifiedName);
31+
[ImplementedAs=deprecated_get_attribute] DOMString? getAttribute(DOMString qualifiedName);
3232
[CEReactions] undefined setAttribute(DOMString qualifiedName, DOMString value);
3333
[CEReactions] undefined setAttributeNS(DOMString? namespace , DOMString qualifiedName , DOMString value);
3434
[CEReactions] Attr? setAttributeNode(Attr attr);

Userland/Libraries/LibWeb/DOM/Node.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,7 +1387,7 @@ bool Node::is_equal_node(Node const* other_node) const
13871387
// If A is an element, each attribute in its attribute list has an attribute that equals an attribute in B’s attribute list.
13881388
bool has_same_attributes = true;
13891389
this_element.for_each_attribute([&](auto& name, auto& value) {
1390-
if (other_element.get_attribute(name) != value)
1390+
if (other_element.deprecated_get_attribute(name) != value)
13911391
has_same_attributes = false;
13921392
});
13931393
if (!has_same_attributes)
@@ -1473,8 +1473,8 @@ DeprecatedString Node::debug_description() const
14731473
builder.append(node_name().to_deprecated_fly_string().to_lowercase());
14741474
if (is_element()) {
14751475
auto& element = static_cast<DOM::Element const&>(*this);
1476-
if (auto id = element.get_attribute(HTML::AttributeNames::id); !id.is_null())
1477-
builder.appendff("#{}", id);
1476+
if (auto id = element.get_attribute(HTML::AttributeNames::id); id.has_value())
1477+
builder.appendff("#{}", id.value());
14781478
for (auto const& class_name : element.class_names())
14791479
builder.appendff(".{}", class_name);
14801480
}

Userland/Libraries/LibWeb/DOM/RadioNodeList.cpp

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -60,44 +60,38 @@ FlyString RadioNodeList::value() const
6060
return String {};
6161

6262
// 3. If element is an element with no value attribute, return the string "on".
63-
auto const value = element->get_attribute(HTML::AttributeNames::value);
64-
if (value.is_null())
65-
return "on"_fly_string;
66-
6763
// 4. Otherwise, return the value of element's value attribute.
68-
return MUST(FlyString::from_deprecated_fly_string(value));
64+
return element->get_attribute(HTML::AttributeNames::value).value_or("on"_string);
6965
}
7066

7167
void RadioNodeList::set_value(FlyString const& value)
7268
{
7369
HTML::HTMLInputElement* element = nullptr;
7470

75-
auto deprecated_value = value.to_deprecated_fly_string();
76-
7771
// 1. If the new value is the string "on": let element be the first element in tree order represented by the RadioNodeList object
7872
// that is an input element whose type attribute is in the Radio Button state and whose value content attribute is either absent,
7973
// or present and equal to the new value, if any. If no such element exists, then instead let element be null.
8074
if (value == "on"sv) {
81-
element = verify_cast<HTML::HTMLInputElement>(first_matching([&deprecated_value](auto const& node) {
75+
element = verify_cast<HTML::HTMLInputElement>(first_matching([&value](auto const& node) {
8276
auto const* button = radio_button(node);
8377
if (!button)
8478
return false;
8579

86-
auto const value = button->get_attribute(HTML::AttributeNames::value);
87-
return value.is_null() || value == deprecated_value;
80+
auto const maybe_value = button->get_attribute(HTML::AttributeNames::value);
81+
return !maybe_value.has_value() || maybe_value.value() == value;
8882
}));
8983
}
9084
// 2. Otherwise: let element be the first element in tree order represented by the RadioNodeList object that is an input element whose
91-
// type attribute is in the Radio Button state and whose value content attribute is present and equal to the new value, if any. If
92-
// no such element exists, then instead let element be null.
85+
// type attribute is in the Radio Button state and whose value content attribute is present and equal to the new value, if any. If
86+
// no such element exists, then instead let element be null.
9387
else {
94-
element = verify_cast<HTML::HTMLInputElement>(first_matching([&deprecated_value](auto const& node) {
88+
element = verify_cast<HTML::HTMLInputElement>(first_matching([&value](auto const& node) {
9589
auto const* button = radio_button(node);
9690
if (!button)
9791
return false;
9892

99-
auto const value = button->get_attribute(HTML::AttributeNames::value);
100-
return !value.is_null() && value == deprecated_value;
93+
auto const maybe_value = button->get_attribute(HTML::AttributeNames::value);
94+
return maybe_value.has_value() && maybe_value.value() == value;
10195
}));
10296
}
10397

Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ static bool is_form_control(DOM::Element const& element)
394394
}
395395

396396
if (is<HTMLInputElement>(element)
397-
&& !element.get_attribute(HTML::AttributeNames::type).equals_ignoring_ascii_case("image"sv)) {
397+
&& !element.deprecated_get_attribute(HTML::AttributeNames::type).equals_ignoring_ascii_case("image"sv)) {
398398
return true;
399399
}
400400

Userland/Libraries/LibWeb/HTML/HTMLHeadingElement.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class HTMLHeadingElement final : public HTMLElement {
2525
virtual DeprecatedString aria_level() const override
2626
{
2727
// TODO: aria-level = the number in the element's tag name
28-
return get_attribute("aria-level"sv);
28+
return deprecated_get_attribute("aria-level"sv);
2929
}
3030

3131
private:

Userland/Libraries/LibWeb/HTML/HTMLIFrameElement.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ void HTMLIFrameElement::process_the_iframe_attributes(bool initial_insertion)
9090
}
9191

9292
// 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.
93-
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());
93+
navigate_an_iframe_or_frame(AK::URL("about:srcdoc"sv), ReferrerPolicy::ReferrerPolicy::EmptyString, get_attribute(HTML::AttributeNames::srcdoc));
9494

9595
// FIXME: The resulting Document must be considered an iframe srcdoc document.
9696

Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ unsigned HTMLImageElement::width() const
175175
return paintable_box->content_width().to_int();
176176

177177
// NOTE: This step seems to not be in the spec, but all browsers do it.
178-
auto width_attr = get_attribute(HTML::AttributeNames::width);
178+
auto width_attr = deprecated_get_attribute(HTML::AttributeNames::width);
179179
if (auto converted = width_attr.to_uint(); converted.has_value())
180180
return *converted;
181181

@@ -203,7 +203,7 @@ unsigned HTMLImageElement::height() const
203203
return paintable_box->content_height().to_int();
204204

205205
// NOTE: This step seems to not be in the spec, but all browsers do it.
206-
auto height_attr = get_attribute(HTML::AttributeNames::height);
206+
auto height_attr = deprecated_get_attribute(HTML::AttributeNames::height);
207207
if (auto converted = height_attr.to_uint(); converted.has_value())
208208
return *converted;
209209

Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ DeprecatedString HTMLInputElement::value() const
352352
// https://html.spec.whatwg.org/multipage/input.html#dom-input-value-default-on
353353
if (type_state() == TypeAttributeState::Checkbox || type_state() == TypeAttributeState::RadioButton) {
354354
// On getting, if the element has a value content attribute, return that attribute's value; otherwise, return the string "on".
355-
return has_attribute(AttributeNames::value) ? get_attribute(AttributeNames::value) : "on";
355+
return get_attribute(AttributeNames::value).value_or("on"_string).to_deprecated_string();
356356
}
357357

358358
// https://html.spec.whatwg.org/multipage/input.html#dom-input-value-default
@@ -362,7 +362,7 @@ DeprecatedString HTMLInputElement::value() const
362362
|| type_state() == TypeAttributeState::ResetButton
363363
|| type_state() == TypeAttributeState::Button) {
364364
// On getting, if the element has a value content attribute, return that attribute's value; otherwise, return the empty string.
365-
return has_attribute(AttributeNames::value) ? get_attribute(AttributeNames::value) : DeprecatedString::empty();
365+
return get_attribute(AttributeNames::value).value_or(String {}).to_deprecated_string();
366366
}
367367

368368
// https://html.spec.whatwg.org/multipage/input.html#dom-input-value-value
@@ -934,7 +934,7 @@ void HTMLInputElement::reset_algorithm()
934934
m_dirty_checkedness = false;
935935

936936
// set the value of the element to the value of the value content attribute, if there is one, or the empty string otherwise,
937-
m_value = has_attribute(AttributeNames::value) ? get_attribute(AttributeNames::value) : DeprecatedString::empty();
937+
m_value = get_attribute(AttributeNames::value).value_or(String {}).to_deprecated_string();
938938

939939
// set the checkedness of the element to true if the element has a checked content attribute and false if it does not,
940940
m_checked = has_attribute(AttributeNames::checked);

Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,7 @@ HTMLLinkElement::LinkProcessingOptions HTMLLinkElement::create_link_options()
160160
LinkProcessingOptions options;
161161
// FIXME: destination the result of translating the state of el's as attribute
162162
// crossorigin the state of el's crossorigin content attribute
163-
options.crossorigin = cors_setting_attribute_from_keyword(
164-
has_attribute(AttributeNames::crossorigin) ? String::from_deprecated_string(get_attribute(AttributeNames::crossorigin)).release_value_but_fixme_should_propagate_errors()
165-
: Optional<String> {});
163+
options.crossorigin = cors_setting_attribute_from_keyword(get_attribute(AttributeNames::crossorigin));
166164
// FIXME: referrer policy the state of el's referrerpolicy content attribute
167165
// FIXME: source set el's source set
168166
// base URL document's URL
@@ -178,16 +176,16 @@ HTMLLinkElement::LinkProcessingOptions HTMLLinkElement::create_link_options()
178176
// FIXME: cryptographic nonce metadata The current value of el's [[CryptographicNonce]] internal slot
179177

180178
// 3. If el has an href attribute, then set options's href to the value of el's href attribute.
181-
if (has_attribute(AttributeNames::href))
182-
options.href = String::from_deprecated_string(get_attribute(AttributeNames::href)).release_value_but_fixme_should_propagate_errors();
179+
if (auto maybe_href = get_attribute(AttributeNames::href); maybe_href.has_value())
180+
options.href = maybe_href.value();
183181

184182
// 4. If el has an integrity attribute, then set options's integrity to the value of el's integrity content attribute.
185-
if (has_attribute(AttributeNames::integrity))
186-
options.integrity = String::from_deprecated_string(get_attribute(AttributeNames::integrity)).release_value_but_fixme_should_propagate_errors();
183+
if (auto maybe_integrity = get_attribute(AttributeNames::integrity); maybe_integrity.has_value())
184+
options.integrity = maybe_integrity.value();
187185

188186
// 5. If el has a type attribute, then set options's type to the value of el's type attribute.
189-
if (has_attribute(AttributeNames::type))
190-
options.type = String::from_deprecated_string(get_attribute(AttributeNames::type)).release_value_but_fixme_should_propagate_errors();
187+
if (auto maybe_type = get_attribute(AttributeNames::type); maybe_type.has_value())
188+
options.type = maybe_type.value();
191189

192190
// FIXME: 6. Assert: options's href is not the empty string, or options's source set is not null.
193191
// A link element with neither an href or an imagesrcset does not represent a link.

Userland/Libraries/LibWeb/Layout/Node.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -858,8 +858,8 @@ DeprecatedString Node::debug_description() const
858858
builder.appendff("<{}>", dom_node()->node_name());
859859
if (dom_node()->is_element()) {
860860
auto& element = static_cast<DOM::Element const&>(*dom_node());
861-
if (auto id = element.get_attribute(HTML::AttributeNames::id); !id.is_null())
862-
builder.appendff("#{}", id);
861+
if (auto id = element.get_attribute(HTML::AttributeNames::id); id.has_value())
862+
builder.appendff("#{}", id.value());
863863
for (auto const& class_name : element.class_names())
864864
builder.appendff(".{}", class_name);
865865
}

Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ JS::GCPtr<SVGGradientElement const> SVGGradientElement::linked_gradient() const
9494
// FIXME: This entire function is an ad-hoc hack!
9595
// It can only resolve #<ids> in the same document.
9696

97-
auto link = has_attribute(AttributeNames::href) ? get_attribute(AttributeNames::href) : get_attribute("xlink:href"sv);
97+
auto link = has_attribute(AttributeNames::href) ? deprecated_get_attribute(AttributeNames::href) : deprecated_get_attribute("xlink:href"sv);
9898
if (auto href = link; !href.is_empty()) {
9999
auto url = document().parse_url(href);
100100
auto id = url.fragment();

Userland/Services/WebContent/ConnectionFromClient.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ void ConnectionFromClient::debug_request(DeprecatedString const& request, Deprec
484484
load_html("<h1>Failed to find &lt;link rel=&quot;match&quot; /&gt; in ref test page!</h1> Make sure you added it.");
485485
} else {
486486
auto link = maybe_link.release_value();
487-
auto url = document->parse_url(link->get_attribute(Web::HTML::AttributeNames::href));
487+
auto url = document->parse_url(link->deprecated_get_attribute(Web::HTML::AttributeNames::href));
488488
load_url(url);
489489
}
490490
}

Userland/Services/WebContent/WebDriverConnection.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1075,7 +1075,7 @@ Messages::WebDriverClient::GetElementAttributeResponse WebDriverConnection::get_
10751075
// -> Otherwise
10761076
else {
10771077
// The result of getting an attribute by name name.
1078-
result = element->get_attribute(deprecated_name);
1078+
result = element->deprecated_get_attribute(deprecated_name);
10791079
}
10801080

10811081
// 5. Return success with data result.

0 commit comments

Comments
 (0)