Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Fix duplicate root html #510

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Prev

Implement the rough beginnings of the document node

  • Loading branch information
jjjjw committed Jun 9, 2013
commit 100f190995c384f3d0bde50289c35261591f199e
@@ -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")
}

@@ -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);
@@ -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")
};

@@ -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,
}
}

@@ -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)
}
@@ -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;

@@ -232,9 +232,7 @@ pub fn parse_html(url: Url,
let url2 = url.clone(), url3 = url.clone();

// Build the the document node.
// TODO(jj): Since this node is a formality, we should use a different element type
// for clarity.
let doc_node = ~HTMLHtmlElement { parent: Element::new(HTMLHtmlElementTypeId, ~"html") };
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);
@@ -431,22 +429,9 @@ pub fn parse_html(url: Url,
css_chan.send(CSSTaskExit);
js_chan.send(JSTaskExit);

let html_el = get_root_html_node(&doc_node);

HtmlParserResult {
root: html_el,
root: doc_node,
style_port: stylesheet_port,
js_port: js_result_port,
}
}

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.
// This is a Hubbub approach that looks odd when combined with our usage.
for doc_node.each_child |kid| {
if kid.is_element() && kid.type_id() == ElementNodeTypeId(HTMLHtmlElementTypeId) {
return kid;
}
}
fail!("The document tree has no root html element!")
}
@@ -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);
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.