LibWeb: Unset stylesheet properties when removing from a StyleSheetList

Previously, the parent CSS stylesheet, owner node and owner CSS rule
properties were not unset when removing a sheet from a StyleSheetList.

This change moves the methods for adding and removing sheets to and
from a StyleSheetList, directly into the StyleSheetList class and
ensures they are called as required by the CSSOM specification.

Tests/LibWeb/Text/expected/HTMLLinkElement-disabled.txt

@@ -5,6 +5,8 @@ onload event fired
 link.disabled: false
 document.styleSheets.length: 1
 background color: rgb(0, 128, 0)
+sheet.ownerNode is link element: true
 document.styleSheets.length after link disabled again: 0
 link.disabled after link disabled again: true
 background color after link disabled again: rgba(0, 0, 0, 0)
+sheet.ownerNode is null: true

Tests/LibWeb/Text/input/HTMLLinkElement-disabled.html

@@ -14,11 +14,14 @@
  println(`link.disabled: ${link.disabled}`);
  println(`document.styleSheets.length: ${document.styleSheets.length}`);
  println(`background color: ${documentStyle.backgroundColor}`);
+ const sheet = document.styleSheets[0];
+ println(`sheet.ownerNode is link element: ${sheet.ownerNode === link}`);
 
  link.disabled = true;
  println(`document.styleSheets.length after link disabled again: ${document.styleSheets.length}`);
  println(`link.disabled after link disabled again: ${link.disabled}`);
  println(`background color after link disabled again: ${documentStyle.backgroundColor}`);
+ println(`sheet.ownerNode is null: ${sheet.ownerNode === null}`);
 
  done();
  };

Userland/Libraries/LibWeb/CSS/StyleSheetList.cpp

@@ -15,6 +15,58 @@ namespace Web::CSS {
 
 JS_DEFINE_ALLOCATOR(StyleSheetList);
 
+// https://www.w3.org/TR/cssom/#remove-a-css-style-sheet
+void StyleSheetList::remove_a_css_style_sheet(CSS::CSSStyleSheet& sheet)
+{
+ // 1. Remove the CSS style sheet from the list of document or shadow root CSS style sheets.
+ remove_sheet(sheet);
+
+ // 2. Set the CSS style sheet’s parent CSS style sheet, owner node and owner CSS rule to null.
+ sheet.set_parent_css_style_sheet(nullptr);
+ sheet.set_owner_node(nullptr);
+ sheet.set_owner_css_rule(nullptr);
+}
+
+// https://www.w3.org/TR/cssom/#add-a-css-style-sheet
+void StyleSheetList::add_a_css_style_sheet(CSS::CSSStyleSheet& sheet)
+{
+ // 1. Add the CSS style sheet to the list of document or shadow root CSS style sheets at the appropriate location. The remainder of these steps deal with the disabled flag.
+ add_sheet(sheet);
+
+ // 2. If the disabled flag is set, then return.
+ if (sheet.disabled())
+ return;
+
+ // FIXME: 3. If the title is not the empty string, the alternate flag is unset, and preferred CSS style sheet set name is the empty string change the preferred CSS style sheet set name to the title.
+
+ // FIXME: 4. If any of the following is true, then unset the disabled flag and return:
+ // The title is the empty string.
+ // The last CSS style sheet set name is null and the title is a case-sensitive match for the preferred CSS style sheet set name.
+ // The title is a case-sensitive match for the last CSS style sheet set name.
+
+ // FIXME: 5. Set the disabled flag.
+}
+
+// https://www.w3.org/TR/cssom/#create-a-css-style-sheet
+void StyleSheetList::create_a_css_style_sheet(String type, DOM::Element* owner_node, String media, String title, bool alternate, bool origin_clean, Optional<String> location, CSS::CSSStyleSheet* parent_style_sheet, CSS::CSSRule* owner_rule, CSS::CSSStyleSheet& sheet)
+{
+ // 1. Create a new CSS style sheet object and set its properties as specified.
+ // FIXME: We receive `sheet` from the caller already. This is weird.
+
+ sheet.set_parent_css_style_sheet(parent_style_sheet);
+ sheet.set_owner_css_rule(owner_rule);
+ sheet.set_owner_node(owner_node);
+ sheet.set_type(move(type));
+ sheet.set_media(move(media));
+ sheet.set_title(move(title));
+ sheet.set_alternate(alternate);
+ sheet.set_origin_clean(origin_clean);
+ sheet.set_location(move(location));
+
+ // 2. Then run the add a CSS style sheet steps for the newly created CSS style sheet.
+ add_a_css_style_sheet(sheet);
+}
+
 void StyleSheetList::add_sheet(CSSStyleSheet& sheet)
 {
  sheet.set_style_sheet_list({}, this);

Userland/Libraries/LibWeb/CSS/StyleSheetList.h

@@ -19,8 +19,9 @@ class StyleSheetList final : public Bindings::PlatformObject {
 public:
  [[nodiscard]] static JS::NonnullGCPtr<StyleSheetList> create(DOM::Document&);
 
- void add_sheet(CSSStyleSheet&);
- void remove_sheet(CSSStyleSheet&);
+ void add_a_css_style_sheet(CSS::CSSStyleSheet&);
+ void remove_a_css_style_sheet(CSS::CSSStyleSheet&);
+ void create_a_css_style_sheet(String type, DOM::Element* owner_node, String media, String title, bool alternate, bool origin_clean, Optional<String> location, CSS::CSSStyleSheet* parent_style_sheet, CSS::CSSRule* owner_rule, CSS::CSSStyleSheet&);
 
  Vector<JS::NonnullGCPtr<CSSStyleSheet>> const& sheets() const { return m_sheets; }
  Vector<JS::NonnullGCPtr<CSSStyleSheet>>& sheets() { return m_sheets; }
@@ -46,6 +47,9 @@ class StyleSheetList final : public Bindings::PlatformObject {
  virtual void initialize(JS::Realm&) override;
  virtual void visit_edges(Cell::Visitor&) override;
 
+ void add_sheet(CSSStyleSheet&);
+ void remove_sheet(CSSStyleSheet&);
+
  void sort_sheets();
 
  JS::NonnullGCPtr<DOM::Document> m_document;

Userland/Libraries/LibWeb/DOM/StyleElementUtils.cpp

@@ -33,7 +33,7 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element)
  // 2. If element has an associated CSS style sheet, remove the CSS style sheet in question.
 
  if (m_associated_css_style_sheet) {
- remove_a_css_style_sheet(style_element.document_or_shadow_root_style_sheets(), *m_associated_css_style_sheet);
+ style_element.document_or_shadow_root_style_sheets().remove_a_css_style_sheet(*m_associated_css_style_sheet);
 
  // FIXME: This should probably be handled by StyleSheet::set_owner_node().
  m_associated_css_style_sheet = nullptr;
@@ -60,8 +60,7 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element)
  m_associated_css_style_sheet = sheet;
 
  // 6. Create a CSS style sheet with the following properties...
- create_a_css_style_sheet(
- style_element.document_or_shadow_root_style_sheets(),
+ style_element.document_or_shadow_root_style_sheets().create_a_css_style_sheet(
  "text/css"_string,
  &style_element,
  style_element.attribute(HTML::AttributeNames::media).value_or({}),
@@ -76,56 +75,4 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element)
  *sheet);
 }
 
-// https://www.w3.org/TR/cssom/#remove-a-css-style-sheet
-void StyleElementUtils::remove_a_css_style_sheet(CSS::StyleSheetList& style_sheets, CSS::CSSStyleSheet& sheet)
-{
- // 1. Remove the CSS style sheet from the list of document or shadow root CSS style sheets.
- style_sheets.remove_sheet(sheet);
-
- // 2. Set the CSS style sheet’s parent CSS style sheet, owner node and owner CSS rule to null.
- sheet.set_parent_css_style_sheet(nullptr);
- sheet.set_owner_node(nullptr);
- sheet.set_owner_css_rule(nullptr);
-}
-
-// https://www.w3.org/TR/cssom/#create-a-css-style-sheet
-void StyleElementUtils::create_a_css_style_sheet(CSS::StyleSheetList& style_sheets, String type, DOM::Element* owner_node, String media, String title, bool alternate, bool origin_clean, Optional<String> location, CSS::CSSStyleSheet* parent_style_sheet, CSS::CSSRule* owner_rule, CSS::CSSStyleSheet& sheet)
-{
- // 1. Create a new CSS style sheet object and set its properties as specified.
- // FIXME: We receive `sheet` from the caller already. This is weird.
-
- sheet.set_parent_css_style_sheet(parent_style_sheet);
- sheet.set_owner_css_rule(owner_rule);
- sheet.set_owner_node(owner_node);
- sheet.set_type(move(type));
- sheet.set_media(move(media));
- sheet.set_title(move(title));
- sheet.set_alternate(alternate);
- sheet.set_origin_clean(origin_clean);
- sheet.set_location(move(location));
-
- // 2. Then run the add a CSS style sheet steps for the newly created CSS style sheet.
- add_a_css_style_sheet(style_sheets, sheet);
-}
-
-// https://www.w3.org/TR/cssom/#add-a-css-style-sheet
-void StyleElementUtils::add_a_css_style_sheet(CSS::StyleSheetList& style_sheets, CSS::CSSStyleSheet& sheet)
-{
- // 1. Add the CSS style sheet to the list of document or shadow root CSS style sheets at the appropriate location. The remainder of these steps deal with the disabled flag.
- style_sheets.add_sheet(sheet);
-
- // 2. If the disabled flag is set, then return.
- if (sheet.disabled())
- return;
-
- // FIXME: 3. If the title is not the empty string, the alternate flag is unset, and preferred CSS style sheet set name is the empty string change the preferred CSS style sheet set name to the title.
-
- // FIXME: 4. If any of the following is true, then unset the disabled flag and return:
- // The title is the empty string.
- // The last CSS style sheet set name is null and the title is a case-sensitive match for the preferred CSS style sheet set name.
- // The title is a case-sensitive match for the last CSS style sheet set name.
-
- // FIXME: 5. Set the disabled flag.
-}
-
 }

Userland/Libraries/LibWeb/DOM/StyleElementUtils.h

@@ -20,10 +20,6 @@ class StyleElementUtils {
  CSS::CSSStyleSheet const* sheet() const { return m_associated_css_style_sheet; }
 
 private:
- void remove_a_css_style_sheet(CSS::StyleSheetList&, CSS::CSSStyleSheet& sheet);
- void create_a_css_style_sheet(CSS::StyleSheetList&, String type, DOM::Element* owner_node, String media, String title, bool alternate, bool origin_clean, Optional<String> location, CSS::CSSStyleSheet* parent_style_sheet, CSS::CSSRule* owner_rule, CSS::CSSStyleSheet& sheet);
- void add_a_css_style_sheet(CSS::StyleSheetList&, CSS::CSSStyleSheet& sheet);
-
  // https://www.w3.org/TR/cssom/#associated-css-style-sheet
  JS::GCPtr<CSS::CSSStyleSheet> m_associated_css_style_sheet;
 };

Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp

@@ -113,7 +113,7 @@ void HTMLLinkElement::attribute_changed(FlyString const& name, Optional<String>
  // FIXME: Handle alternate stylesheets properly
  if (m_relationship & Relationship::Stylesheet && !(m_relationship & Relationship::Alternate)) {
  if (name == HTML::AttributeNames::disabled && m_loaded_style_sheet)
- document_or_shadow_root_style_sheets().remove_sheet(*m_loaded_style_sheet);
+ document_or_shadow_root_style_sheets().remove_a_css_style_sheet(*m_loaded_style_sheet);
 
  // 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:
@@ -312,7 +312,7 @@ void HTMLLinkElement::process_stylesheet_resource(bool success, Fetch::Infrastru
 
  // 3. If el has an associated CSS style sheet, remove the CSS style sheet.
  if (m_loaded_style_sheet) {
- document_or_shadow_root_style_sheets().remove_sheet(*m_loaded_style_sheet);
+ document_or_shadow_root_style_sheets().remove_a_css_style_sheet(*m_loaded_style_sheet);
  m_loaded_style_sheet = nullptr;
  }
 
@@ -369,9 +369,17 @@ void HTMLLinkElement::process_stylesheet_resource(bool success, Fetch::Infrastru
  m_loaded_style_sheet = parse_css_stylesheet(CSS::Parser::ParsingContext(document(), *response.url()), decoded_string);
 
  if (m_loaded_style_sheet) {
- m_loaded_style_sheet->set_owner_node(this);
- m_loaded_style_sheet->set_media(attribute(HTML::AttributeNames::media).value_or({}));
- document().style_sheets().add_sheet(*m_loaded_style_sheet);
+ document().style_sheets().create_a_css_style_sheet(
+ "text/css"_string,
+ this,
+ attribute(HTML::AttributeNames::media).value_or({}),
+ in_a_document_tree() ? attribute(HTML::AttributeNames::title).value_or({}) : String {},
+ false,
+ true,
+ {},
+ nullptr,
+ nullptr,
+ *m_loaded_style_sheet);
  } else {
  dbgln_if(CSS_LOADER_DEBUG, "HTMLLinkElement: Failed to parse stylesheet: {}", resource()->url());
  }