From 6e745b5a00dfeea1a65d5b02c5bd3e834ad1b79d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 7 Oct 2015 10:52:44 +0200 Subject: [PATCH 1/3] Avoid panicking in the implementation of HTMLOptionElement#text for non-element, non-text children. --- components/script/dom/htmloptionelement.rs | 3 ++- .../the-option-element/option-text-recurse.html | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/components/script/dom/htmloptionelement.rs b/components/script/dom/htmloptionelement.rs index 462ba07021db..a300e0ca74e0 100644 --- a/components/script/dom/htmloptionelement.rs +++ b/components/script/dom/htmloptionelement.rs @@ -7,6 +7,7 @@ use dom::bindings::codegen::Bindings::CharacterDataBinding::CharacterDataMethods use dom::bindings::codegen::Bindings::HTMLOptionElementBinding; use dom::bindings::codegen::Bindings::HTMLOptionElementBinding::HTMLOptionElementMethods; use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods; +use dom::bindings::codegen::InheritTypes::ElementDerived; use dom::bindings::codegen::InheritTypes::{CharacterDataCast, ElementCast, HTMLElementCast, NodeCast, TextDerived}; use dom::bindings::codegen::InheritTypes::{HTMLOptionElementDerived}; use dom::bindings::codegen::InheritTypes::{HTMLScriptElementDerived}; @@ -71,7 +72,7 @@ fn collect_text(node: &&Node, value: &mut DOMString) { if child.r().is_text() { let characterdata = CharacterDataCast::to_ref(child.r()).unwrap(); value.push_str(&characterdata.Data()); - } else { + } else if child.is_element() { collect_text(&child.r(), value); } } diff --git a/tests/wpt/web-platform-tests/html/semantics/forms/the-option-element/option-text-recurse.html b/tests/wpt/web-platform-tests/html/semantics/forms/the-option-element/option-text-recurse.html index 46baa8e1ce5b..cf854f5260bb 100644 --- a/tests/wpt/web-platform-tests/html/semantics/forms/the-option-element/option-text-recurse.html +++ b/tests/wpt/web-platform-tests/html/semantics/forms/the-option-element/option-text-recurse.html @@ -74,4 +74,19 @@ option.appendChild(document.createTextNode("text")); assert_equals(option.text, "text"); }, "option.text should work if the option is in a MathML script element"); + +test(function() { + var option = document.createElement("option"); + option.appendChild(document.createTextNode("te")); + option.appendChild(document.createComment("comment")); + option.appendChild(document.createTextNode("xt")); + assert_equals(option.text, "text"); +}, "option.text should ignore comment children"); +test(function() { + var option = document.createElement("option"); + option.appendChild(document.createTextNode("te")); + option.appendChild(document.createProcessingInstruction("target", "data")); + option.appendChild(document.createTextNode("xt")); + assert_equals(option.text, "text"); +}, "option.text should ignore PI children"); From a701397a9dff033bcaedbd9a5e1523902469d015 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 7 Oct 2015 11:07:20 +0200 Subject: [PATCH 2/3] Pass an Element to collect_text. --- components/script/dom/htmloptionelement.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/components/script/dom/htmloptionelement.rs b/components/script/dom/htmloptionelement.rs index a300e0ca74e0..61dcc4a050bc 100644 --- a/components/script/dom/htmloptionelement.rs +++ b/components/script/dom/htmloptionelement.rs @@ -7,13 +7,12 @@ use dom::bindings::codegen::Bindings::CharacterDataBinding::CharacterDataMethods use dom::bindings::codegen::Bindings::HTMLOptionElementBinding; use dom::bindings::codegen::Bindings::HTMLOptionElementBinding::HTMLOptionElementMethods; use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods; -use dom::bindings::codegen::InheritTypes::ElementDerived; use dom::bindings::codegen::InheritTypes::{CharacterDataCast, ElementCast, HTMLElementCast, NodeCast, TextDerived}; use dom::bindings::codegen::InheritTypes::{HTMLOptionElementDerived}; use dom::bindings::codegen::InheritTypes::{HTMLScriptElementDerived}; use dom::bindings::js::Root; use dom::document::Document; -use dom::element::{AttributeMutation, ElementTypeId}; +use dom::element::{Element, AttributeMutation, ElementTypeId}; use dom::eventtarget::{EventTarget, EventTargetTypeId}; use dom::htmlelement::{HTMLElement, HTMLElementTypeId}; use dom::node::{Node, NodeTypeId}; @@ -61,19 +60,18 @@ impl HTMLOptionElement { } } -fn collect_text(node: &&Node, value: &mut DOMString) { - let elem = ElementCast::to_ref(*node).unwrap(); - let svg_script = *elem.namespace() == ns!(SVG) && elem.local_name() == &atom!("script"); - let html_script = node.is_htmlscriptelement(); +fn collect_text(element: &Element, value: &mut DOMString) { + let svg_script = *element.namespace() == ns!(SVG) && element.local_name() == &atom!("script"); + let html_script = element.is_htmlscriptelement(); if svg_script || html_script { return; } else { - for child in node.children() { + for child in NodeCast::from_ref(element).children() { if child.r().is_text() { let characterdata = CharacterDataCast::to_ref(child.r()).unwrap(); value.push_str(&characterdata.Data()); - } else if child.is_element() { - collect_text(&child.r(), value); + } else if let Some(element_child) = ElementCast::to_ref(&*child) { + collect_text(element_child, value); } } } @@ -91,9 +89,9 @@ impl HTMLOptionElementMethods for HTMLOptionElement { // https://www.whatwg.org/html/#dom-option-text fn Text(&self) -> DOMString { - let node = NodeCast::from_ref(self); + let element = ElementCast::from_ref(self); let mut content = String::new(); - collect_text(&node, &mut content); + collect_text(element, &mut content); str_join(split_html_space_chars(&content), " ") } From b96594ea509e895208cda4761734451008cd47f8 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 7 Oct 2015 11:15:20 +0200 Subject: [PATCH 3/3] Remove a pointless level of indentation. --- components/script/dom/htmloptionelement.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/components/script/dom/htmloptionelement.rs b/components/script/dom/htmloptionelement.rs index 61dcc4a050bc..c2ff22096ab8 100644 --- a/components/script/dom/htmloptionelement.rs +++ b/components/script/dom/htmloptionelement.rs @@ -65,14 +65,14 @@ fn collect_text(element: &Element, value: &mut DOMString) { let html_script = element.is_htmlscriptelement(); if svg_script || html_script { return; - } else { - for child in NodeCast::from_ref(element).children() { - if child.r().is_text() { - let characterdata = CharacterDataCast::to_ref(child.r()).unwrap(); - value.push_str(&characterdata.Data()); - } else if let Some(element_child) = ElementCast::to_ref(&*child) { - collect_text(element_child, value); - } + } + + for child in NodeCast::from_ref(element).children() { + if child.r().is_text() { + let characterdata = CharacterDataCast::to_ref(child.r()).unwrap(); + value.push_str(&characterdata.Data()); + } else if let Some(element_child) = ElementCast::to_ref(&*child) { + collect_text(element_child, value); } } }