Auto merge of #252 - jdm:revert-form-owner, r=jdm

Revert "Implement support for form owner"

This reverts commit 69f7069 (#249).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/html5ever/252)
<!-- Reviewable:end -->

Author: bors-servo

examples/noop-tree-builder.rs

@@ -57,10 +57,6 @@ impl TreeSink for Sink {
         x == y
     }
 
-    fn same_tree(&self, _x: usize, _y: usize) -> bool {
-        true
-    }
-
     fn elem_name(&self, target: usize) -> QualName {
         self.names.get(&target).expect("not an element").clone()
     }
@@ -75,15 +71,13 @@ impl TreeSink for Sink {
         self.get_id()
     }
 
-    fn has_parent_node(&self, _node: usize) -> bool {
-        // `node` will have a parent unless a script moved it, and we're
-        // not running scripts.  Therefore we can aways return true.
-        true
-    }
-
     fn append_before_sibling(&mut self,
             _sibling: usize,
-            _new_node: NodeOrText<usize>) { }
+            _new_node: NodeOrText<usize>) -> Result<(), NodeOrText<usize>> {
+        // `sibling` will have a parent unless a script moved it, and we're
+        // not running scripts.  Therefore we can aways return `Ok(())`.
+        Ok(())
+    }
 
     fn parse_error(&mut self, _msg: Cow<'static, str>) { }
     fn set_quirks_mode(&mut self, _mode: QuirksMode) { }

examples/print-tree-actions.rs

@@ -82,12 +82,6 @@ impl TreeSink for Sink {
         id
     }
 
-    fn has_parent_node(&self, _node: usize) -> bool  {
-        // `node` will have a parent unless a script moved it, and we're
-        // not running scripts.  Therefore we can aways return true
-        true
-    }
-
     fn append(&mut self, parent: usize, child: NodeOrText<usize>) {
         match child {
             AppendNode(n)
@@ -99,13 +93,17 @@ impl TreeSink for Sink {
 
     fn append_before_sibling(&mut self,
             sibling: usize,
-            new_node: NodeOrText<usize>) {
+            new_node: NodeOrText<usize>) -> Result<(), NodeOrText<usize>> {
         match new_node {
             AppendNode(n)
                 => println!("Append node {} before {}", n, sibling),
             AppendText(t)
                 => println!("Append text before {}: \"{}\"", sibling, escape_default(&t)),
         }
+
+        // `sibling` will have a parent unless a script moved it, and we're
+        // not running scripts.  Therefore we can aways return `Ok(())`.
+        Ok(())
     }
 
     fn append_doctype_to_document(&mut self,
@@ -123,12 +121,6 @@ impl TreeSink for Sink {
         }
     }
 
-    fn associate_with_form(&mut self, _target: usize, _form: usize) {
-        // No form owner support. Since same_tree always returns
-        // true we cannot be sure that this associate_with_form call is
-        // valid
-    }
-
     fn remove_from_parent(&mut self, target: usize) {
         println!("Remove {} from parent", target);
     }

src/rcdom.rs

@@ -223,11 +223,6 @@ impl TreeSink for RcDom {
         new_node(Comment(text))
     }
 
-    fn has_parent_node(&self, node: Handle) -> bool {
-        let node = node.borrow();
-        node.parent.is_some()
-    }
-
     fn append(&mut self, parent: Handle, child: NodeOrText<Handle>) {
         // Append to an existing Text node if we have one.
         match child {
@@ -246,9 +241,8 @@ impl TreeSink for RcDom {
 
     fn append_before_sibling(&mut self,
             sibling: Handle,
-            child: NodeOrText<Handle>) {
-        let (parent, i) = get_parent_and_index(&sibling)
-            .expect("append_before_sibling called on node without parent");
+            child: NodeOrText<Handle>) -> Result<(), NodeOrText<Handle>> {
+        let (parent, i) = unwrap_or_return!(get_parent_and_index(&sibling), Err(child));
 
         let child = match (child, i) {
             // No previous node.
@@ -259,7 +253,7 @@ impl TreeSink for RcDom {
                 let parent = parent.borrow();
                 let prev = &parent.children[i-1];
                 if append_to_existing_text(prev, &text) {
-                    return;
+                    return Ok(());
                 }
                 new_node(Text(text))
             }
@@ -277,6 +271,7 @@ impl TreeSink for RcDom {
 
         child.borrow_mut().parent = Some(Rc::downgrade(&parent));
         parent.borrow_mut().children.insert(i, child);
+        Ok(())
     }
 
     fn append_doctype_to_document(&mut self,

src/tree_builder/actions.rs

@@ -210,8 +210,40 @@ impl<Handle, Sink> TreeBuilderActions<Handle>
 
     // Insert at the "appropriate place for inserting a node".
     fn insert_appropriately(&mut self, child: NodeOrText<Handle>, override_target: Option<Handle>) {
-        let insertion_point = self.appropriate_place_for_insertion(override_target);
-        self.insert_at(insertion_point, child);
+        declare_tag_set!(foster_target = "table" "tbody" "tfoot" "thead" "tr");
+        let target = override_target.unwrap_or_else(|| self.current_node());
+        if !(self.foster_parenting && self.elem_in(target.clone(), foster_target)) {
+            if self.html_elem_named(target.clone(), local_name!("template")) {
+                // No foster parenting (inside template).
+                let contents = self.sink.get_template_contents(target);
+                self.sink.append(contents, child);
+            } else {
+                // No foster parenting (the common case).
+                self.sink.append(target, child);
+            }
+            return;
+        }
+
+        // Foster parenting
+        let mut iter = self.open_elems.iter().rev().peekable();
+        while let Some(elem) = iter.next() {
+            if self.html_elem_named(elem.clone(), local_name!("template")) {
+                let contents = self.sink.get_template_contents(elem.clone());
+                self.sink.append(contents, child);
+                return;
+            } else if self.html_elem_named(elem.clone(), local_name!("table")) {
+                // Try inserting "inside last table's parent node, immediately before last table"
+                if let Err(child) = self.sink.append_before_sibling(elem.clone(), child) {
+                    // If last_table has no parent, we regain ownership of the child.
+                    // Insert "inside previous element, after its last child (if any)"
+                    let previous_element = (*iter.peek().unwrap()).clone();
+                    self.sink.append(previous_element, child);
+                }
+                return;
+            }
+        }
+        let html_elem = self.html_elem();
+        self.sink.append(html_elem, child);
     }
 
     fn adoption_agency(&mut self, subject: LocalName) {
@@ -743,40 +775,10 @@ impl<Handle, Sink> TreeBuilderActions<Handle>
         // FIXME: application cache selection algorithm
     }
 
-    // https://html.spec.whatwg.org/multipage/#create-an-element-for-the-token
     fn insert_element(&mut self, push: PushFlag, ns: Namespace, name: LocalName, attrs: Vec<Attribute>)
             -> Handle {
-        declare_tag_set!(form_associatable =
-            "button" "fieldset" "input" "object"
-            "output" "select" "textarea" "img");
-
-        declare_tag_set!(listed = [form_associatable] - "img");
-
-        // Step 7.
-        let qname = QualName::new(ns, name);
-        let elem = self.sink.create_element(qname.clone(), attrs.clone());
-
-        let insertion_point = self.appropriate_place_for_insertion(None);
-        let tree_node = match insertion_point {
-            LastChild(ref p) |
-            BeforeSibling(ref p) => p.clone()
-        };
-
-        // Step 12.
-        if form_associatable(qname.clone()) &&
-           self.form_elem.is_some() &&
-           !self.in_html_elem_named(local_name!("template")) &&
-           !(listed(qname.clone()) &&
-                attrs.iter().any(|a| a.name == qualname!("","form"))) {
-
-               let form = self.form_elem.as_ref().unwrap().clone();
-               if self.sink.same_tree(tree_node, form.clone()) {
-                   self.sink.associate_with_form(elem.clone(), form)
-               }
-        }
-
-        self.insert_at(insertion_point, AppendNode(elem.clone()));
-
+        let elem = self.sink.create_element(QualName::new(ns, name), attrs);
+        self.insert_appropriately(AppendNode(elem.clone()), None);
         match push {
             Push => self.push(&elem),
             NoPush => (),

src/tree_builder/interface.rs

@@ -72,11 +72,6 @@ pub trait TreeSink {
     /// Do two handles refer to the same node?
     fn same_node(&self, x: Self::Handle, y: Self::Handle) -> bool;
 
-    /// Are two handles present in the same tree
-    fn same_tree(&self, x: Self::Handle, y: Self::Handle) -> bool {
-        true
-    }
-
     /// What is the name of this element?
     ///
     /// Should never be called on a non-element node;
@@ -98,18 +93,15 @@ pub trait TreeSink {
     /// Create a comment node.
     fn create_comment(&mut self, text: StrTendril) -> Self::Handle;
 
-    /// Does the node have a parent?
-    fn has_parent_node(&self, node: Self::Handle) -> bool;
-
     /// Append a node as the last child of the given node.  If this would
     /// produce adjacent sibling text nodes, it should concatenate the text
     /// instead.
     ///
     /// The child node will not already have a parent.
     fn append(&mut self, parent: Self::Handle, child: NodeOrText<Self::Handle>);
 
-    /// Append a node as the sibling immediately before the given node.
-    /// This method will only be called if has_parent_node(sibling) is true
+    /// Append a node as the sibling immediately before the given node.  If that node
+    /// has no parent, do nothing and return Err(new_node).
     ///
     /// The tree builder promises that `sibling` is not a text node.  However its
     /// old previous sibling, which would become the new node's previous sibling,
@@ -119,7 +111,7 @@ pub trait TreeSink {
     /// NB: `new_node` may have an old parent, from which it should be removed.
     fn append_before_sibling(&mut self,
         sibling: Self::Handle,
-        new_node: NodeOrText<Self::Handle>);
+        new_node: NodeOrText<Self::Handle>) -> Result<(), NodeOrText<Self::Handle>>;
 
     /// Append a `DOCTYPE` element to the `Document` node.
     fn append_doctype_to_document(&mut self,
@@ -132,9 +124,6 @@ pub trait TreeSink {
     /// with something else than an element.
     fn add_attrs_if_missing(&mut self, target: Self::Handle, attrs: Vec<Attribute>);
 
-    /// Associate the given form-associatable element with the form element
-    fn associate_with_form(&mut self, target: Self::Handle, form: Self::Handle) {}
-
     /// Detach the given node from its parent.
     fn remove_from_parent(&mut self, target: Self::Handle);
 

src/tree_builder/mod.rs

@@ -369,53 +369,6 @@ impl<Handle, Sink> TreeBuilder<Handle, Sink>
     pub fn is_fragment(&self) -> bool {
         self.context_elem.is_some()
     }
-
-    fn appropriate_place_for_insertion(&mut self,
-                                       override_target: Option<Handle>)
-                                       -> InsertionPoint<Handle> {
-        use self::tag_sets::*;
-
-        declare_tag_set!(foster_target = "table" "tbody" "tfoot" "thead" "tr");
-        let target = override_target.unwrap_or_else(|| self.current_node());
-        if !(self.foster_parenting && self.elem_in(target.clone(), foster_target)) {
-            if self.html_elem_named(target.clone(), local_name!("template")) {
-                // No foster parenting (inside template).
-                let contents = self.sink.get_template_contents(target);
-                return LastChild(contents);
-            } else {
-                // No foster parenting (the common case).
-                return LastChild(target);
-            }
-        }
-
-        // Foster parenting
-        let mut iter = self.open_elems.iter().rev().peekable();
-        while let Some(elem) = iter.next() {
-            if self.html_elem_named(elem.clone(), local_name!("template")) {
-                let contents = self.sink.get_template_contents(elem.clone());
-                return LastChild(contents);
-            } else if self.html_elem_named(elem.clone(), local_name!("table")) {
-                // Try inserting "inside last table's parent node, immediately before last table"
-                if self.sink.has_parent_node(elem.clone()) {
-                    return BeforeSibling(elem.clone());
-                } else {
-                    // If elem has no parent, we regain ownership of the child.
-                    // Insert "inside previous element, after its last child (if any)"
-                    let previous_element = (*iter.peek().unwrap()).clone();
-                    return LastChild(previous_element);
-                }
-            }
-        }
-        let html_elem = self.html_elem();
-        LastChild(html_elem)
-    }
-
-    fn insert_at(&mut self, insertion_point: InsertionPoint<Handle>, child: NodeOrText<Handle>) {
-        match insertion_point {
-            LastChild(parent) => self.sink.append(parent, child),
-            BeforeSibling(sibling) => self.sink.append_before_sibling(sibling, child)
-        }
-    }
 }
 
 impl<Handle, Sink> TokenSink
@@ -576,18 +529,13 @@ mod test {
             self.rcdom.create_comment(text)
         }
 
-        fn has_parent_node(&self, node: Handle) -> bool {
-            let node = node.borrow();
-            node.parent.is_some()
-        }
-
         fn append(&mut self, parent: Handle, child: NodeOrText<Handle>) {
             self.rcdom.append(parent, child)
         }
 
         fn append_before_sibling(&mut self,
                 sibling: Handle,
-                child: NodeOrText<Handle>) {
+                child: NodeOrText<Handle>) -> Result<(), NodeOrText<Handle>> {
             self.rcdom.append_before_sibling(sibling, child)
         }
 

src/tree_builder/types.rs

@@ -19,7 +19,6 @@ pub use self::SplitStatus::*;
 pub use self::Token::*;
 pub use self::ProcessResult::*;
 pub use self::FormatEntry::*;
-pub use self::InsertionPoint::*;
 
 #[derive(PartialEq, Eq, Copy, Clone, Debug)]
 pub enum InsertionMode {
@@ -81,13 +80,3 @@ pub enum FormatEntry<Handle> {
     Element(Handle, Tag),
     Marker,
 }
-
-pub enum InsertionPoint<Handle> {
-    /// Holds the parent
-    LastChild(Handle),
-    /// Holds the sibling before which the node will be inserted
-    /// TODO: Is the parent node needed? Is there a problem with using
-    /// the sibling to find if the form element is in the same home
-    /// subtree?
-    BeforeSibling(Handle)
-}