Let embedder handle "maybe clone option into selectedcontent" algorithm (#719)

* Let embedder handle "maybe clone option into selectedcontent" algorithm

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Prepare a new release

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

Author: simonwuelker

Cargo.toml

@@ -10,7 +10,7 @@ members = [
 ]
 
 [workspace.package]
-version = "0.37.1"
+version = "0.38.0"
 license = "MIT OR Apache-2.0"
 authors = [ "The html5ever Project Developers" ]
 repository = "https://github.com/servo/html5ever"
@@ -21,9 +21,9 @@ rust-version = "1.71.0"
 # Repo dependencies
 tendril = { version = "0.5", path = "tendril" }
 web_atoms = { version = "0.2.1", path = "web_atoms" }
-markup5ever = { version = "0.37.1", path = "markup5ever" }
-xml5ever = { version = "0.37.1", path = "xml5ever" }
-html5ever = { version = "0.37.1", path = "html5ever" }
+markup5ever = { version = "0.38", path = "markup5ever" }
+xml5ever = { version = "0.38", path = "xml5ever" }
+html5ever = { version = "0.38", path = "html5ever" }
 
 # External dependencies
 encoding_rs = "0.8.12"

html5ever/examples/arena.rs

@@ -339,21 +339,6 @@ impl<'arena> TreeSink for Sink<'arena> {
             new_parent.append(child)
         }
     }
-
-    fn clone_subtree(&self, node: &Self::Handle) -> Self::Handle {
-        // Allocate the new node in the arena using Clone
-        let cloned_node = self.arena.alloc(Node::new(node.data.clone()));
-
-        // Clone all children and append them
-        let mut child = node.first_child.get();
-        while let Some(current_child) = child {
-            let cloned_child = self.clone_subtree(&current_child);
-            cloned_node.append(cloned_child);
-            child = current_child.next_sibling.get();
-        }
-
-        cloned_node
-    }
 }
 
 /// In this example an "arena" is created and filled with the DOM nodes.

html5ever/examples/noop-tree-builder.rs

@@ -113,11 +113,6 @@ impl TreeSink for Sink {
     fn remove_from_parent(&self, _target: &usize) {}
     fn reparent_children(&self, _node: &usize, _new_parent: &usize) {}
     fn mark_script_already_started(&self, _node: &usize) {}
-
-    fn clone_subtree(&self, _node: &Self::Handle) -> Self::Handle {
-        // For this noop example, just return a new placeholder ID
-        self.get_id()
-    }
 }
 
 /// In this example we implement the TreeSink trait which takes each parsed elements and insert

html5ever/examples/print-tree-actions.rs

@@ -167,12 +167,6 @@ impl TreeSink for Sink {
     fn pop(&self, elem: &usize) {
         println!("Popped element {elem}");
     }
-
-    fn clone_subtree(&self, node: &Self::Handle) -> Self::Handle {
-        println!("Clone subtree for node {node}");
-        // For this example, just return a new placeholder ID
-        self.get_id()
-    }
 }
 
 /// Same example as the "noop-tree-builder", but this time every function implemented in our

html5ever/src/tree_builder/mod.rs

@@ -119,9 +119,6 @@ pub struct TreeBuilder<Handle, Sink> {
     /// Form element pointer.
     form_elem: RefCell<Option<Handle>>,
 
-    /// selectedcontent element pointer.
-    selectedcontent_elem: RefCell<Option<Handle>>,
-    //§ END
     /// Frameset-ok flag.
     frameset_ok: Cell<bool>,
 
@@ -166,7 +163,6 @@ where
             active_formatting: Default::default(),
             head_elem: Default::default(),
             form_elem: Default::default(),
-            selectedcontent_elem: Default::default(),
             frameset_ok: Cell::new(true),
             ignore_lf: Default::default(),
             foster_parenting: Default::default(),
@@ -207,7 +203,6 @@ where
             active_formatting: Default::default(),
             head_elem: Default::default(),
             form_elem: RefCell::new(form_elem),
-            selectedcontent_elem: Default::default(),
             frameset_ok: Cell::new(true),
             ignore_lf: Default::default(),
             foster_parenting: Default::default(),
@@ -290,10 +285,6 @@ where
             tracer.trace_handle(form_elem);
         }
 
-        if let Some(selectedcontent_elem) = self.selectedcontent_elem.borrow().as_ref() {
-            tracer.trace_handle(selectedcontent_elem);
-        }
-
         if let Some(context_elem) = self.context_elem.borrow().as_ref() {
             tracer.trace_handle(context_elem);
         }
@@ -1360,7 +1351,7 @@ where
         // FIXME: application cache selection algorithm
     }
 
-    // https://html.spec.whatwg.org/multipage/#create-an-element-for-the-token
+    /// <https://html.spec.whatwg.org/multipage/#create-an-element-for-the-token>
     fn insert_element(
         &self,
         push: PushFlag,
@@ -1405,12 +1396,6 @@ where
 
         self.insert_at(insertion_point, AppendNode(elem.clone()));
 
-        if qname.local == local_name!("selectedcontent")
-            && self.selectedcontent_elem.borrow().is_none()
-        {
-            *self.selectedcontent_elem.borrow_mut() = Some(elem.clone());
-        }
-
         match push {
             PushFlag::Push => self.push(&elem),
             PushFlag::NoPush => (),
@@ -1595,19 +1580,6 @@ where
         self.remove_from_stack(&node);
     }
 
-    fn maybe_clone_option_into_selectedcontent(&self, option: &Handle) {
-        if let Some(selectedcontent) = self.selectedcontent_elem.borrow().as_ref().cloned() {
-            self.clone_option_into_selectedcontent(option, &selectedcontent);
-        }
-    }
-
-    fn clone_option_into_selectedcontent(&self, option: &Handle, selectedcontent: &Handle) {
-        self.sink
-            .reparent_children(selectedcontent, &self.sink.get_document());
-        let cloned_option = self.sink.clone_subtree(option);
-        self.sink.reparent_children(&cloned_option, selectedcontent);
-    }
-
     //§ tree-construction
     fn is_foreign(&self, token: &Token) -> bool {
         if let Token::Eof = *token {

html5ever/src/tree_builder/rules.rs

@@ -662,7 +662,8 @@ where
                     }
                     ProcessResult::Done
                 },
-
+                // FIXME: This branch does not exist like this in the specification, because it should run for
+                // implicitly closed option tags too. See https://github.com/servo/html5ever/issues/712.
                 Token::Tag(tag @ tag!(</option>)) => {
                     let option_in_stack = self
                         .open_elems
@@ -680,7 +681,8 @@ where
                             .iter()
                             .any(|elem| self.sink.same_node(elem, &option))
                         {
-                            self.maybe_clone_option_into_selectedcontent(&option);
+                            self.sink
+                                .maybe_clone_an_option_into_selectedcontent(&option);
                         }
                     }
 

html5ever/tests/driver.rs

@@ -99,11 +99,6 @@ impl TreeSink for Sink {
     fn remove_from_parent(&self, _target: &usize) {}
     fn reparent_children(&self, _node: &usize, _new_parent: &usize) {}
     fn mark_script_already_started(&self, _node: &usize) {}
-
-    fn clone_subtree(&self, _node: &Self::Handle) -> Self::Handle {
-        // For this noop example, just return a new placeholder ID
-        self.get_id()
-    }
 }
 
 #[test]

markup5ever/interface/tree_builder.rs

@@ -236,9 +236,6 @@ pub trait TreeSink {
     /// Remove all the children from node and append them to new_parent.
     fn reparent_children(&self, node: &Self::Handle, new_parent: &Self::Handle);
 
-    /// Clone a node and all its descendants, returning the cloned node.
-    fn clone_subtree(&self, node: &Self::Handle) -> Self::Handle;
-
     /// Returns true if the adjusted current node is an HTML integration point
     /// and the token is a start tag.
     fn is_mathml_annotation_xml_integration_point(&self, _handle: &Self::Handle) -> bool {
@@ -263,6 +260,17 @@ pub trait TreeSink {
     ) -> bool {
         false
     }
+
+    /// Implements [`maybe clone an option into selectedcontent`](https://html.spec.whatwg.org/#maybe-clone-an-option-into-selectedcontent).
+    ///
+    /// The provided handle is guaranteed to be an `<option>` element.
+    ///
+    /// Leaving this method unimplemented will not cause panics, but will result in a (slightly) incorrect DOM tree.
+    ///
+    /// This method will never be called from `xml5ever`.
+    fn maybe_clone_an_option_into_selectedcontent(&self, option: &Self::Handle) {
+        _ = option;
+    }
 }
 
 /// Trace hooks for a garbage-collected DOM.