servo: Merge #6568 - Implement Range#insertNode (from dzbarsky:delete_range); r=jdm

Gecko doesn't really follow the spec but it seems to throw a HierarchyRequest error when parent is null.
Any ideas who I should talk to about fixing the spec to account for the null checks?

Source-Repo: https://github.com/servo/servo
Source-Revision: acf47a02cf38b5c82e7c78cc1f6660a7daa9969a

servo/components/script/dom/node.rs

@@ -1495,9 +1495,10 @@ impl Node {
         // If node is an element, it is _affected by a base URL change_.
     }
 
-    // https://dom.spec.whatwg.org/#concept-node-pre-insert
-    fn pre_insert(node: &Node, parent: &Node, child: Option<&Node>)
-                  -> Fallible<Root<Node>> {
+    // https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
+    pub fn ensure_pre_insertion_validity(node: &Node,
+                                         parent: &Node,
+                                         child: Option<&Node>) -> ErrorResult {
         // Step 1.
         match parent.type_id() {
             NodeTypeId::Document |
@@ -1538,78 +1539,83 @@ impl Node {
         }
 
         // Step 6.
-        match parent.type_id() {
-            NodeTypeId::Document => {
-                match node.type_id() {
-                    // Step 6.1
-                    NodeTypeId::DocumentFragment => {
-                        // Step 6.1.1(b)
-                        if node.children()
-                               .any(|c| c.r().is_text())
-                        {
-                            return Err(HierarchyRequest);
-                        }
-                        match node.child_elements().count() {
-                            0 => (),
-                            // Step 6.1.2
-                            1 => {
-                                if !parent.child_elements().is_empty() {
-                                    return Err(HierarchyRequest);
-                                }
-                                if let Some(child) = child {
-                                    if child.inclusively_following_siblings()
-                                        .any(|child| child.r().is_doctype()) {
-                                            return Err(HierarchyRequest);
-                                    }
-                                }
-                            },
-                            // Step 6.1.1(a)
-                            _ => return Err(HierarchyRequest),
-                        }
-                    },
-                    // Step 6.2
-                    NodeTypeId::Element(_) => {
-                        if !parent.child_elements().is_empty() {
-                            return Err(HierarchyRequest);
-                        }
-                        if let Some(ref child) = child {
-                            if child.inclusively_following_siblings()
-                                .any(|child| child.r().is_doctype()) {
-                                    return Err(HierarchyRequest);
+        if parent.type_id() == NodeTypeId::Document {
+            match node.type_id() {
+                // Step 6.1
+                NodeTypeId::DocumentFragment => {
+                    // Step 6.1.1(b)
+                    if node.children()
+                           .any(|c| c.r().is_text())
+                    {
+                        return Err(HierarchyRequest);
+                    }
+                    match node.child_elements().count() {
+                        0 => (),
+                        // Step 6.1.2
+                        1 => {
+                            if !parent.child_elements().is_empty() {
+                                return Err(HierarchyRequest);
                             }
-                        }
-                    },
-                    // Step 6.3
-                    NodeTypeId::DocumentType => {
-                        if parent.children()
-                                 .any(|c| c.r().is_doctype())
-                        {
-                            return Err(HierarchyRequest);
-                        }
-                        match child {
-                            Some(child) => {
-                                if parent.children()
-                                         .take_while(|c| c.r() != child)
-                                         .any(|c| c.r().is_element())
-                                {
-                                    return Err(HierarchyRequest);
+                            if let Some(child) = child {
+                                if child.inclusively_following_siblings()
+                                    .any(|child| child.r().is_doctype()) {
+                                        return Err(HierarchyRequest);
                                 }
-                            },
-                            None => {
-                                if !parent.child_elements().is_empty() {
-                                    return Err(HierarchyRequest);
-                                }
-                            },
+                            }
+                        },
+                        // Step 6.1.1(a)
+                        _ => return Err(HierarchyRequest),
+                    }
+                },
+                // Step 6.2
+                NodeTypeId::Element(_) => {
+                    if !parent.child_elements().is_empty() {
+                        return Err(HierarchyRequest);
+                    }
+                    if let Some(ref child) = child {
+                        if child.inclusively_following_siblings()
+                            .any(|child| child.r().is_doctype()) {
+                                return Err(HierarchyRequest);
                         }
-                    },
-                    NodeTypeId::CharacterData(_) => (),
-                    NodeTypeId::Document => unreachable!(),
-                }
-            },
-            _ => (),
+                    }
+                },
+                // Step 6.3
+                NodeTypeId::DocumentType => {
+                    if parent.children()
+                             .any(|c| c.r().is_doctype())
+                    {
+                        return Err(HierarchyRequest);
+                    }
+                    match child {
+                        Some(child) => {
+                            if parent.children()
+                                     .take_while(|c| c.r() != child)
+                                     .any(|c| c.r().is_element())
+                            {
+                                return Err(HierarchyRequest);
+                            }
+                        },
+                        None => {
+                            if !parent.child_elements().is_empty() {
+                                return Err(HierarchyRequest);
+                            }
+                        },
+                    }
+                },
+                NodeTypeId::CharacterData(_) => (),
+                NodeTypeId::Document => unreachable!(),
+            }
         }
+        Ok(())
+    }
 
-        // Step 7-8.
+    // https://dom.spec.whatwg.org/#concept-node-pre-insert
+    pub fn pre_insert(node: &Node, parent: &Node, child: Option<&Node>)
+                      -> Fallible<Root<Node>> {
+        // Step 1.
+        try!(Node::ensure_pre_insertion_validity(node, parent, child));
+
+        // Steps 2-3.
         let reference_child_root;
         let reference_child = match child {
             Some(child) if child == node => {
@@ -1619,14 +1625,14 @@ impl Node {
             _ => child
         };
 
-        // Step 9.
+        // Step 4.
         let document = document_from_node(parent);
         Node::adopt(node, document.r());
 
-        // Step 10.
+        // Step 5.
         Node::insert(node, parent, reference_child, SuppressObserver::Unsuppressed);
 
-        // Step 11.
+        // Step 6.
         return Ok(Root::from_ref(node))
     }
 

servo/components/script/dom/range.rs

@@ -4,17 +4,20 @@
 
 use dom::bindings::codegen::Bindings::NodeBinding::NodeConstants;
 use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
+use dom::bindings::codegen::Bindings::NodeListBinding::NodeListMethods;
 use dom::bindings::codegen::Bindings::RangeBinding::{self, RangeConstants};
 use dom::bindings::codegen::Bindings::RangeBinding::RangeMethods;
+use dom::bindings::codegen::Bindings::TextBinding::TextMethods;
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
-use dom::bindings::codegen::InheritTypes::NodeCast;
+use dom::bindings::codegen::InheritTypes::{NodeCast, TextCast};
 use dom::bindings::error::{Error, ErrorResult, Fallible};
+use dom::bindings::error::Error::HierarchyRequest;
 use dom::bindings::global::GlobalRef;
-use dom::bindings::js::{JS, Root};
+use dom::bindings::js::{JS, Root, RootedReference};
 use dom::bindings::utils::{Reflector, reflect_dom_object};
+use dom::characterdata::CharacterDataTypeId;
 use dom::document::{Document, DocumentHelpers};
-use dom::node::{Node, NodeHelpers};
-
+use dom::node::{Node, NodeHelpers, NodeTypeId};
 use std::cell::RefCell;
 use std::cmp::{Ord, Ordering, PartialEq, PartialOrd};
 use std::rc::Rc;
@@ -288,6 +291,90 @@ impl<'a> RangeMethods for &'a Range {
     fn Detach(self) {
         // This method intentionally left blank.
     }
+
+    // https://dom.spec.whatwg.org/#dom-range-insertnode
+    // https://dom.spec.whatwg.org/#concept-range-insert
+    fn InsertNode(self, node: &Node) -> ErrorResult {
+        let (start_node, start_offset) = {
+            let inner = self.inner().borrow();
+            let start = &inner.start;
+            (start.node(), start.offset())
+        };
+
+        // Step 1.
+        match start_node.type_id() {
+            // Handled under step 2.
+            NodeTypeId::CharacterData(CharacterDataTypeId::Text) => (),
+            NodeTypeId::CharacterData(_) => return Err(HierarchyRequest),
+            _ => ()
+        }
+
+        // Step 2.
+        let (reference_node, parent) =
+            if start_node.type_id() == NodeTypeId::CharacterData(CharacterDataTypeId::Text) {
+                // Step 3.
+                let parent = match start_node.GetParentNode() {
+                    Some(parent) => parent,
+                    // Step 1.
+                    None => return Err(HierarchyRequest)
+                };
+                // Step 5.
+                (Some(Root::from_ref(start_node.r())), parent)
+            } else {
+                // Steps 4-5.
+                let child = start_node.ChildNodes().Item(start_offset);
+                (child, Root::from_ref(start_node.r()))
+            };
+
+        // Step 6.
+        try!(Node::ensure_pre_insertion_validity(node,
+                                                 parent.r(),
+                                                 reference_node.r()));
+
+        // Step 7.
+        let split_text;
+        let reference_node =
+            match TextCast::to_ref(start_node.r()) {
+                Some(text) => {
+                    split_text = try!(text.SplitText(start_offset));
+                    let new_reference = NodeCast::from_root(split_text);
+                    assert!(new_reference.GetParentNode().r() == Some(parent.r()));
+                    Some(new_reference)
+                },
+                _ => reference_node
+            };
+
+        // Step 8.
+        let reference_node = if Some(node) == reference_node.r() {
+            node.GetNextSibling()
+        } else {
+            reference_node
+        };
+
+        // Step 9.
+        node.remove_self();
+
+        // Step 10.
+        let new_offset =
+            reference_node.r().map_or(parent.len(), |node| node.index());
+
+        // Step 11
+        let new_offset = new_offset + if node.type_id() == NodeTypeId::DocumentFragment {
+            node.len()
+        } else {
+            1
+        };
+
+        // Step 12.
+        try!(Node::pre_insert(node, parent.r(), reference_node.r()));
+
+        // Step 13.
+        if self.Collapsed() {
+            self.inner().borrow_mut().set_end(parent.r(), new_offset);
+        }
+
+        Ok(())
+    }
 }
 
 #[derive(JSTraceable)]

servo/components/script/dom/webidls/Range.webidl

@@ -48,8 +48,8 @@ interface Range {
   // DocumentFragment extractContents();
   // [NewObject, Throws]
   // DocumentFragment cloneContents();
-  // [Throws]
-  // void insertNode(Node node);
+  [Throws]
+  void insertNode(Node node);
   // [Throws]
   // void surroundContents(Node newParent);