From 3e4a1cbd55f55ace873be7c8a0b1c0814b21e5dc Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Wed, 30 Jul 2025 21:28:02 +0200 Subject: [PATCH] LibWeb: Use ConservativeVector for recorded node values in Editing API The `RecordedNodeValue` struct contains a `GC::Ref` to a DOM node, which might disappear as a result of a garbage collection. For example, during the "outdent" command, we record nodes, split the parent of those nodes potentially resulting in all kinds of DOM changes, and then try to restore the nodes' values. This caused a crash in the `editing/run/outdent.html` WPT subtests. By returning a `ConservativeVector`, we make sure the `GC::Ref` gets marked during sweeps and nodes do not suddenly disappear. --- Libraries/LibWeb/Editing/Commands.cpp | 8 +++---- .../LibWeb/Editing/Internal/Algorithms.cpp | 22 +++++++++---------- .../LibWeb/Editing/Internal/Algorithms.h | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Libraries/LibWeb/Editing/Commands.cpp b/Libraries/LibWeb/Editing/Commands.cpp index 3212fdbf541..2c521022452 100644 --- a/Libraries/LibWeb/Editing/Commands.cpp +++ b/Libraries/LibWeb/Editing/Commands.cpp @@ -240,7 +240,7 @@ bool command_delete_action(DOM::Document& document, Utf16String const&) // 3. Record the values of the one-node list consisting of node, and let values be the // result. - auto values = record_the_values_of_nodes({ *node }); + auto values = record_the_values_of_nodes(document.heap(), { *node }); // 4. Split the parent of the one-node list consisting of node. split_the_parent_of_nodes({ *node }); @@ -666,7 +666,7 @@ bool command_format_block_action(DOM::Document& document, Utf16String const& val }); // 7. Record the values of node list, and let values be the result. - auto values = record_the_values_of_nodes(node_list); + auto values = record_the_values_of_nodes(document.heap(), node_list); // 8. For each node in node list, while node is the descendant of an editable HTML element in the same editing host, // whose local name is a formattable block name, and which is not the ancestor of a prohibited paragraph child, @@ -714,7 +714,7 @@ bool command_format_block_action(DOM::Document& document, Utf16String const& val }); // 2. Record the values of sublist, and let values be the result. - auto values = record_the_values_of_nodes(sublist); + auto values = record_the_values_of_nodes(document.heap(), sublist); // 3. Remove the first member of node list from its parent, preserving its descendants. remove_node_preserving_its_descendants(node_list.first()); @@ -2141,7 +2141,7 @@ bool command_outdent_action(DOM::Document& document, Utf16String const&) sublist.append(node_list.take_first()); // 6. Record the values of sublist, and let values be the result. - auto values = record_the_values_of_nodes(sublist); + auto values = record_the_values_of_nodes(document.heap(), sublist); // 7. Split the parent of sublist. split_the_parent_of_nodes(sublist); diff --git a/Libraries/LibWeb/Editing/Internal/Algorithms.cpp b/Libraries/LibWeb/Editing/Internal/Algorithms.cpp index ef77b584702..1392e21f9a3 100644 --- a/Libraries/LibWeb/Editing/Internal/Algorithms.cpp +++ b/Libraries/LibWeb/Editing/Internal/Algorithms.cpp @@ -961,7 +961,7 @@ void delete_the_selection(Selection& selection, bool block_merging, bool strip_w } // 9. Record the values of children, and let values be the result. - values = record_the_values_of_nodes(children); + values = record_the_values_of_nodes(document.heap(), children); // 10. While children's first member's parent is not start block, split the parent of children. while (children.first()->parent() != start_block) @@ -1005,7 +1005,7 @@ void delete_the_selection(Selection& selection, bool block_merging, bool strip_w nodes_to_move.append(*nodes_to_move.last()->next_sibling()); // 8. Record the values of nodes to move, and let values be the result. - values = record_the_values_of_nodes(nodes_to_move); + values = record_the_values_of_nodes(document.heap(), nodes_to_move); // 9. For each node in nodes to move, append node as the last child of start block, preserving ranges. auto new_position = start_block->length(); @@ -1032,7 +1032,7 @@ void delete_the_selection(Selection& selection, bool block_merging, bool strip_w end_block_children.append(child); return IterationDecision::Continue; }); - values = record_the_values_of_nodes(end_block_children); + values = record_the_values_of_nodes(document.heap(), end_block_children); // 4. While end block has children, append the first child of end block to start block, preserving ranges. auto new_position = start_block->length(); @@ -1328,7 +1328,7 @@ void fix_disallowed_ancestors_of_node(GC::Ref node) return IterationDecision::Continue; // 1. Record the values of the one-node list consisting of child, and let values be the result. - auto values = record_the_values_of_nodes({ child }); + auto values = record_the_values_of_nodes(child.heap(), { child }); // 2. Split the parent of the one-node list consisting of child. split_the_parent_of_nodes({ child }); @@ -1344,7 +1344,7 @@ void fix_disallowed_ancestors_of_node(GC::Ref node) } // 3. Record the values of the one-node list consisting of node, and let values be the result. - auto values = record_the_values_of_nodes({ node }); + auto values = record_the_values_of_nodes(node->heap(), { node }); // 4. While node is not an allowed child of its parent, split the parent of the one-node list consisting of node. while (!is_allowed_child_of_node(node, GC::Ref { *node->parent() })) @@ -3026,7 +3026,7 @@ void outdent(GC::Ref node) // 4. Otherwise: else { // 1. Record the values of node's children, and let values be the result. - auto values = record_the_values_of_nodes(children); + auto values = record_the_values_of_nodes(node->heap(), children); // 2. Remove node, preserving its descendants. remove_node_preserving_its_descendants(node); @@ -3314,10 +3314,10 @@ Vector record_current_states_and_values(DOM::Document const& d } // https://w3c.github.io/editing/docs/execCommand/#record-the-values -Vector record_the_values_of_nodes(Vector> const& node_list) +GC::ConservativeVector record_the_values_of_nodes(GC::Heap& heap, Vector> const& node_list) { // 1. Let values be a list of (node, command, specified command value) triples, initially empty. - Vector values; + GC::ConservativeVector values { heap }; // 2. For each node in node list, for each command in the list "subscript", "bold", "fontName", // "fontSize", "foreColor", "hiliteColor", "italic", "strikethrough", and "underline" in that @@ -4126,7 +4126,7 @@ void toggle_lists(DOM::Document& document, FlyString const& tag_name) }); // 2. Record the values of children, and let values be the result. - auto values = record_the_values_of_nodes(children); + auto values = record_the_values_of_nodes(document.heap(), children); // 3. Split the parent of children. split_the_parent_of_nodes(children); @@ -4201,7 +4201,7 @@ void toggle_lists(DOM::Document& document, FlyString const& tag_name) sublist.append(node_list.take_first()); // 5. Record the values of sublist, and let values be the result. - auto values = record_the_values_of_nodes(sublist); + auto values = record_the_values_of_nodes(document.heap(), sublist); // 6. Split the parent of sublist. split_the_parent_of_nodes(sublist); @@ -4282,7 +4282,7 @@ void toggle_lists(DOM::Document& document, FlyString const& tag_name) if (!sublist.is_empty() && is(sublist.first()->parent()) && static_cast(*sublist.first()->parent()).local_name() == other_tag_name) { // 1. Record the values of sublist, and let values be the result. - auto values = record_the_values_of_nodes(sublist); + auto values = record_the_values_of_nodes(document.heap(), sublist); // 2. Split the parent of sublist. split_the_parent_of_nodes(sublist); diff --git a/Libraries/LibWeb/Editing/Internal/Algorithms.h b/Libraries/LibWeb/Editing/Internal/Algorithms.h index c88a9d22900..ab69fccdd29 100644 --- a/Libraries/LibWeb/Editing/Internal/Algorithms.h +++ b/Libraries/LibWeb/Editing/Internal/Algorithms.h @@ -106,7 +106,7 @@ Optional previous_equivalent_point(DOM::BoundaryPoint); void push_down_values(FlyString const&, GC::Ref, Optional); Vector record_current_overrides(DOM::Document const&); Vector record_current_states_and_values(DOM::Document const&); -Vector record_the_values_of_nodes(Vector> const&); +GC::ConservativeVector record_the_values_of_nodes(GC::Heap&, Vector> const&); void remove_extraneous_line_breaks_at_the_end_of_node(GC::Ref); void remove_extraneous_line_breaks_before_node(GC::Ref); void remove_extraneous_line_breaks_from_a_node(GC::Ref);