Refactoring HTMLOptionElement::Text into iterative style (#37167)

The original implementation of `HTMLOptionElement::Text` is recursive,
and the program may run out of stack space for a sufficiently large
number of iterations. The patch switches to an iterative implementation,
with `TreeIterator`.

Note that, instead of the usual `while let Some(node) = iterator.next()`
approach, we use `while let Some(node) = iterator.peek()` with the newly
added `TreeIterator::peek` function. This is because the choice of the
next node depends on some checks performed inside the `while` block,
whereas the `next` function determines the next node before entering the
block.

Moreover, the `TreeIterator::peek` function is added, instead of
wrapping the iterator into `Peekable`. This is because we will lose
access to the `TreeIterator::next_skipping_children` function if we wrap
it into `Peekable`.

Testing: This refactoring has to pass the existing tests.
Fixes: #36959

Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
This commit is contained in:
Kingsley Yung 2025-05-29 01:58:33 +08:00 committed by GitHub
parent 398764a928
commit be7efc94a3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 26 additions and 21 deletions

View file

@ -152,25 +152,6 @@ impl HTMLOptionElement {
}
}
// FIXME(ajeffrey): Provide a way of buffering DOMStrings other than using Strings
fn collect_text(element: &Element, value: &mut String) {
let svg_script =
*element.namespace() == ns!(svg) && element.local_name() == &local_name!("script");
let html_script = element.is::<HTMLScriptElement>();
if svg_script || html_script {
return;
}
for child in element.upcast::<Node>().children() {
if child.is::<Text>() {
let characterdata = child.downcast::<CharacterData>().unwrap();
value.push_str(&characterdata.Data());
} else if let Some(element_child) = child.downcast() {
collect_text(element_child, value);
}
}
}
impl HTMLOptionElementMethods<crate::DomTypeHolder> for HTMLOptionElement {
/// <https://html.spec.whatwg.org/multipage/#dom-option>
fn Option(
@ -216,8 +197,28 @@ impl HTMLOptionElementMethods<crate::DomTypeHolder> for HTMLOptionElement {
/// <https://html.spec.whatwg.org/multipage/#dom-option-text>
fn Text(&self) -> DOMString {
let mut content = String::new();
collect_text(self.upcast(), &mut content);
let mut content = DOMString::new();
let mut iterator = self.upcast::<Node>().traverse_preorder(ShadowIncluding::No);
while let Some(node) = iterator.peek() {
if let Some(element) = node.downcast::<Element>() {
let html_script = element.is::<HTMLScriptElement>();
let svg_script = *element.namespace() == ns!(svg) &&
element.local_name() == &local_name!("script");
if html_script || svg_script {
iterator.next_skipping_children();
continue;
}
}
if node.is::<Text>() {
let characterdata = node.downcast::<CharacterData>().unwrap();
content.push_str(&characterdata.Data());
}
iterator.next();
}
DOMString::from(str_join(split_html_space_chars(&content), " "))
}

View file

@ -2035,6 +2035,10 @@ impl TreeIterator {
self.current = None;
Some(current)
}
pub(crate) fn peek(&self) -> Option<&DomRoot<Node>> {
self.current.as_ref()
}
}
impl Iterator for TreeIterator {