LibWeb: Merge did_remove_attribute() into attribute_changed()

Instead of having two virtuals for attribute change notifications,
there is now only one. When the attribute is removed, the value is null.

Userland/Libraries/LibWeb/DOM/Element.cpp

@@ -227,9 +227,7 @@ WebIDL::ExceptionOr<JS::GCPtr<Attr>> Element::set_attribute_node_ns(Attr& attr)
 void Element::remove_attribute(DeprecatedFlyString const& name)
 {
     m_attributes->remove_attribute(name);
-
+    attribute_changed(name, {});
-    did_remove_attribute(name);
-
     invalidate_style_after_attribute_change(name);
 }
 
@@ -275,9 +273,7 @@ WebIDL::ExceptionOr<bool> Element::toggle_attribute(DeprecatedFlyString const& n
     // 5. Otherwise, if force is not given or is false, remove an attribute given qualifiedName and this, and then return false.
     if (!force.has_value() || !force.value()) {
         m_attributes->remove_attribute(name);
-
+        attribute_changed(name, {});
-        did_remove_attribute(name);
-
         invalidate_style_after_attribute_change(name);
     }
 
@@ -373,19 +369,16 @@ void Element::attribute_changed(DeprecatedFlyString const& name, DeprecatedStrin
         if (m_class_list)
             m_class_list->associated_attribute_changed(value);
     } else if (name == HTML::AttributeNames::style) {
-        // https://drafts.csswg.org/cssom/#ref-for-cssstyledeclaration-updating-flag
+        if (value.is_null()) {
-        if (m_inline_style && m_inline_style->is_updating())
+            if (!m_inline_style) {
-            return;
+                m_inline_style = nullptr;
-        m_inline_style = parse_css_style_attribute(CSS::Parser::ParsingContext(document()), value, *this);
+                set_needs_style_update(true);
-        set_needs_style_update(true);
+            }
-    }
+        } else {
-}
+            // https://drafts.csswg.org/cssom/#ref-for-cssstyledeclaration-updating-flag
-
+            if (m_inline_style && m_inline_style->is_updating())
-void Element::did_remove_attribute(DeprecatedFlyString const& name)
+                return;
-{
+            m_inline_style = parse_css_style_attribute(CSS::Parser::ParsingContext(document()), value, *this);
-    if (name == HTML::AttributeNames::style) {
-        if (m_inline_style) {
-            m_inline_style = nullptr;
             set_needs_style_update(true);
         }
     }

Userland/Libraries/LibWeb/DOM/Element.h

@@ -122,7 +122,6 @@ public:
 
     virtual void apply_presentational_hints(CSS::StyleProperties&) const { }
     virtual void attribute_changed(DeprecatedFlyString const& name, DeprecatedString const& value);
-    virtual void did_remove_attribute(DeprecatedFlyString const&);
 
     struct [[nodiscard]] RequiredInvalidationAfterStyleChange {
         bool repaint { false };

Userland/Libraries/LibWeb/HTML/HTMLElement.cpp

@@ -233,15 +233,19 @@ void HTMLElement::attribute_changed(DeprecatedFlyString const& name, DeprecatedS
     Element::attribute_changed(name, value);
 
     if (name == HTML::AttributeNames::contenteditable) {
-        if ((!value.is_null() && value.is_empty()) || value.equals_ignoring_ascii_case("true"sv)) {
+        if (value.is_null()) {
-            // "true", an empty string or a missing value map to the "true" state.
-            m_content_editable_state = ContentEditableState::True;
-        } else if (value.equals_ignoring_ascii_case("false"sv)) {
-            // "false" maps to the "false" state.
-            m_content_editable_state = ContentEditableState::False;
-        } else {
-            // Having no such attribute or an invalid value maps to the "inherit" state.
             m_content_editable_state = ContentEditableState::Inherit;
+        } else {
+            if (value.is_empty() || value.equals_ignoring_ascii_case("true"sv)) {
+                // "true", an empty string or a missing value map to the "true" state.
+                m_content_editable_state = ContentEditableState::True;
+            } else if (value.equals_ignoring_ascii_case("false"sv)) {
+                // "false" maps to the "false" state.
+                m_content_editable_state = ContentEditableState::False;
+            } else {
+                // Having no such attribute or an invalid value maps to the "inherit" state.
+                m_content_editable_state = ContentEditableState::Inherit;
+            }
         }
     }
 
@@ -256,14 +260,6 @@ void HTMLElement::attribute_changed(DeprecatedFlyString const& name, DeprecatedS
 #undef __ENUMERATE
 }
 
-void HTMLElement::did_remove_attribute(DeprecatedFlyString const& name)
-{
-    Base::did_remove_attribute(name);
-    if (name == HTML::AttributeNames::contenteditable) {
-        m_content_editable_state = ContentEditableState::Inherit;
-    }
-}
-
 // https://html.spec.whatwg.org/multipage/interaction.html#dom-focus
 void HTMLElement::focus()
 {

Userland/Libraries/LibWeb/HTML/HTMLElement.h

@@ -71,7 +71,6 @@ protected:
     virtual JS::ThrowCompletionOr<void> initialize(JS::Realm&) override;
 
     virtual void attribute_changed(DeprecatedFlyString const& name, DeprecatedString const& value) override;
-    virtual void did_remove_attribute(DeprecatedFlyString const& name) override;
 
     virtual void visit_edges(Cell::Visitor&) override;
 

Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp

@@ -80,7 +80,11 @@ void HTMLImageElement::attribute_changed(DeprecatedFlyString const& name, Deprec
     HTMLElement::attribute_changed(name, value);
 
     if (name == HTML::AttributeNames::crossorigin) {
-        m_cors_setting = cors_setting_attribute_from_keyword(String::from_deprecated_string(value).release_value_but_fixme_should_propagate_errors());
+        if (value.is_null()) {
+            m_cors_setting = CORSSettingAttribute::NoCORS;
+        } else {
+            m_cors_setting = cors_setting_attribute_from_keyword(String::from_deprecated_string(value).release_value_but_fixme_should_propagate_errors());
+        }
     }
 
     if (name.is_one_of(HTML::AttributeNames::src, HTML::AttributeNames::srcset)) {
@@ -93,15 +97,6 @@ void HTMLImageElement::attribute_changed(DeprecatedFlyString const& name, Deprec
     }
 }
 
-void HTMLImageElement::did_remove_attribute(DeprecatedFlyString const& name)
-{
-    Base::did_remove_attribute(name);
-
-    if (name == HTML::AttributeNames::crossorigin) {
-        m_cors_setting = CORSSettingAttribute::NoCORS;
-    }
-}
-
 JS::GCPtr<Layout::Node> HTMLImageElement::create_layout_node(NonnullRefPtr<CSS::StyleProperties> style)
 {
     return heap().allocate_without_realm<Layout::ImageBox>(document(), *this, move(style), *this);

Userland/Libraries/LibWeb/HTML/HTMLImageElement.h

@@ -29,7 +29,6 @@ public:
     virtual ~HTMLImageElement() override;
 
     virtual void attribute_changed(DeprecatedFlyString const& name, DeprecatedString const& value) override;
-    virtual void did_remove_attribute(DeprecatedFlyString const& name) override;
 
     DeprecatedString alt() const { return attribute(HTML::AttributeNames::alt); }
     DeprecatedString src() const { return attribute(HTML::AttributeNames::src); }

Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp

@@ -497,16 +497,30 @@ void HTMLInputElement::attribute_changed(DeprecatedFlyString const& name, Deprec
 {
     HTMLElement::attribute_changed(name, value);
     if (name == HTML::AttributeNames::checked) {
-        // When the checked content attribute is added, if the control does not have dirty checkedness,
+        if (value.is_null()) {
-        // the user agent must set the checkedness of the element to true
+            // When the checked content attribute is removed, if the control does not have dirty checkedness,
-        if (!m_dirty_checkedness)
+            // the user agent must set the checkedness of the element to false.
-            set_checked(true, ChangeSource::Programmatic);
+            if (!m_dirty_checkedness)
+                set_checked(false, ChangeSource::Programmatic);
+        } else {
+            // When the checked content attribute is added, if the control does not have dirty checkedness,
+            // the user agent must set the checkedness of the element to true
+            if (!m_dirty_checkedness)
+                set_checked(true, ChangeSource::Programmatic);
+        }
     } else if (name == HTML::AttributeNames::type) {
         m_type = parse_type_attribute(value);
     } else if (name == HTML::AttributeNames::value) {
-        if (!m_dirty_value) {
+        if (value.is_null()) {
-            m_value = value_sanitization_algorithm(value);
+            if (!m_dirty_value) {
-            update_placeholder_visibility();
+                m_value = DeprecatedString::empty();
+                update_placeholder_visibility();
+            }
+        } else {
+            if (!m_dirty_value) {
+                m_value = value_sanitization_algorithm(value);
+                update_placeholder_visibility();
+            }
         }
     } else if (name == HTML::AttributeNames::placeholder) {
         if (m_placeholder_text_node)
@@ -528,25 +542,6 @@ HTMLInputElement::TypeAttributeState HTMLInputElement::parse_type_attribute(Stri
     return HTMLInputElement::TypeAttributeState::Text;
 }
 
-void HTMLInputElement::did_remove_attribute(DeprecatedFlyString const& name)
-{
-    HTMLElement::did_remove_attribute(name);
-    if (name == HTML::AttributeNames::checked) {
-        // When the checked content attribute is removed, if the control does not have dirty checkedness,
-        // the user agent must set the checkedness of the element to false.
-        if (!m_dirty_checkedness)
-            set_checked(false, ChangeSource::Programmatic);
-    } else if (name == HTML::AttributeNames::value) {
-        if (!m_dirty_value) {
-            m_value = DeprecatedString::empty();
-            update_placeholder_visibility();
-        }
-    } else if (name == HTML::AttributeNames::placeholder) {
-        if (m_placeholder_text_node)
-            m_placeholder_text_node->set_data({});
-    }
-}
-
 DeprecatedString HTMLInputElement::type() const
 {
     switch (m_type) {
@@ -1104,5 +1099,4 @@ bool HTMLInputElement::is_submit_button() const
     return type_state() == TypeAttributeState::SubmitButton
         || type_state() == TypeAttributeState::ImageButton;
 }
-
 }

Userland/Libraries/LibWeb/HTML/HTMLInputElement.h

@@ -105,7 +105,6 @@ public:
 
     // ^HTMLElement
     virtual void attribute_changed(DeprecatedFlyString const&, DeprecatedString const&) override;
-    virtual void did_remove_attribute(DeprecatedFlyString const&) override;
 
     // ^FormAssociatedElement
     // https://html.spec.whatwg.org/multipage/forms.html#category-listed

Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp

@@ -149,25 +149,6 @@ void HTMLLinkElement::resource_did_load()
     }
 }
 
-void HTMLLinkElement::did_remove_attribute(DeprecatedFlyString const& attr)
-{
-    if (m_relationship & Relationship::Stylesheet) {
-        // https://html.spec.whatwg.org/multipage/links.html#link-type-stylesheet:fetch-and-process-the-linked-resource
-        // The appropriate times to fetch and process this type of link are:
-        if (
-            // - When the href attribute of the link element of an external resource link that is already browsing-context connected is changed.
-            attr == AttributeNames::href ||
-            // - When the disabled attribute of the link element of an external resource link that is already browsing-context connected is set, changed, or removed.
-            attr == AttributeNames::disabled ||
-            // - When the crossorigin attribute of the link element of an external resource link that is already browsing-context connected is set, changed, or removed.
-            attr == AttributeNames::crossorigin
-            // FIXME: - When the type attribute of the link element of an external resource link that is already browsing-context connected, but was previously not obtained due to the type attribute specifying an unsupported type, is removed or changed.
-        ) {
-            fetch_and_process_linked_resource();
-        }
-    }
-}
-
 // https://html.spec.whatwg.org/multipage/semantics.html#create-link-options-from-element
 HTMLLinkElement::LinkProcessingOptions HTMLLinkElement::create_link_options()
 {

Userland/Libraries/LibWeb/HTML/HTMLLinkElement.h

@@ -46,7 +46,6 @@ private:
     virtual void resource_did_load() override;
 
     // ^ HTMLElement
-    virtual void did_remove_attribute(DeprecatedFlyString const&) override;
     virtual void visit_edges(Cell::Visitor&) override;
 
     struct LinkProcessingOptions {

Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp

@@ -88,20 +88,16 @@ void HTMLMediaElement::attribute_changed(DeprecatedFlyString const& name, Deprec
 {
     Base::attribute_changed(name, value);
 
-    if (name == HTML::AttributeNames::src)
+    if (name == HTML::AttributeNames::src) {
         load_element().release_value_but_fixme_should_propagate_errors();
-    else if (name == HTML::AttributeNames::crossorigin)
+    } else if (name == HTML::AttributeNames::crossorigin) {
-        m_crossorigin = cors_setting_attribute_from_keyword(String::from_deprecated_string(value).release_value_but_fixme_should_propagate_errors());
+        if (value.is_null())
-    else if (name == HTML::AttributeNames::muted)
+            m_crossorigin = cors_setting_attribute_from_keyword({});
+        else
+            m_crossorigin = cors_setting_attribute_from_keyword(String::from_deprecated_string(value).release_value_but_fixme_should_propagate_errors());
+    } else if (name == HTML::AttributeNames::muted) {
         set_muted(true);
-}
+    }
-
-void HTMLMediaElement::did_remove_attribute(DeprecatedFlyString const& name)
-{
-    Base::did_remove_attribute(name);
-
-    if (name == HTML::AttributeNames::crossorigin)
-        m_crossorigin = cors_setting_attribute_from_keyword({});
 }
 
 // https://html.spec.whatwg.org/multipage/media.html#playing-the-media-resource:media-element-83

Userland/Libraries/LibWeb/HTML/HTMLMediaElement.h

@@ -125,7 +125,6 @@ protected:
     virtual void visit_edges(Cell::Visitor&) override;
 
     virtual void attribute_changed(DeprecatedFlyString const& name, DeprecatedString const& value) override;
-    virtual void did_remove_attribute(DeprecatedFlyString const&) override;
     virtual void removed_from(DOM::Node*) override;
     virtual void children_changed() override;
 

Userland/Libraries/LibWeb/HTML/HTMLOptionElement.cpp

@@ -38,22 +38,17 @@ void HTMLOptionElement::attribute_changed(DeprecatedFlyString const& name, Depre
     HTMLElement::attribute_changed(name, value);
 
     if (name == HTML::AttributeNames::selected) {
-        // Except where otherwise specified, when the element is created, its selectedness must be set to true
+        if (value.is_null()) {
-        // if the element has a selected attribute. Whenever an option element's selected attribute is added,
+            // Whenever an option element's selected attribute is removed, if its dirtiness is false, its selectedness must be set to false.
-        // if its dirtiness is false, its selectedness must be set to true.
+            if (!m_dirty)
-        if (!m_dirty)
+                m_selected = false;
-            m_selected = true;
+        } else {
-    }
+            // Except where otherwise specified, when the element is created, its selectedness must be set to true
-}
+            // if the element has a selected attribute. Whenever an option element's selected attribute is added,
-
+            // if its dirtiness is false, its selectedness must be set to true.
-void HTMLOptionElement::did_remove_attribute(DeprecatedFlyString const& name)
+            if (!m_dirty)
-{
+                m_selected = true;
-    HTMLElement::did_remove_attribute(name);
+        }
-
-    if (name == HTML::AttributeNames::selected) {
-        // Whenever an option element's selected attribute is removed, if its dirtiness is false, its selectedness must be set to false.
-        if (!m_dirty)
-            m_selected = false;
     }
 }
 

Userland/Libraries/LibWeb/HTML/HTMLOptionElement.h

@@ -41,7 +41,6 @@ private:
     virtual JS::ThrowCompletionOr<void> initialize(JS::Realm&) override;
 
     void attribute_changed(DeprecatedFlyString const& name, DeprecatedString const& value) override;
-    void did_remove_attribute(DeprecatedFlyString const& name) override;
 
     void ask_for_a_reset();
 

Userland/Libraries/LibWeb/HTML/HTMLScriptElement.cpp

@@ -51,20 +51,17 @@ void HTMLScriptElement::attribute_changed(DeprecatedFlyString const& name, Depre
 {
     Base::attribute_changed(name, value);
 
-    if (name == HTML::AttributeNames::crossorigin)
+    if (name == HTML::AttributeNames::crossorigin) {
-        m_crossorigin = cors_setting_attribute_from_keyword(String::from_deprecated_string(value).release_value_but_fixme_should_propagate_errors());
+        if (value.is_null())
-    else if (name == HTML::AttributeNames::referrerpolicy)
+            m_crossorigin = cors_setting_attribute_from_keyword({});
-        m_referrer_policy = ReferrerPolicy::from_string(value);
+        else
-}
+            m_crossorigin = cors_setting_attribute_from_keyword(String::from_deprecated_string(value).release_value_but_fixme_should_propagate_errors());
-
+    } else if (name == HTML::AttributeNames::referrerpolicy) {
-void HTMLScriptElement::did_remove_attribute(DeprecatedFlyString const& name)
+        if (value.is_null())
-{
+            m_referrer_policy.clear();
-    Base::did_remove_attribute(name);
+        else
-
+            m_referrer_policy = ReferrerPolicy::from_string(value);
-    if (name == HTML::AttributeNames::crossorigin)
+    }
-        m_crossorigin = cors_setting_attribute_from_keyword({});
-    else if (name == HTML::AttributeNames::referrerpolicy)
-        m_referrer_policy.clear();
 }
 
 void HTMLScriptElement::begin_delaying_document_load_event(DOM::Document& document)

Userland/Libraries/LibWeb/HTML/HTMLScriptElement.h

@@ -63,7 +63,6 @@ private:
     virtual void visit_edges(Cell::Visitor&) override;
 
     virtual void attribute_changed(DeprecatedFlyString const& name, DeprecatedString const& value) override;
-    virtual void did_remove_attribute(DeprecatedFlyString const&) override;
 
     // https://html.spec.whatwg.org/multipage/scripting.html#prepare-the-script-element
     void prepare_script();

Userland/Libraries/LibWeb/HTML/HTMLVideoElement.cpp

@@ -46,16 +46,12 @@ void HTMLVideoElement::attribute_changed(DeprecatedFlyString const& name, Deprec
 {
     Base::attribute_changed(name, value);
 
-    if (name == HTML::AttributeNames::poster)
+    if (name == HTML::AttributeNames::poster) {
-        determine_element_poster_frame(value).release_value_but_fixme_should_propagate_errors();
+        if (value.is_null())
-}
+            determine_element_poster_frame({}).release_value_but_fixme_should_propagate_errors();
-
+        else
-void HTMLVideoElement::did_remove_attribute(DeprecatedFlyString const& name)
+            determine_element_poster_frame(value).release_value_but_fixme_should_propagate_errors();
-{
+    }
-    Base::did_remove_attribute(name);
-
-    if (name == HTML::AttributeNames::poster)
-        determine_element_poster_frame({}).release_value_but_fixme_should_propagate_errors();
 }
 
 JS::GCPtr<Layout::Node> HTMLVideoElement::create_layout_node(NonnullRefPtr<CSS::StyleProperties> style)

Userland/Libraries/LibWeb/HTML/HTMLVideoElement.h

@@ -48,7 +48,6 @@ private:
     virtual void visit_edges(Cell::Visitor&) override;
 
     virtual void attribute_changed(DeprecatedFlyString const& name, DeprecatedString const& value) override;
-    virtual void did_remove_attribute(DeprecatedFlyString const&) override;
 
     virtual JS::GCPtr<Layout::Node> create_layout_node(NonnullRefPtr<CSS::StyleProperties>) override;