Selaa lähdekoodia

LibWeb: Let style elements remember which StyleSheetList they live in

Instead of trying to locate the relevant StyleSheetList on style element
removal from the DOM, we now simply keep a pointer to the list instead.

This fixes an issue where using attachShadow() on an element that had
a declarative shadow DOM would cause any style elements present to use
the wrong StyleSheetList when removing themselves from the tree.
Andreas Kling 10 kuukautta sitten
vanhempi
commit
8543b8ad6a

+ 1 - 0
Tests/LibWeb/Text/expected/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.txt

@@ -0,0 +1 @@
+   PASS (didn't crash)

+ 9 - 0
Tests/LibWeb/Text/input/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.html

@@ -0,0 +1,9 @@
+<div id="hmmst"><template shadowrootmode="open"><div><style></style></div></template></div>
+<script src="../include.js"></script>
+<script>
+    test(() => {
+        hmmst.attachShadow({ mode: "open" });
+        println("PASS (didn't crash)");
+        hmmst.remove();
+    });
+</script>

+ 11 - 18
Userland/Libraries/LibWeb/DOM/StyleElementUtils.cpp

@@ -13,15 +13,6 @@
 
 
 namespace Web::DOM {
 namespace Web::DOM {
 
 
-static CSS::StyleSheetList& relevant_style_sheet_list_for_node(DOM::Node& node)
-{
-    auto& root_node = node.root();
-    if (is<DOM::ShadowRoot>(root_node))
-        return static_cast<DOM::ShadowRoot&>(root_node).style_sheets();
-
-    return node.document().style_sheets();
-}
-
 // The user agent must run the "update a style block" algorithm whenever one of the following conditions occur:
 // The user agent must run the "update a style block" algorithm whenever one of the following conditions occur:
 // FIXME: The element is popped off the stack of open elements of an HTML parser or XML parser.
 // FIXME: The element is popped off the stack of open elements of an HTML parser or XML parser.
 //
 //
@@ -32,7 +23,7 @@ static CSS::StyleSheetList& relevant_style_sheet_list_for_node(DOM::Node& node)
 // The element is not on the stack of open elements of an HTML parser or XML parser, and it becomes connected or disconnected.
 // The element is not on the stack of open elements of an HTML parser or XML parser, and it becomes connected or disconnected.
 //
 //
 // https://html.spec.whatwg.org/multipage/semantics.html#update-a-style-block
 // https://html.spec.whatwg.org/multipage/semantics.html#update-a-style-block
-void StyleElementUtils::update_a_style_block(DOM::Element& style_element, JS::GCPtr<DOM::Node> old_parent_if_removed_from)
+void StyleElementUtils::update_a_style_block(DOM::Element& style_element)
 {
 {
     // OPTIMIZATION: Skip parsing CSS if we're in the middle of parsing a HTML fragment.
     // OPTIMIZATION: Skip parsing CSS if we're in the middle of parsing a HTML fragment.
     //               The style block will be parsed upon insertion into a proper document.
     //               The style block will be parsed upon insertion into a proper document.
@@ -43,13 +34,8 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element, JS::GC
     // 2. If element has an associated CSS style sheet, remove the CSS style sheet in question.
     // 2. If element has an associated CSS style sheet, remove the CSS style sheet in question.
 
 
     if (m_associated_css_style_sheet) {
     if (m_associated_css_style_sheet) {
-        // NOTE: If we're here in response to a node being removed from the tree, we need to remove the stylesheet from the style scope
-        //       of the old parent, not the style scope of the node itself, since it's too late to find it that way!
-        if (old_parent_if_removed_from) {
-            relevant_style_sheet_list_for_node(*old_parent_if_removed_from).remove_a_css_style_sheet(*m_associated_css_style_sheet);
-        } else {
-            style_element.document_or_shadow_root_style_sheets().remove_a_css_style_sheet(*m_associated_css_style_sheet);
-        }
+        m_style_sheet_list->remove_a_css_style_sheet(*m_associated_css_style_sheet);
+        m_style_sheet_list = nullptr;
 
 
         // FIXME: This should probably be handled by StyleSheet::set_owner_node().
         // FIXME: This should probably be handled by StyleSheet::set_owner_node().
         m_associated_css_style_sheet = nullptr;
         m_associated_css_style_sheet = nullptr;
@@ -76,7 +62,8 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element, JS::GC
     m_associated_css_style_sheet = sheet;
     m_associated_css_style_sheet = sheet;
 
 
     // 6. Create a CSS style sheet with the following properties...
     // 6. Create a CSS style sheet with the following properties...
-    style_element.document_or_shadow_root_style_sheets().create_a_css_style_sheet(
+    m_style_sheet_list = style_element.document_or_shadow_root_style_sheets();
+    m_style_sheet_list->create_a_css_style_sheet(
         "text/css"_string,
         "text/css"_string,
         &style_element,
         &style_element,
         style_element.attribute(HTML::AttributeNames::media).value_or({}),
         style_element.attribute(HTML::AttributeNames::media).value_or({}),
@@ -91,4 +78,10 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element, JS::GC
         *sheet);
         *sheet);
 }
 }
 
 
+void StyleElementUtils::visit_edges(JS::Cell::Visitor& visitor)
+{
+    visitor.visit(m_associated_css_style_sheet);
+    visitor.visit(m_style_sheet_list);
+}
+
 }
 }

+ 7 - 5
Userland/Libraries/LibWeb/DOM/StyleElementUtils.h

@@ -14,19 +14,21 @@ namespace Web::DOM {
 
 
 class StyleElementUtils {
 class StyleElementUtils {
 public:
 public:
-    void update_a_style_block(DOM::Element& style_element, JS::GCPtr<DOM::Node> old_parent_if_removed_from = nullptr);
+    void update_a_style_block(DOM::Element& style_element);
 
 
     CSS::CSSStyleSheet* sheet() { return m_associated_css_style_sheet; }
     CSS::CSSStyleSheet* sheet() { return m_associated_css_style_sheet; }
     CSS::CSSStyleSheet const* sheet() const { return m_associated_css_style_sheet; }
     CSS::CSSStyleSheet const* sheet() const { return m_associated_css_style_sheet; }
 
 
-    void visit_edges(JS::Cell::Visitor& visitor)
-    {
-        visitor.visit(m_associated_css_style_sheet);
-    }
+    [[nodiscard]] JS::GCPtr<CSS::StyleSheetList> style_sheet_list() { return m_style_sheet_list; }
+    [[nodiscard]] JS::GCPtr<CSS::StyleSheetList const> style_sheet_list() const { return m_style_sheet_list; }
+
+    void visit_edges(JS::Cell::Visitor&);
 
 
 private:
 private:
     // https://www.w3.org/TR/cssom/#associated-css-style-sheet
     // https://www.w3.org/TR/cssom/#associated-css-style-sheet
     JS::GCPtr<CSS::CSSStyleSheet> m_associated_css_style_sheet;
     JS::GCPtr<CSS::CSSStyleSheet> m_associated_css_style_sheet;
+
+    JS::GCPtr<CSS::StyleSheetList> m_style_sheet_list;
 };
 };
 
 
 }
 }

+ 1 - 1
Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp

@@ -46,7 +46,7 @@ void HTMLStyleElement::inserted()
 
 
 void HTMLStyleElement::removed_from(Node* old_parent)
 void HTMLStyleElement::removed_from(Node* old_parent)
 {
 {
-    m_style_element_utils.update_a_style_block(*this, old_parent);
+    m_style_element_utils.update_a_style_block(*this);
     Base::removed_from(old_parent);
     Base::removed_from(old_parent);
 }
 }
 
 

+ 1 - 1
Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp

@@ -44,7 +44,7 @@ void SVGStyleElement::inserted()
 
 
 void SVGStyleElement::removed_from(Node* old_parent)
 void SVGStyleElement::removed_from(Node* old_parent)
 {
 {
-    m_style_element_utils.update_a_style_block(*this, old_parent);
+    m_style_element_utils.update_a_style_block(*this);
     Base::removed_from(old_parent);
     Base::removed_from(old_parent);
 }
 }