RFC: Fix duplicate root html

src/components/main/layout/box_builder.rs

@@ -18,7 +18,7 @@ use layout::inline::{InlineFlowData, InlineLayout};
 use newcss::values::{CSSDisplay, CSSDisplayBlock, CSSDisplayInline, CSSDisplayInlineBlock};
 use newcss::values::{CSSDisplayNone};
 use script::dom::element::*;
-use script::dom::node::{AbstractNode, CommentNodeTypeId, DoctypeNodeTypeId};
+use script::dom::node::{AbstractNode, CommentNodeTypeId, DoctypeNodeTypeId, DocumentNodeTypeId};
 use script::dom::node::{ElementNodeTypeId, LayoutView, TextNodeTypeId};
 use servo_util::range::Range;
 use servo_util::tree::{TreeNodeRef, TreeUtils};
@@ -66,7 +66,7 @@ priv fn simulate_UA_display_rules(node: AbstractNode<LayoutView>) -> CSSDisplay
     }
 
     match node.type_id() {
-        DoctypeNodeTypeId | CommentNodeTypeId => CSSDisplayNone,
+        DoctypeNodeTypeId | CommentNodeTypeId | DocumentNodeTypeId => CSSDisplayNone,
         TextNodeTypeId => CSSDisplayInline,
         ElementNodeTypeId(element_type_id) => {
             match element_type_id {
@@ -99,7 +99,7 @@ impl BoxGenerator {
         return false;
     }
 
-    // TODO: implement this, generating spacer 
+    // TODO: implement this, generating spacer
     fn make_inline_spacer_for_node_side(&self,
                                         _: &LayoutContext,
                                         _: AbstractNode<LayoutView>,
@@ -210,15 +210,15 @@ impl BuilderContext {
         debug!("BuilderContext: cloning context");
         copy self
     }
-    
+
     priv fn attach_child_flow(&self, child: FlowContext) {
         let default_collector = &mut *self.default_collector;
         debug!("BuilderContext: Adding child flow f%? of f%?",
                default_collector.flow.id(),
                child.id());
         default_collector.flow.add_child(child);
     }
-    
+
     priv fn create_child_flow_of_type(&self,
                                       flow_type: FlowContextType,
                                       builder: &mut LayoutTreeBuilder,
@@ -228,7 +228,7 @@ impl BuilderContext {
 
         BuilderContext::new(@mut BoxGenerator::new(new_flow))
     }
-        
+
     priv fn make_inline_collector(&mut self,
                                   builder: &mut LayoutTreeBuilder,
                                   node: AbstractNode<LayoutView>)
@@ -271,7 +271,7 @@ impl BuilderContext {
             v => v
         };
 
-        let containing_context = match (simulated_display, self.default_collector.flow) { 
+        let containing_context = match (simulated_display, self.default_collector.flow) {
             (CSSDisplayBlock, BlockFlow(info)) => match (info.is_root, node.parent_node()) {
                 // If this is the root node, then use the root flow's
                 // context. Otherwise, make a child block context.
@@ -308,7 +308,7 @@ pub impl LayoutTreeBuilder {
 
         let mut this_ctx = match parent_ctx.containing_context_for_node(cur_node, self) {
             Some(ctx) => ctx,
-            None => { return; } // no context because of display: none. Stop building subtree. 
+            None => { return; } // no context because of display: none. Stop building subtree.
         };
         debug!("point a: %s", cur_node.debug_str());
         this_ctx.default_collector.push_node(layout_ctx, self, cur_node);
@@ -383,13 +383,13 @@ pub impl LayoutTreeBuilder {
                         let boxes = &first_flow.inline().boxes;
                         if boxes.len() == 1 && boxes[0].is_whitespace_only() {
                             debug!("LayoutTreeBuilder: pruning whitespace-only first child flow \
-                                    f%d from parent f%d", 
+                                    f%d from parent f%d",
                                    first_flow.id(),
                                    parent_flow.id());
                             do_remove = true;
                         }
                         }
-                        if (do_remove) { 
+                        if (do_remove) {
                             parent_flow.remove_child(*first_flow);
                         }
                     }
@@ -402,7 +402,7 @@ pub impl LayoutTreeBuilder {
                         let boxes = &last_flow.inline().boxes;
                         if boxes.len() == 1 && boxes.last().is_whitespace_only() {
                             debug!("LayoutTreeBuilder: pruning whitespace-only last child flow \
-                                    f%d from parent f%d", 
+                                    f%d from parent f%d",
                                    last_flow.id(),
                                    parent_flow.id());
                             do_remove = true;
@@ -419,7 +419,7 @@ pub impl LayoutTreeBuilder {
     }
 
     fn fixup_split_inline(&self, _: FlowContext) {
-        // TODO: finish me. 
+        // TODO: finish me.
         fail!(~"TODO: handle case where an inline is split by a block")
     }
 

src/components/script/dom/bindings/document.rs

@@ -8,6 +8,8 @@ use dom::bindings::utils::{jsval_to_str, WrapNewBindingObject, CacheableWrapper}
 use dom::bindings::utils;
 use dom::document::Document;
 use dom::htmlcollection::HTMLCollection;
+use dom::node::{AbstractNode, ScriptView, ElementNodeTypeId};
+use dom::element::{HTMLHtmlElementTypeId};
 use js::glue::bindgen::*;
 use js::glue::{PROPERTY_STUB, STRICT_PROPERTY_STUB};
 use js::jsapi::bindgen::{JS_DefineProperties};
@@ -18,6 +20,7 @@ use js::rust::{Compartment, jsobj};
 use js::{JSPROP_NATIVE_ACCESSORS};
 use js::{JS_ARGV, JSPROP_ENUMERATE, JSPROP_SHARED, JSVAL_NULL, JS_THIS_OBJECT, JS_SET_RVAL};
 use script_task::task_from_context;
+use servo_util::tree::{TreeUtils};
 
 use core::libc::c_uint;
 use core::ptr::null;
@@ -29,14 +32,26 @@ extern fn getDocumentElement(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> J
             return 0;
         }
 
+        //FIXME(jj): These should not be mutable.
         let doc = &mut (*unwrap(obj)).payload;
         let root = &mut doc.root;
-        assert!(root.is_element());
-        root.wrap(cx, ptr::null(), vp); //XXXjdm proper scope at some point
+        let mut root_el = get_root_html_node(root);
+        root_el.wrap(cx, ptr::null(), vp); //XXXjdm proper scope at some point
         return 1;
     }
 }
 
+// TODO(jj): Handle xml etc.
+fn get_root_html_node(doc_node: &AbstractNode<ScriptView>) -> AbstractNode<ScriptView> {
+    // We need to look at the children of the document node to find the root html node of the document.
+    for doc_node.each_child |kid| {
+        if kid.is_element() && kid.type_id() == ElementNodeTypeId(HTMLHtmlElementTypeId) {
+            return kid;
+        }
+    }
+    fail!("The document has no root html element!")
+}
+
 extern fn getElementsByTagName(cx: *JSContext, _argc: c_uint, vp: *JSVal) -> JSBool {
     unsafe {
         let obj = JS_THIS_OBJECT(cx, vp);

src/components/script/dom/bindings/element.rs

@@ -7,7 +7,7 @@ use dom::bindings::utils::jsval_to_str;
 use dom::bindings::utils::{domstring_to_jsval, WrapNewBindingObject};
 use dom::bindings::utils::{str, CacheableWrapper, DOM_OBJECT_SLOT, DOMString};
 use dom::element::*;
-use dom::node::{AbstractNode, Element, ElementNodeTypeId, ScriptView};
+use dom::node::{AbstractNode, Element, ElementNodeTypeId, ScriptView, DocumentNodeTypeId};
 use layout_interface::{ContentBoxQuery, ContentBoxResponse};
 use script_task::task_from_context;
 use super::utils;
@@ -272,7 +272,7 @@ extern fn getTagName(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool {
         let node = unwrap(obj);
         do node.with_imm_element |elem| {
             let s = str(copy elem.tag_name);
-            *vp = domstring_to_jsval(cx, &s);            
+            *vp = domstring_to_jsval(cx, &s);
         }
     }
     return 1;
@@ -285,6 +285,10 @@ pub fn create(cx: *JSContext, node: &mut AbstractNode<ScriptView>) -> jsobj {
         ElementNodeTypeId(HTMLImageElementTypeId) => ~"HTMLImageElement",
         ElementNodeTypeId(HTMLScriptElementTypeId) => ~"HTMLScriptElement",
         ElementNodeTypeId(_) => ~"HTMLElement",
+        // FIXME(jj): This is gross.
+        // This dummy document element should not be created.
+        // We need a way to combine the document and this node.
+        DocumentNodeTypeId => ~"HTMLElement",
         _ => fail!(~"element::create only handles elements")
     };
 

src/components/script/dom/bindings/node.rs

@@ -7,7 +7,7 @@ use dom::bindings::text;
 use dom::bindings::utils;
 use dom::bindings::utils::{CacheableWrapper, WrapperCache, DerivedWrapper};
 use dom::node::{AbstractNode, Node, ElementNodeTypeId, TextNodeTypeId, CommentNodeTypeId};
-use dom::node::{DoctypeNodeTypeId, ScriptView};
+use dom::node::{DoctypeNodeTypeId, DocumentNodeTypeId, ScriptView};
 
 use core::libc::c_uint;
 use core::ptr::null;
@@ -44,7 +44,7 @@ pub fn init(compartment: @mut Compartment) {
          flags: (JSPROP_SHARED | JSPROP_ENUMERATE | JSPROP_NATIVE_ACCESSORS) as u8,
          getter: JSPropertyOpWrapper {op: getNodeType, info: null()},
          setter: JSStrictPropertyOpWrapper {op: null(), info: null()}},
-        
+
         JSPropertySpec {
          name: null(),
          tinyid: 0,
@@ -64,6 +64,10 @@ pub fn create(cx: *JSContext, node: &mut AbstractNode<ScriptView>) -> jsobj {
         TextNodeTypeId |
         CommentNodeTypeId |
         DoctypeNodeTypeId => text::create(cx, node),
+        // FIXME(jj): This is gross.
+        // I think we want to somehow create the document along with the document node.
+        // i.e. they should be the same thing.
+        DocumentNodeTypeId => element::create(cx, node),
      }
 }
 
@@ -122,7 +126,8 @@ impl Node<ScriptView> {
             ElementNodeTypeId(_) => 1,
             TextNodeTypeId       => 3,
             CommentNodeTypeId    => 8,
-            DoctypeNodeTypeId    => 10
+            DocumentNodeTypeId   => 9,
+            DoctypeNodeTypeId    => 10,
         }
     }
 

src/components/script/dom/node.rs

@@ -88,10 +88,11 @@ pub struct Node<View> {
 /// The different types of nodes.
 #[deriving(Eq)]
 pub enum NodeTypeId {
-    DoctypeNodeTypeId,
-    CommentNodeTypeId,
     ElementNodeTypeId(ElementTypeId),
     TextNodeTypeId,
+    CommentNodeTypeId,
+    DocumentNodeTypeId,
+    DoctypeNodeTypeId,
 }
 
 //
@@ -371,7 +372,6 @@ impl<View> AbstractNode<View> {
         s += self.debug_str();
         debug!("%s", s);
 
-        // FIXME: this should have a pure version?
         for self.each_child() |kid| {
             kid.dump_indent(indent + 1u)
         }

src/components/script/html/hubbub_html_parser.rs

@@ -3,8 +3,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use dom::element::*;
-use dom::node::{AbstractNode, Comment, Doctype, Element, ElementNodeTypeId, Node, ScriptView};
-use dom::node::{Text};
+use dom::node::{AbstractNode, Comment, Doctype, Element, Text, Node, ScriptView};
+use dom::node::{ElementNodeTypeId, DocumentNodeTypeId};
 use html::cssparse::{InlineProvenance, StylesheetProvenance, UrlProvenance, spawn_css_parser};
 use newcss::stylesheet::Stylesheet;
 
@@ -40,7 +40,7 @@ type JSResult = ~[~[u8]];
 
 enum CSSMessage {
     CSSTaskNewFile(StylesheetProvenance),
-    CSSTaskExit   
+    CSSTaskExit
 }
 
 enum JSMessage {
@@ -231,13 +231,13 @@ pub fn parse_html(url: Url,
 
     let url2 = url.clone(), url3 = url.clone();
 
-    // Build the root node.
-    let root = ~HTMLHtmlElement { parent: Element::new(HTMLHtmlElementTypeId, ~"html") };
-    let root = unsafe { Node::as_abstract_node(root) };
-    debug!("created new node");
+    // Build the the document node.
+    let doc_node = ~Node::new(DocumentNodeTypeId);
+    let doc_node = unsafe { Node::as_abstract_node(doc_node) };
+    debug!("created document node");
     let mut parser = hubbub::Parser("UTF-8", false);
     debug!("created parser");
-    parser.set_document_node(root.to_hubbub_node());
+    parser.set_document_node(doc_node.to_hubbub_node());
     parser.enable_scripting(true);
 
     // Performs various actions necessary after appending has taken place. Currently, this
@@ -430,9 +430,8 @@ pub fn parse_html(url: Url,
     js_chan.send(JSTaskExit);
 
     HtmlParserResult {
-        root: root,
+        root: doc_node,
         style_port: stylesheet_port,
         js_port: js_result_port,
     }
 }
-

src/components/script/script_task.rs

@@ -336,6 +336,7 @@ impl ScriptContext {
         let document = Document(root_node, Some(window));
 
         // Tie the root into the document.
+        // FIXME(jj): Remove this code? The document is created with a root node.
         do root_node.with_mut_base |base| {
             base.add_to_doc(document)
         }

src/test/html/test_docelem.js

@@ -13,5 +13,3 @@ debug("elem.firstChild: " + elem.firstChild);
 debug("elem.firstChild.tagName: " + elem.firstChild.tagName);
 debug("elem.firstChild.nextSibling: " + elem.firstChild.nextSibling);
 debug("elem.firstChild.nextSibling.tagName: " + elem.firstChild.nextSibling.tagName);
-debug("elem.nextSibling: " + elem.nextSibling);
-debug("elem.nextSibling.tagName: " + elem.nextSibling.tagName);