Clone implicitly closed options into selectedcontent

[staylor]

Fixes #712.

Problem

The tree builder only called TreeSink::maybe_clone_an_option_into_selectedcontent from a special case for an explicit </option> end tag (the FIXME in rules.rs that points at #712). If the option was closed any other way, it was never cloned into the <selectedcontent>. That covers another <option>, <optgroup> or <hr>, </select>, <input> or a nested <select>, an ancestor's end tag, the adoption agency algorithm, clearing the stack back to a table context, and the end of the input:

<select><button><selectedcontent></selectedcontent></button><option selected>X<option>Y</select>
<!-- main: <selectedcontent> stays empty; expected: it contains "X" -->

A related problem: several paths removed elements from the stack of open elements without calling TreeSink::pop, so sinks that track pops never heard about those elements:

• open_elems.truncate(..) in the adoption agency algorithm (when there is no furthest block), in "any other end tag" (process_end_tag_in_body), for <frameset>, and for end tags in foreign content;
• open_elems.remove(..) in the adoption agency's inner loop, which removes nodes between the formatting element and the furthest block;
• pop_until and pop_until_current, which most end tags go through (</div>, </select>, </table>, closing table cells, and so on).

For <div><span></div> on main, the sink hears about neither the span nor the div (print-tree-actions only reports head, body and html being popped). Even the explicit </option> path called the selectedcontent hook without ever reporting that the option was popped.

Spec

• The option element: "When an option element option is popped off the stack of open elements of an HTML parser or XML parser, update descendant selectedcontent elements for an option given option."
• whatwg/html@4ce63af1 ("Improve behavior for parsing option end tags", fixes Popping node vs. end tag behavior in <option> and <selectedcontent> whatwg/html#11653) removed the "An end tag whose tag name is 'option'" step from the "in body" insertion mode and added the requirement above, which at the time said to run "maybe clone an option into selectedcontent". whatwg/html@b346db728f has since renamed the algorithm. Either way, the parser's job is the same: run the option's steps every time an option is popped.
• Stop parsing, step 4: "Pop all the nodes off the stack of open elements." So options that are still open at the end of the input are popped too.

Fix

• Every removal from the stack of open elements now goes through one method, TreeBuilder::remove_from_stack_at(index). It removes the element, calls TreeSink::pop, and then, for HTML option elements, calls TreeSink::maybe_clone_an_option_into_selectedcontent. The other helpers are built on it: pop, remove_from_stack, a new pop_to_len (which replaces the four truncations and the drain in end()), pop_until, pop_until_current, and the adoption agency's inner-loop removals. The RefCell borrow of the stack is released before calling into the sink. The old end() held it across all of its TreeSink::pop calls.
• The </option> special case is gone. </option> is now handled by "any other end tag", as in the spec.
• Removing an element from the middle of the stack counts as a pop. remove_from_stack already reported these to the sink, and Blink does the same: HTMLElementStack::RemoveNonTopCommon calls FinishParsingChildren, which is where it updates selectedcontent. The adoption agency's "replace the entry for node in the stack of open elements" does not count as a pop. It only ever replaces formatting elements, and Blink's HTMLElementStack::Replace doesn't treat it as one either.
• The docs for TreeSink::maybe_clone_an_option_into_selectedcontent now describe when it is called: right after TreeSink::pop, for every popped option.

rcdom

The tree construction tests run through rcdom, and rcdom's hook never did anything. get_a_selects_enabled_selectedcontent checked the select's own name (self.data) instead of each descendant's (node.data), so it never found a <selectedcontent>, not even after an explicit </option>. Fixing that meant rcdom's clone code ran for the first time, and that showed three problems with it:

• "Replace all" overwrote selectedcontent.children without clearing the removed children's parent pointers. Clones also copied their originals' parent pointers. With input like <select><b><selectedcontent><div><option selected>X</option></b>, the adoption agency later panicked with "have parent but couldn't find in parent's children!". The fuzz target parses with RcDom, so a fuzzer could reach this.
• The clone can remove elements that are still on the stack of open elements from the tree, for example when an option is nested inside the selectedcontent. When such a subtree was dropped, Drop for Node took apart nodes that were still referenced and left dangling parent pointers.
• The clone was recursive, unlike rcdom's iterative Drop and serializer.

The clone now detaches the old children and appends the new ones. clone_with_subtree is iterative and sets parent pointers correctly. Drop clears the parent pointer of each child of a node it drops, and leaves alone any child that is still referenced elsewhere.

Tests

• rcdom/custom-html5lib-tree-construction-tests/selectedcontent.dat: 13 html5lib-format tree construction tests. rcdom/tests/html-tree-builder.rs now reads this directory as well as html5lib-tests/tree-construction, the same way html-tokenizer.rs reads custom-html5lib-tokenizer-tests. Each test closes a selected option a different way:

  • an explicit </option>
  • <option>, <optgroup> or <hr>
  • </select>, with and without a child of the option still open
  • the end of the input, with and without misnested formatting elements (the second is webkit02.dat-45 plus selected)
  • a formatting element's end tag (adoption agency algorithm)
  • an ancestor's end tag ("any other end tag")
  • a nested <select>, or <input>
  • <td>, which clears the stack back to a table row context

  The options use a selected attribute because rcdom only treats options with that attribute as selected. I checked the expected trees against Chromium 147 (headless, DOMParser) and all 13 match. The #errors sections come from the spec, and their counts match what html5ever reports with exact_errors.

• html5ever/tests/tree_builder.rs: uses a sink that builds no tree and logs pop and maybe_clone_an_option_into_selectedcontent calls.

  • It checks that elements are reported as popped when they are closed through each of the paths listed above: pop_until, clearing the stack back to a table context, the adoption agency with and without a furthest block, "any other end tag", end tags in foreign content, and <frameset>.
  • It checks that every popped HTML option gets exactly one hook call right after its pop, whether it was closed by </option>, <option>, <optgroup>, <hr>, </select> or the end of the input. That includes </option>, so a double call would be caught.
  • It checks that an SVG <option> doesn't get a hook call.

  Both tests fail on main.

Results, before and after this change. For the html5lib and WPT rows I ran the compiled html-tree-builder harness with an empty ignore list:

	main	this PR
selectedcontent.dat (13 cases × 2 scripting modes)	0/26	26/26
↳ with only the rcdom lookup fix (to separate its effect from the tree builder change)	2/26 (explicit </option> only)	n/a
html5lib-tests webkit02.dat-44, -45 (pinned submodule, skipped in data/test/ignore)	fail	fail
WPT html/syntax/parsing/resources/webkit02.dat-44…-47 (tree construction tests now live in WPT; -46 and -47 aren't in the pinned submodule)	0/4	1/4 (-47)
the same four WPT cases with a scratch sink that also selects the first option by default	0/4	4/4
html5ever/tests/tree_builder.rs	0/2	2/2
whole pinned html5lib tree construction suite, empty ignore list	3511/3515 (only -44/-45 fail)	3511/3515 (unchanged)

With an empty ignore list, the only html5lib failures on both main and this branch are webkit02.dat-44 and -45 (×2 scripting modes). Both still fail under rcdom for a reason that has nothing to do with the tree builder. Neither option has a selected attribute, and the spec selects the first option by default, which rcdom doesn't model (see html5lib/html5lib-tests#180). With a sink that does model this, the tree builder now passes them, so they stay in the ignore list for now. WPT-46 fails for the same reason.

On top of that I ran a randomized stress test outside the repo. It parsed 2.3 million random, select-heavy tag-soup documents with RcDom, as documents and as select-context fragments, and serialized them, like the fuzz target. With only the lookup fix it found the rcdom panic described above. With this PR it finds nothing. A 200,000-deep subtree inside a selected option also clones fine.

cargo fmt --all -- --check, cargo clippy --all-features --all-targets, cargo test --workspace, cargo doc with -D warnings, and the MSRV cargo check --lib --all-features on Rust 1.85 all pass.

Notes for reviewers

• Servo will now get maybe_clone_an_option_into_selectedcontent for implicitly closed options. It will also get TreeSink::pop for elements it never heard about before. Its pop hooks (<style>, <title>, <textarea>) were already reached through pop() in the "text" insertion mode, so they are unaffected.
• A sink that worked around this by cloning options from TreeSink::pop should move that work to maybe_clone_an_option_into_selectedcontent, or it will clone twice.
• Not changed here:
  • void elements are still inserted without being pushed, so TreeSink::pop is never called for them;
  • xml5ever still doesn't call the hook (as documented), although the spec requirement also covers the XML parser;
  • rcdom still doesn't model default selectedness or selectedcontent disabledness;
  • the spec link on maybe_clone_an_option_into_selectedcontent (#maybe-clone-an-option-into-selectedcontent) no longer resolves since whatwg/html@b346db728f renamed the algorithm.

---

This patch and its description were written with an AI coding assistant.