Auto merge of #7901 - Ms2ger:option-text, r=nox

Cleanup the HTMLOptionElement#text implementation.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7901)
servo · Oct 7, 2015 · 84d8f65 · 84d8f65
2 parents 409fbaf + b96594e
commit 84d8f65
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 15 deletions.
diff --git a/components/script/dom/htmloptionelement.rs b/components/script/dom/htmloptionelement.rs
@@ -12,7 +12,7 @@ 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};
@@ -60,20 +60,19 @@ 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() {
-            if child.r().is_text() {
-                let characterdata = CharacterDataCast::to_ref(child.r()).unwrap();
-                value.push_str(&characterdata.Data());
-            } else {
-                collect_text(&child.r(), 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);
         }
     }
 }
@@ -90,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), " ")
     }
 

diff --git a/...s/wpt/web-platform-tests/html/semantics/forms/the-option-element/option-text-recurse.html b/...s/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");
 </script>