From cd84c5ccbbde351b9b7af800ebe2ef05d7117316 Mon Sep 17 00:00:00 2001
From: Scott Taylor <scott.c.taylor@mac.com>
Date: Fri, 9 Oct 2026 13:01:47 -0400
Subject: [PATCH] Clone implicitly closed options into selectedcontent

The HTML spec runs "maybe clone an option into selectedcontent" whenever
an option element is popped off the stack of open elements; whatwg/html
4ce63af1 moved it there from a dedicated "</option>" step. html5ever only
ran it from a special case for "</option>", so options closed implicitly,
by another <option>, an <optgroup> or <hr>, a </select> end tag or the end
of the input, were never cloned.

Separately, several paths dropped elements from the stack of open
elements without calling TreeSink::pop: the truncations in the adoption
agency algorithm, in "any other end tag", for <frameset> and for end tags
in foreign content; the adoption agency's removal of nodes between the
formatting element and the furthest block; and pop_until and
pop_until_current, which most end tags go through. Sinks that track pops
missed all of those elements.

Route every removal from the stack of open elements through one helper,
which tells the sink about the element and then, for HTML option
elements, calls TreeSink::maybe_clone_an_option_into_selectedcontent.
The "</option>" special case goes away, and "</option>" is handled by
"any other end tag", as in the spec.

rcdom's implementation of the hook never found the selectedcontent
element, because it looked at the select's name instead of each
descendant's, so it never cloned anything. Fix that, and make the clone
safe to run: replace the selectedcontent's children without leaving them
with stale parent pointers, clone without recursing, and when dropping a
node leave alone any child that is still referenced elsewhere, since the
clone can remove elements that are still on the stack of open elements
from the tree.

Add tree construction tests for options closed in each of these ways, in
a new directory of custom html5lib-style tests, and tests for the pops
and selectedcontent calls the tree builder makes to its sink.
webkit02.dat-44 and -45 stay skipped: they also depend on the first
option being selected by default, which rcdom does not model.

Fixes #712
---
 html5ever/src/tree_builder/mod.rs             |  67 +++--
 html5ever/src/tree_builder/rules.rs           |  30 +--
 html5ever/tests/tree_builder.rs               | 169 ++++++++++++
 markup5ever/interface/tree_builder.rs         |   4 +
 .../selectedcontent.dat                       | 255 ++++++++++++++++++
 rcdom/lib.rs                                  |  48 +++-
 rcdom/tests/html-tree-builder.rs              |  39 +--
 7 files changed, 534 insertions(+), 78 deletions(-)
 create mode 100644 html5ever/tests/tree_builder.rs
 create mode 100644 rcdom/custom-html5lib-tree-construction-tests/selectedcontent.dat

diff --git a/html5ever/src/tree_builder/mod.rs b/html5ever/src/tree_builder/mod.rs
index 803a398e..1f5b8d35 100644
--- a/html5ever/src/tree_builder/mod.rs
+++ b/html5ever/src/tree_builder/mod.rs
@@ -544,9 +544,9 @@ where
     }
 
     fn end(&self) {
-        for elem in self.open_elems.borrow_mut().drain(..).rev() {
-            self.sink.pop(&elem);
-        }
+        // <https://html.spec.whatwg.org/#stop-parsing>
+        // Step 4. Pop all the nodes off the stack of open elements.
+        self.pop_to_len(0);
     }
 
     fn adjusted_current_node_present_but_not_in_html_namespace(&self) -> bool {
@@ -778,7 +778,7 @@ where
 
             let Some((furthest_block_index, furthest_block)) = maybe_furthest_block else {
                 // 10.
-                self.open_elems.borrow_mut().truncate(fmt_elem_stack_index);
+                self.pop_to_len(fmt_elem_stack_index);
                 self.active_formatting.borrow_mut().remove(fmt_elem_index);
                 return;
             };
@@ -813,13 +813,13 @@ where
                 if inner_counter > 3 {
                     self.position_in_active_formatting(&node)
                         .map(|position| self.active_formatting.borrow_mut().remove(position));
-                    self.open_elems.borrow_mut().remove(node_index);
+                    self.remove_from_stack_at(node_index);
                     continue;
                 }
 
                 let Some(node_formatting_index) = self.position_in_active_formatting(&node) else {
                     // 13.6.
-                    self.open_elems.borrow_mut().remove(node_index);
+                    self.remove_from_stack_at(node_index);
                     continue;
                 };
 
@@ -839,6 +839,7 @@ where
                     tag.attrs.clone(),
                     tag.had_duplicate_attributes,
                 );
+                // Replacing the entry for node in the stack of open elements does not pop node.
                 self.open_elems.borrow_mut()[node_index] = new_element.clone();
                 self.active_formatting.borrow_mut()[node_formatting_index] =
                     FormatEntry::Element(new_element.clone(), tag);
@@ -924,15 +925,22 @@ where
         self.open_elems.borrow_mut().push(elem.clone());
     }
 
+    /// Pop the current node off the stack of open elements.
     fn pop(&self) -> Handle {
-        let elem = self
+        let current_node_index = self
             .open_elems
-            .borrow_mut()
-            .pop()
+            .borrow()
+            .len()
+            .checked_sub(1)
             .expect("no current element");
+        self.remove_from_stack_at(current_node_index)
+    }
 
-        self.sink.pop(&elem);
-        elem
+    /// Pop elements off the stack of open elements until only `len` of them are left.
+    fn pop_to_len(&self, len: usize) {
+        while self.open_elems.borrow().len() > len {
+            self.pop();
+        }
     }
 
     fn remove_from_stack(&self, elem: &Handle) {
@@ -942,11 +950,28 @@ where
             .iter()
             .rposition(|x| self.sink.same_node(elem, x));
         if let Some(position) = position {
-            self.open_elems.borrow_mut().remove(position);
-            self.sink.pop(elem);
+            self.remove_from_stack_at(position);
         }
     }
 
+    /// Remove the element at `index` from the stack of open elements.
+    ///
+    /// Every element leaves the stack of open elements through here, whether it is the current
+    /// node or not, so that the sink is always told about it and the steps that run when an
+    /// element is popped are never skipped.
+    fn remove_from_stack_at(&self, index: usize) -> Handle {
+        let elem = self.open_elems.borrow_mut().remove(index);
+        self.sink.pop(&elem);
+
+        // Popping an option element, however it was closed, may update the selectedcontent of
+        // its select: <https://html.spec.whatwg.org/#the-option-element:stack-of-open-elements>
+        if self.html_elem_named(&elem, local_name!("option")) {
+            self.sink.maybe_clone_an_option_into_selectedcontent(&elem);
+        }
+
+        elem
+    }
+
     fn is_marker_or_open(&self, entry: &FormatEntry<Handle>) -> bool {
         match *entry {
             FormatEntry::Marker => true,
@@ -1167,7 +1192,7 @@ where
         TagSet: Fn(ExpandedName) -> bool,
     {
         while !self.current_node_in(&tag_set) {
-            self.open_elems.borrow_mut().pop();
+            self.pop();
         }
     }
 
@@ -1178,15 +1203,11 @@ where
         P: Fn(ExpandedName) -> bool,
     {
         let mut n = 0;
-        loop {
+        while !self.open_elems.borrow().is_empty() {
             n += 1;
-            match self.open_elems.borrow_mut().pop() {
-                None => break,
-                Some(elem) => {
-                    if pred(self.sink.elem_name(&elem).expanded()) {
-                        break;
-                    }
-                },
+            let elem = self.pop();
+            if pred(self.sink.elem_name(&elem).expanded()) {
+                break;
             }
         }
         n
@@ -1581,7 +1602,7 @@ where
             // mis-nested tags
             self.unexpected(&tag);
         }
-        self.open_elems.borrow_mut().truncate(match_idx);
+        self.pop_to_len(match_idx);
     }
 
     fn handle_misnested_a_tags(&self, tag: &Tag) {
diff --git a/html5ever/src/tree_builder/rules.rs b/html5ever/src/tree_builder/rules.rs
index ed5291cf..4a0c6db1 100644
--- a/html5ever/src/tree_builder/rules.rs
+++ b/html5ever/src/tree_builder/rules.rs
@@ -468,7 +468,7 @@ where
 
                     // FIXME: can we get here in the fragment case?
                     // What to do with the first element then?
-                    self.open_elems.borrow_mut().truncate(1);
+                    self.pop_to_len(1);
                     self.insert_element_for(tag);
                     self.mode.set(InsertionMode::InFrameset);
                     ProcessResult::Done
@@ -663,32 +663,6 @@ 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
-                        .borrow()
-                        .iter()
-                        .find(|elem| self.html_elem_named(elem, local_name!("option")))
-                        .cloned();
-
-                    self.process_end_tag_in_body(tag);
-
-                    if let Some(option) = option_in_stack {
-                        if !self
-                            .open_elems
-                            .borrow()
-                            .iter()
-                            .any(|elem| self.sink.same_node(elem, &option))
-                        {
-                            self.sink
-                                .maybe_clone_an_option_into_selectedcontent(&option);
-                        }
-                    }
-
-                    ProcessResult::Done
-                },
 
                 Token::Tag(tag!(</p>)) => {
                     if !self.in_scope_named(button_scope, local_name!("p")) {
@@ -1671,7 +1645,7 @@ where
                     }
 
                     if eq {
-                        self.open_elems.borrow_mut().truncate(stack_idx);
+                        self.pop_to_len(stack_idx);
                         return ProcessResult::Done;
                     }
 
diff --git a/html5ever/tests/tree_builder.rs b/html5ever/tests/tree_builder.rs
new file mode 100644
index 00000000..aa187c7b
--- /dev/null
+++ b/html5ever/tests/tree_builder.rs
@@ -0,0 +1,169 @@
+use html5ever::interface::{ElementFlags, NodeOrText, QuirksMode, TreeSink};
+use html5ever::tendril::{StrTendril, TendrilSink};
+use html5ever::{
+    local_name, ns, parse_document, parse_fragment, Attribute, ExpandedName, QualName,
+};
+use std::borrow::Cow;
+use std::cell::RefCell;
+
+/// A sink that doesn't build a tree. It logs the elements that are popped off the stack of open
+/// elements, and the options that the tree builder asks it to clone into a selectedcontent.
+struct Sink {
+    /// The name of the element that each handle refers to, or `None` for other nodes.
+    names: RefCell<Vec<Option<&'static QualName>>>,
+    log: RefCell<Vec<String>>,
+}
+
+impl Sink {
+    fn new() -> Self {
+        Sink {
+            // The document.
+            names: RefCell::new(vec![None]),
+            log: RefCell::default(),
+        }
+    }
+
+    fn new_node(&self, name: Option<QualName>) -> usize {
+        let mut names = self.names.borrow_mut();
+        // Leak the name so that `elem_name` can return a reference to it.
+        names.push(name.map(|name| &*Box::leak(Box::new(name))));
+        names.len() - 1
+    }
+
+    fn log(&self, event: &str, element: &usize) {
+        let local_name = self.elem_name(element).local.to_string();
+        self.log.borrow_mut().push(format!("{event} {local_name}"));
+    }
+}
+
+impl TreeSink for Sink {
+    type Handle = usize;
+    type Output = String;
+    type ElemName<'a> = ExpandedName<'a>;
+
+    fn finish(self) -> String {
+        self.log.into_inner().join(", ")
+    }
+
+    fn get_document(&self) -> usize {
+        0
+    }
+
+    fn elem_name<'a>(&'a self, target: &'a usize) -> ExpandedName<'a> {
+        self.names.borrow()[*target]
+            .expect("not an element")
+            .expanded()
+    }
+
+    fn create_element(&self, name: QualName, _: Vec<Attribute>, flags: ElementFlags) -> usize {
+        let element = self.new_node(Some(name));
+        if flags.template {
+            // The template contents get the next handle.
+            self.new_node(None);
+        }
+        element
+    }
+
+    fn create_comment(&self, _text: StrTendril) -> usize {
+        self.new_node(None)
+    }
+
+    fn create_pi(&self, _target: StrTendril, _data: StrTendril) -> usize {
+        self.new_node(None)
+    }
+
+    fn get_template_contents(&self, target: &usize) -> usize {
+        target + 1
+    }
+
+    fn same_node(&self, x: &usize, y: &usize) -> bool {
+        x == y
+    }
+
+    fn pop(&self, node: &usize) {
+        self.log("pop", node);
+    }
+
+    fn maybe_clone_an_option_into_selectedcontent(&self, option: &usize) {
+        self.log("maybe clone", option);
+    }
+
+    fn parse_error(&self, _msg: Cow<'static, str>) {}
+    fn set_quirks_mode(&self, _mode: QuirksMode) {}
+    fn append(&self, _parent: &usize, _child: NodeOrText<usize>) {}
+    fn append_before_sibling(&self, _sibling: &usize, _new_node: NodeOrText<usize>) {}
+    fn append_based_on_parent_node(&self, _: &usize, _: &usize, _: NodeOrText<usize>) {}
+    fn append_doctype_to_document(&self, _: StrTendril, _: StrTendril, _: StrTendril) {}
+    fn add_attrs_if_missing(&self, _target: &usize, _attrs: Vec<Attribute>) {}
+    fn remove_from_parent(&self, _target: &usize) {}
+    fn reparent_children(&self, _node: &usize, _new_parent: &usize) {}
+}
+
+/// Parse `input` as the contents of a `<body>` element, and return what the sink logged.
+fn parse_body_fragment(input: &str) -> String {
+    let context = QualName::new(None, ns!(html), local_name!("body"));
+    parse_fragment(Sink::new(), Default::default(), context, vec![], false).one(input)
+}
+
+#[test]
+fn every_element_that_leaves_the_stack_of_open_elements_is_popped() {
+    // An end tag that pops elements until it pops its own element.
+    assert_eq!(
+        parse_body_fragment("<div><span></div>"),
+        "pop span, pop div, pop html"
+    );
+    // Clearing the stack back to a table context.
+    assert_eq!(
+        parse_body_fragment("<table><b><tr>"),
+        "pop b, pop tr, pop tbody, pop table, pop html"
+    );
+    // The adoption agency algorithm, without a furthest block.
+    assert_eq!(parse_body_fragment("<b><i></b>"), "pop i, pop b, pop html");
+    // The adoption agency algorithm, removing the span between the formatting element and the
+    // furthest block from the stack.
+    assert_eq!(
+        parse_body_fragment("<b><span><div></b>"),
+        "pop span, pop b, pop b, pop div, pop html"
+    );
+    // "Any other end tag" in the "in body" insertion mode.
+    assert_eq!(
+        parse_body_fragment("<span><em></span>"),
+        "pop em, pop span, pop html"
+    );
+    // An end tag in foreign content.
+    assert_eq!(
+        parse_body_fragment("<svg><g></g></svg>"),
+        "pop g, pop svg, pop html"
+    );
+    // A frameset start tag, which only does anything in a document.
+    assert_eq!(
+        parse_document(Sink::new(), Default::default()).one("<p><frameset>"),
+        "pop head, pop p, pop body, pop frameset, pop html"
+    );
+}
+
+/// <https://github.com/servo/html5ever/issues/712>
+#[test]
+fn maybe_clone_an_option_into_selectedcontent_runs_for_every_popped_option() {
+    assert_eq!(
+        parse_body_fragment("<select><option></option></select>"),
+        "pop option, maybe clone option, pop select, pop html"
+    );
+    // Options closed by an option, an optgroup, an hr and a select end tag, and at the end of the
+    // input.
+    assert_eq!(
+        parse_body_fragment(
+            "<select><option><option><optgroup><option><hr><option></select><option>"
+        ),
+        "pop option, maybe clone option, \
+         pop option, maybe clone option, \
+         pop option, maybe clone option, pop optgroup, \
+         pop option, maybe clone option, pop select, \
+         pop option, maybe clone option, pop html"
+    );
+    // An SVG element named "option" isn't an option element.
+    assert_eq!(
+        parse_body_fragment("<svg><option></option></svg>"),
+        "pop option, pop svg, pop html"
+    );
+}
diff --git a/markup5ever/interface/tree_builder.rs b/markup5ever/interface/tree_builder.rs
index e1683de0..684ad6ff 100644
--- a/markup5ever/interface/tree_builder.rs
+++ b/markup5ever/interface/tree_builder.rs
@@ -287,6 +287,10 @@ pub trait TreeSink {
 
     /// Implements [`maybe clone an option into selectedcontent`](https://html.spec.whatwg.org/#maybe-clone-an-option-into-selectedcontent).
     ///
+    /// The tree builder calls this, right after [`TreeSink::pop`], every time an `<option>` element
+    /// is popped off the stack of open elements, however it was closed: by an `</option>` end tag,
+    /// by another `<option>`, by `</select>`, at the end of the input, and so on.
+    ///
     /// 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.
diff --git a/rcdom/custom-html5lib-tree-construction-tests/selectedcontent.dat b/rcdom/custom-html5lib-tree-construction-tests/selectedcontent.dat
new file mode 100644
index 00000000..c90d5135
--- /dev/null
+++ b/rcdom/custom-html5lib-tree-construction-tests/selectedcontent.dat
@@ -0,0 +1,255 @@
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>X</option></select>
+#errors
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <option>
+|         selected=""
+|         "X"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>X<option>Y</select>
+#errors
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <option>
+|         selected=""
+|         "X"
+|       <option>
+|         "Y"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>X<optgroup><option>Y</select>
+#errors
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <option>
+|         selected=""
+|         "X"
+|       <optgroup>
+|         <option>
+|           "Y"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>X<hr></select>
+#errors
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <option>
+|         selected=""
+|         "X"
+|       <hr>
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>X</select>
+#errors
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <option>
+|         selected=""
+|         "X"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>X<span>Y</select>
+#errors
+1:101: ERROR: End tag 'select' isn't allowed here. Currently open tags: html, body, select, option, span.
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|           <span>
+|             "Y"
+|       <option>
+|         selected=""
+|         "X"
+|         <span>
+|           "Y"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>X
+#errors
+1:94: ERROR: Premature end of file. Currently open tags: html, body, select, option.
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <option>
+|         selected=""
+|         "X"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>x<i>i<b>ib</i>b
+#errors
+1:103: ERROR: End tag 'i' isn't allowed here. Currently open tags: html, body, select, option, i, b.
+1:108: ERROR: Premature end of file. Currently open tags: html, body, select, option, b.
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "x"
+|           <i>
+|             "i"
+|             <b>
+|               "ib"
+|           <b>
+|             "b"
+|       <option>
+|         selected=""
+|         "x"
+|         <i>
+|           "i"
+|           <b>
+|             "ib"
+|         <b>
+|           "b"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><b><option selected>X</b></select>
+#errors
+1:97: ERROR: End tag 'b' isn't allowed here. Currently open tags: html, body, select, b, option.
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <b>
+|         <option>
+|           selected=""
+|           "X"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><span><option selected>X<i>Y</span></select>
+#errors
+1:104: ERROR: End tag 'span' isn't allowed here. Currently open tags: html, body, select, span, option, i.
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|           <i>
+|             "Y"
+|       <span>
+|         <option>
+|           selected=""
+|           "X"
+|           <i>
+|             "Y"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>X<select>
+#errors
+1:94: ERROR: Start tag 'select' isn't allowed here. Currently open tags: html, body, select, option.
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <option>
+|         selected=""
+|         "X"
+
+#data
+<!DOCTYPE html><select><button><selectedcontent></selectedcontent></button><option selected>X<input>
+#errors
+1:94: ERROR: Start tag 'input' isn't allowed here. Currently open tags: html, body, select, option.
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <option>
+|         selected=""
+|         "X"
+|     <input>
+
+#data
+<!DOCTYPE html><table><tr><select><button><selectedcontent></selectedcontent></button><option selected>X<td></td></tr></table>
+#errors
+1:27: ERROR: Start tag 'select' isn't allowed here. Currently open tags: html, body, table, tbody, tr.
+1:35: ERROR: Start tag 'button' isn't allowed here. Currently open tags: html, body, table, tbody, tr, select.
+1:43: ERROR: Start tag 'selectedcontent' isn't allowed here. Currently open tags: html, body, table, tbody, tr, select, button.
+1:60: ERROR: End tag 'selectedcontent' isn't allowed here. Currently open tags: html, body, table, tbody, tr, select, button, selectedcontent.
+1:78: ERROR: End tag 'button' isn't allowed here. Currently open tags: html, body, table, tbody, tr, select, button.
+1:87: ERROR: Start tag 'option' isn't allowed here. Currently open tags: html, body, table, tbody, tr, select.
+1:104: ERROR: Character tokens aren't legal here
+#document
+| <!DOCTYPE html>
+| <html>
+|   <head>
+|   <body>
+|     <select>
+|       <button>
+|         <selectedcontent>
+|           "X"
+|       <option>
+|         selected=""
+|         "X"
+|     <table>
+|       <tbody>
+|         <tr>
+|           <td>
diff --git a/rcdom/lib.rs b/rcdom/lib.rs
index 1ee9306b..1eb9f352 100644
--- a/rcdom/lib.rs
+++ b/rcdom/lib.rs
@@ -202,7 +202,7 @@ impl Node {
         while let Some(node) = remaining.pop_front() {
             remaining.extend(node.children.borrow().iter().cloned());
 
-            let NodeData::Element { name, .. } = &self.data else {
+            let NodeData::Element { name, .. } = &node.data else {
                 continue;
             };
             if name.local_name() == &local_name!("selectedcontent") {
@@ -235,25 +235,39 @@ impl Node {
         }
 
         // Step 3. Replace all with documentFragment within selectedcontent.
-        *selectedcontent.children.borrow_mut() = document_fragment;
+        for child in selectedcontent.children.take() {
+            child.parent.set(None);
+        }
+        for child_clone in document_fragment {
+            append(&selectedcontent, child_clone);
+        }
     }
 
     /// Clones the node and all of its descendants, returning a handle to the new subtree.
-    ///
-    /// This function will run into infinite recursion when the DOM tree contains cycles and it makes
-    /// no attempts to guard against that.
     fn clone_with_subtree(&self) -> Rc<Self> {
-        let children = self
+        let clone = Node::new(self.data.clone());
+
+        // Like dropping, cloning doesn't recurse, so that deep trees can't overflow the stack.
+        let mut to_clone: Vec<(Handle, Handle)> = self
             .children
             .borrow()
             .iter()
-            .map(|child| child.clone_with_subtree())
+            .rev()
+            .map(|child| (child.clone(), clone.clone()))
             .collect();
-        Rc::new(Self {
-            parent: Cell::new(self.parent()),
-            data: self.data.clone(),
-            children: RefCell::new(children),
-        })
+        while let Some((node, parent_clone)) = to_clone.pop() {
+            let node_clone = Node::new(node.data.clone());
+            append(&parent_clone, node_clone.clone());
+            to_clone.extend(
+                node.children
+                    .borrow()
+                    .iter()
+                    .rev()
+                    .map(|child| (child.clone(), node_clone.clone())),
+            );
+        }
+
+        clone
     }
 }
 
@@ -261,6 +275,16 @@ impl Drop for Node {
     fn drop(&mut self) {
         let mut nodes = mem::take(&mut *self.children.borrow_mut());
         while let Some(node) = nodes.pop() {
+            // The parent of this node is going away.
+            node.parent.set(None);
+
+            // A node can outlive its parent, for example an element on the stack of open
+            // elements that a selectedcontent update removed from the tree. Leave such nodes
+            // intact.
+            if Rc::strong_count(&node) > 1 {
+                continue;
+            }
+
             let children = mem::take(&mut *node.children.borrow_mut());
             nodes.extend(children);
             if let NodeData::Element {
diff --git a/rcdom/tests/html-tree-builder.rs b/rcdom/tests/html-tree-builder.rs
index be611aa7..b8b58fc2 100644
--- a/rcdom/tests/html-tree-builder.rs
+++ b/rcdom/tests/html-tree-builder.rs
@@ -261,25 +261,34 @@ fn context_name(context: &str) -> QualName {
 fn tests(src_dir: &Path, ignores: &HashSet<String>) -> Vec<Test> {
     let mut tests = vec![];
 
+    let mut add_tests = |path: &Path, file: fs::File| {
+        let buf = io::BufReader::new(file);
+        let lines = buf.lines().map(|res| res.expect("couldn't read"));
+        let data = parse_tests(lines);
+
+        for (i, test) in data.into_iter().enumerate() {
+            make_test(
+                &mut tests,
+                ignores,
+                path.file_name().unwrap().to_str().unwrap(),
+                i,
+                test,
+            );
+        }
+    };
+
     foreach_html5lib_test(
         src_dir,
         "html5lib-tests/tree-construction",
         OsStr::new("dat"),
-        |path, file| {
-            let buf = io::BufReader::new(file);
-            let lines = buf.lines().map(|res| res.expect("couldn't read"));
-            let data = parse_tests(lines);
-
-            for (i, test) in data.into_iter().enumerate() {
-                make_test(
-                    &mut tests,
-                    ignores,
-                    path.file_name().unwrap().to_str().unwrap(),
-                    i,
-                    test,
-                );
-            }
-        },
+        &mut add_tests,
+    );
+
+    foreach_html5lib_test(
+        src_dir,
+        "custom-html5lib-tree-construction-tests",
+        OsStr::new("dat"),
+        &mut add_tests,
     );
 
     tests