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); - } + // + // 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: + 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) -> 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!()) => { - 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!(

)) => { 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>>, + log: RefCell>, +} + +impl Sink { + fn new() -> Self { + Sink { + // The document. + names: RefCell::new(vec![None]), + log: RefCell::default(), + } + } + + fn new_node(&self, name: Option) -> 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, 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) {} + fn append_before_sibling(&self, _sibling: &usize, _new_node: NodeOrText) {} + fn append_based_on_parent_node(&self, _: &usize, _: &usize, _: NodeOrText) {} + fn append_doctype_to_document(&self, _: StrTendril, _: StrTendril, _: StrTendril) {} + fn add_attrs_if_missing(&self, _target: &usize, _attrs: Vec) {} + fn remove_from_parent(&self, _target: &usize) {} + fn reparent_children(&self, _node: &usize, _new_parent: &usize) {} +} + +/// Parse `input` as the contents of a `` 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("
"), + "pop span, pop div, pop html" + ); + // Clearing the stack back to a table context. + assert_eq!( + parse_body_fragment(""), + "pop b, pop tr, pop tbody, pop table, pop html" + ); + // The adoption agency algorithm, without a furthest block. + assert_eq!(parse_body_fragment(""), "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("
"), + "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(""), + "pop em, pop span, pop html" + ); + // An end tag in foreign content. + assert_eq!( + parse_body_fragment(""), + "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("

"), + "pop head, pop p, pop body, pop frameset, pop html" + ); +} + +/// +#[test] +fn maybe_clone_an_option_into_selectedcontent_runs_for_every_popped_option() { + assert_eq!( + parse_body_fragment(""), + "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( + "` end tag, + /// by another `


+#errors +#document +| +| +| +| +|
+#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 +| +| +| +| +|