Skip to content

Commit

Permalink
Make NodeTypeId include CharacterData variant
Browse files Browse the repository at this point in the history
NodeTypeId is supposed to reflect the WebIDL inheritance hierarchy.
All of Text/ProcessingInstruction/Comment inherit from CharacterData,
which inherits from Node. There should be a CharacterDataTypeId value
that differentiates between those, instead.
  • Loading branch information
Jinwoo-Song committed Apr 29, 2015
1 parent 1e15014 commit f404853
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 78 deletions.
15 changes: 7 additions & 8 deletions components/layout/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use text::TextRunScanner;
use wrapper::{PostorderNodeMutTraversal, PseudoElementType, TLayoutNode, ThreadSafeLayoutNode};

use gfx::display_list::OpaqueNode;
use script::dom::characterdata::CharacterDataTypeId;
use script::dom::element::ElementTypeId;
use script::dom::htmlelement::HTMLElementTypeId;
use script::dom::htmlobjectelement::is_image_data;
Expand Down Expand Up @@ -789,7 +790,7 @@ impl<'a> FlowConstructor<'a> {
// fragment that needs to be generated for this inline node.
let mut fragments = LinkedList::new();
match (node.get_pseudo_element_type(), node.type_id()) {
(_, Some(NodeTypeId::Text)) => {
(_, Some(NodeTypeId::CharacterData(CharacterDataTypeId::Text))) => {
self.create_fragments_for_node_text_content(&mut fragments, node, &style)
}
(PseudoElementType::Normal, _) => {
Expand Down Expand Up @@ -1239,12 +1240,12 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
};
(munged_display, style.get_box().float, style.get_box().position)
}
Some(NodeTypeId::Text) => (display::T::inline, float::T::none, position::T::static_),
Some(NodeTypeId::Comment) |
Some(NodeTypeId::CharacterData(CharacterDataTypeId::Text)) => (display::T::inline, float::T::none, position::T::static_),
Some(NodeTypeId::CharacterData(CharacterDataTypeId::Comment)) |
Some(NodeTypeId::CharacterData(CharacterDataTypeId::ProcessingInstruction)) |
Some(NodeTypeId::DocumentType) |
Some(NodeTypeId::DocumentFragment) |
Some(NodeTypeId::Document) |
Some(NodeTypeId::ProcessingInstruction) => {
Some(NodeTypeId::Document) => {
(display::T::none, float::T::none, position::T::static_)
}
};
Expand Down Expand Up @@ -1382,9 +1383,7 @@ impl<'ln> NodeUtils for ThreadSafeLayoutNode<'ln> {
fn is_replaced_content(&self) -> bool {
match self.type_id() {
None |
Some(NodeTypeId::Text) |
Some(NodeTypeId::ProcessingInstruction) |
Some(NodeTypeId::Comment) |
Some(NodeTypeId::CharacterData(_)) |
Some(NodeTypeId::DocumentType) |
Some(NodeTypeId::DocumentFragment) |
Some(NodeTypeId::Document) |
Expand Down
3 changes: 2 additions & 1 deletion components/layout/css/matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use incremental::{self, RestyleDamage};
use opaque_node::OpaqueNodeMethods;
use wrapper::{LayoutElement, LayoutNode, TLayoutNode};

use script::dom::characterdata::CharacterDataTypeId;
use script::dom::node::NodeTypeId;
use script::layout_interface::Animation;
use selectors::bloom::BloomFilter;
Expand Down Expand Up @@ -676,7 +677,7 @@ impl<'ln> MatchMethods for LayoutNode<'ln> {
&mut None => panic!("no layout data"),
&mut Some(ref mut layout_data) => {
match self.type_id() {
Some(NodeTypeId::Text) => {
Some(NodeTypeId::CharacterData(CharacterDataTypeId::Text)) => {
// Text nodes get a copy of the parent style. This ensures
// that during fragment construction any non-inherited
// CSS properties (such as vertical-align) are correctly
Expand Down
4 changes: 2 additions & 2 deletions components/layout/wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use script::dom::bindings::codegen::InheritTypes::{HTMLIFrameElementCast, HTMLCa
use script::dom::bindings::codegen::InheritTypes::{HTMLImageElementCast, HTMLInputElementCast};
use script::dom::bindings::codegen::InheritTypes::{HTMLTextAreaElementCast, NodeCast, TextCast};
use script::dom::bindings::js::LayoutJS;
use script::dom::characterdata::LayoutCharacterDataHelpers;
use script::dom::characterdata::{CharacterDataTypeId, LayoutCharacterDataHelpers};
use script::dom::element::{Element, ElementTypeId};
use script::dom::element::{LayoutElementHelpers, RawLayoutElementHelpers};
use script::dom::htmlelement::HTMLElementTypeId;
Expand Down Expand Up @@ -1048,7 +1048,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
/// `empty_cells` per CSS 2.1 § 17.6.1.1.
pub fn is_content(&self) -> bool {
match self.type_id() {
Some(NodeTypeId::Element(..)) | Some(NodeTypeId::Text(..)) => true,
Some(NodeTypeId::Element(..)) | Some(NodeTypeId::CharacterData(CharacterDataTypeId::Text(..))) => true,
_ => false
}
}
Expand Down
17 changes: 12 additions & 5 deletions components/script/dom/characterdata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,16 @@ pub struct CharacterData {
impl CharacterDataDerived for EventTarget {
fn is_characterdata(&self) -> bool {
match *self.type_id() {
EventTargetTypeId::Node(NodeTypeId::Text) |
EventTargetTypeId::Node(NodeTypeId::Comment) |
EventTargetTypeId::Node(NodeTypeId::ProcessingInstruction) => true,
EventTargetTypeId::Node(NodeTypeId::CharacterData(_)) => true,
_ => false
}
}
}

impl CharacterData {
pub fn new_inherited(id: NodeTypeId, data: DOMString, document: JSRef<Document>) -> CharacterData {
pub fn new_inherited(id: CharacterDataTypeId, data: DOMString, document: JSRef<Document>) -> CharacterData {
CharacterData {
node: Node::new_inherited(id, document),
node: Node::new_inherited(NodeTypeId::CharacterData(id), document),
data: DOMRefCell::new(data),
}
}
Expand Down Expand Up @@ -153,6 +151,15 @@ impl<'a> CharacterDataMethods for JSRef<'a, CharacterData> {
}
}

/// The different types of CharacterData.
#[derive(Copy, Clone, PartialEq, Debug)]
#[jstraceable]
pub enum CharacterDataTypeId {
Comment,
Text,
ProcessingInstruction,
}

pub trait CharacterDataHelpers<'a> {
fn data(self) -> Ref<'a, DOMString>;
}
Expand Down
6 changes: 3 additions & 3 deletions components/script/dom/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use dom::bindings::codegen::InheritTypes::CommentDerived;
use dom::bindings::error::Fallible;
use dom::bindings::global::GlobalRef;
use dom::bindings::js::{JSRef, Rootable, Temporary};
use dom::characterdata::CharacterData;
use dom::characterdata::{CharacterData, CharacterDataTypeId};
use dom::document::Document;
use dom::eventtarget::{EventTarget, EventTargetTypeId};
use dom::node::{Node, NodeTypeId};
Expand All @@ -22,14 +22,14 @@ pub struct Comment {

impl CommentDerived for EventTarget {
fn is_comment(&self) -> bool {
*self.type_id() == EventTargetTypeId::Node(NodeTypeId::Comment)
*self.type_id() == EventTargetTypeId::Node(NodeTypeId::CharacterData(CharacterDataTypeId::Comment))
}
}

impl Comment {
fn new_inherited(text: DOMString, document: JSRef<Document>) -> Comment {
Comment {
characterdata: CharacterData::new_inherited(NodeTypeId::Comment, text, document)
characterdata: CharacterData::new_inherited(CharacterDataTypeId::Comment, text, document)
}
}

Expand Down
80 changes: 31 additions & 49 deletions components/script/dom/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use dom::bindings::js::Unrooted;
use dom::bindings::trace::JSTraceable;
use dom::bindings::trace::RootedVec;
use dom::bindings::utils::{Reflectable, reflect_dom_object};
use dom::characterdata::{CharacterData, CharacterDataHelpers};
use dom::characterdata::{CharacterData, CharacterDataHelpers, CharacterDataTypeId};
use dom::comment::Comment;
use dom::document::{Document, DocumentHelpers, IsHTMLDocument, DocumentSource};
use dom::documentfragment::DocumentFragment;
Expand Down Expand Up @@ -269,13 +269,11 @@ impl LayoutDataRef {
#[derive(Copy, Clone, PartialEq, Debug)]
#[jstraceable]
pub enum NodeTypeId {
CharacterData(CharacterDataTypeId),
DocumentType,
DocumentFragment,
Comment,
Document,
Element(ElementTypeId),
Text,
ProcessingInstruction,
}

trait PrivateNodeHelpers {
Expand Down Expand Up @@ -616,7 +614,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {

#[inline]
fn is_text(self) -> bool {
self.type_id == NodeTypeId::Text
self.type_id == NodeTypeId::CharacterData(CharacterDataTypeId::Text)
}

fn get_flag(self, flag: NodeFlags) -> bool {
Expand Down Expand Up @@ -1420,7 +1418,7 @@ impl Node {

// Step 4-5.
match node.type_id() {
NodeTypeId::Text => {
NodeTypeId::CharacterData(CharacterDataTypeId::Text) => {
if parent.is_document() {
return Err(HierarchyRequest);
}
Expand All @@ -1432,8 +1430,8 @@ impl Node {
},
NodeTypeId::DocumentFragment |
NodeTypeId::Element(_) |
NodeTypeId::ProcessingInstruction |
NodeTypeId::Comment => (),
NodeTypeId::CharacterData(CharacterDataTypeId::ProcessingInstruction) |
NodeTypeId::CharacterData(CharacterDataTypeId::Comment) => (),
NodeTypeId::Document => return Err(HierarchyRequest)
}

Expand Down Expand Up @@ -1507,9 +1505,7 @@ impl Node {
},
}
},
NodeTypeId::Text |
NodeTypeId::ProcessingInstruction |
NodeTypeId::Comment => (),
NodeTypeId::CharacterData(_) => (),
NodeTypeId::Document => unreachable!(),
}
},
Expand Down Expand Up @@ -1711,7 +1707,7 @@ impl Node {
let doc_fragment = DocumentFragment::new(document.r());
NodeCast::from_temporary(doc_fragment)
},
NodeTypeId::Comment => {
NodeTypeId::CharacterData(CharacterDataTypeId::Comment) => {
let cdata = CharacterDataCast::to_ref(node).unwrap();
let comment = Comment::new(cdata.Data(), document.r());
NodeCast::from_temporary(comment)
Expand Down Expand Up @@ -1739,12 +1735,12 @@ impl Node {
document.r(), ElementCreator::ScriptCreated);
NodeCast::from_temporary(element)
},
NodeTypeId::Text => {
NodeTypeId::CharacterData(CharacterDataTypeId::Text) => {
let cdata = CharacterDataCast::to_ref(node).unwrap();
let text = Text::new(cdata.Data(), document.r());
NodeCast::from_temporary(text)
},
NodeTypeId::ProcessingInstruction => {
NodeTypeId::CharacterData(CharacterDataTypeId::ProcessingInstruction) => {
let pi: JSRef<ProcessingInstruction> = ProcessingInstructionCast::to_ref(node).unwrap();
let pi = ProcessingInstruction::new(pi.Target(),
CharacterDataCast::from_ref(pi).Data(), document.r());
Expand Down Expand Up @@ -1819,13 +1815,13 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
// https://dom.spec.whatwg.org/#dom-node-nodetype
fn NodeType(self) -> u16 {
match self.type_id {
NodeTypeId::Element(_) => NodeConstants::ELEMENT_NODE,
NodeTypeId::Text => NodeConstants::TEXT_NODE,
NodeTypeId::ProcessingInstruction => NodeConstants::PROCESSING_INSTRUCTION_NODE,
NodeTypeId::Comment => NodeConstants::COMMENT_NODE,
NodeTypeId::Document => NodeConstants::DOCUMENT_NODE,
NodeTypeId::DocumentType => NodeConstants::DOCUMENT_TYPE_NODE,
NodeTypeId::DocumentFragment => NodeConstants::DOCUMENT_FRAGMENT_NODE,
NodeTypeId::CharacterData(CharacterDataTypeId::Text) => NodeConstants::TEXT_NODE,
NodeTypeId::CharacterData(CharacterDataTypeId::ProcessingInstruction) => NodeConstants::PROCESSING_INSTRUCTION_NODE,
NodeTypeId::CharacterData(CharacterDataTypeId::Comment) => NodeConstants::COMMENT_NODE,
NodeTypeId::Document => NodeConstants::DOCUMENT_NODE,
NodeTypeId::DocumentType => NodeConstants::DOCUMENT_TYPE_NODE,
NodeTypeId::DocumentFragment => NodeConstants::DOCUMENT_FRAGMENT_NODE,
NodeTypeId::Element(_) => NodeConstants::ELEMENT_NODE,
}
}

Expand All @@ -1836,13 +1832,13 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
let elem: JSRef<Element> = ElementCast::to_ref(self).unwrap();
elem.TagName()
}
NodeTypeId::Text => "#text".to_owned(),
NodeTypeId::ProcessingInstruction => {
NodeTypeId::CharacterData(CharacterDataTypeId::Text) => "#text".to_owned(),
NodeTypeId::CharacterData(CharacterDataTypeId::ProcessingInstruction) => {
let processing_instruction: JSRef<ProcessingInstruction> =
ProcessingInstructionCast::to_ref(self).unwrap();
processing_instruction.Target()
}
NodeTypeId::Comment => "#comment".to_owned(),
NodeTypeId::CharacterData(CharacterDataTypeId::Comment) => "#comment".to_owned(),
NodeTypeId::DocumentType => {
let doctype: JSRef<DocumentType> = DocumentTypeCast::to_ref(self).unwrap();
doctype.name().clone()
Expand All @@ -1861,10 +1857,8 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
// https://dom.spec.whatwg.org/#dom-node-ownerdocument
fn GetOwnerDocument(self) -> Option<Temporary<Document>> {
match self.type_id {
NodeTypeId::CharacterData(..) |
NodeTypeId::Element(..) |
NodeTypeId::Comment |
NodeTypeId::Text |
NodeTypeId::ProcessingInstruction |
NodeTypeId::DocumentType |
NodeTypeId::DocumentFragment => Some(self.owner_doc()),
NodeTypeId::Document => None
Expand Down Expand Up @@ -1924,9 +1918,7 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
// https://dom.spec.whatwg.org/#dom-node-nodevalue
fn GetNodeValue(self) -> Option<DOMString> {
match self.type_id {
NodeTypeId::Comment |
NodeTypeId::Text |
NodeTypeId::ProcessingInstruction => {
NodeTypeId::CharacterData(..) => {
let chardata: JSRef<CharacterData> = CharacterDataCast::to_ref(self).unwrap();
Some(chardata.Data())
}
Expand All @@ -1939,9 +1931,7 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
// https://dom.spec.whatwg.org/#dom-node-nodevalue
fn SetNodeValue(self, val: Option<DOMString>) {
match self.type_id {
NodeTypeId::Comment |
NodeTypeId::Text |
NodeTypeId::ProcessingInstruction => {
NodeTypeId::CharacterData(..) => {
self.SetTextContent(val)
}
_ => {}
Expand All @@ -1956,9 +1946,7 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
let content = Node::collect_text_contents(self.traverse_preorder());
Some(content)
}
NodeTypeId::Comment |
NodeTypeId::Text |
NodeTypeId::ProcessingInstruction => {
NodeTypeId::CharacterData(..) => {
let characterdata: JSRef<CharacterData> = CharacterDataCast::to_ref(self).unwrap();
Some(characterdata.Data())
}
Expand Down Expand Up @@ -1986,9 +1974,7 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
// Step 3.
Node::replace_all(node.r(), self);
}
NodeTypeId::Comment |
NodeTypeId::Text |
NodeTypeId::ProcessingInstruction => {
NodeTypeId::CharacterData(..) => {
let characterdata: JSRef<CharacterData> = CharacterDataCast::to_ref(self).unwrap();
characterdata.SetData(value);

Expand Down Expand Up @@ -2034,14 +2020,12 @@ impl<'a> NodeMethods for JSRef<'a, Node> {

// Step 4-5.
match node.type_id() {
NodeTypeId::Text if self.is_document() => return Err(HierarchyRequest),
NodeTypeId::CharacterData(CharacterDataTypeId::Text) if self.is_document() => return Err(HierarchyRequest),
NodeTypeId::DocumentType if !self.is_document() => return Err(HierarchyRequest),
NodeTypeId::DocumentFragment |
NodeTypeId::DocumentType |
NodeTypeId::Element(..) |
NodeTypeId::Text |
NodeTypeId::ProcessingInstruction |
NodeTypeId::Comment => (),
NodeTypeId::CharacterData(..) => (),
NodeTypeId::Document => return Err(HierarchyRequest)
}

Expand Down Expand Up @@ -2107,9 +2091,7 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
return Err(HierarchyRequest);
}
},
NodeTypeId::Text |
NodeTypeId::ProcessingInstruction |
NodeTypeId::Comment => (),
NodeTypeId::CharacterData(..) => (),
NodeTypeId::Document => unreachable!()
}
},
Expand Down Expand Up @@ -2269,9 +2251,9 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
// Step 3.
NodeTypeId::DocumentType if !is_equal_doctype(this, node) => return false,
NodeTypeId::Element(..) if !is_equal_element(this, node) => return false,
NodeTypeId::ProcessingInstruction if !is_equal_processinginstruction(this, node) => return false,
NodeTypeId::Text |
NodeTypeId::Comment if !is_equal_characterdata(this, node) => return false,
NodeTypeId::CharacterData(CharacterDataTypeId::ProcessingInstruction) if !is_equal_processinginstruction(this, node) => return false,
NodeTypeId::CharacterData(CharacterDataTypeId::Text) |
NodeTypeId::CharacterData(CharacterDataTypeId::Comment) if !is_equal_characterdata(this, node) => return false,
// Step 4.
NodeTypeId::Element(..) if !is_equal_element_attrs(this, node) => return false,
_ => ()
Expand Down
6 changes: 3 additions & 3 deletions components/script/dom/processinginstruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use dom::bindings::codegen::Bindings::ProcessingInstructionBinding;
use dom::bindings::codegen::Bindings::ProcessingInstructionBinding::ProcessingInstructionMethods;
use dom::bindings::codegen::InheritTypes::ProcessingInstructionDerived;
use dom::bindings::js::{JSRef, Temporary};
use dom::characterdata::CharacterData;
use dom::characterdata::{CharacterData, CharacterDataTypeId};
use dom::document::Document;
use dom::eventtarget::{EventTarget, EventTargetTypeId};
use dom::node::{Node, NodeTypeId};
Expand All @@ -21,14 +21,14 @@ pub struct ProcessingInstruction {

impl ProcessingInstructionDerived for EventTarget {
fn is_processinginstruction(&self) -> bool {
*self.type_id() == EventTargetTypeId::Node(NodeTypeId::ProcessingInstruction)
*self.type_id() == EventTargetTypeId::Node(NodeTypeId::CharacterData(CharacterDataTypeId::ProcessingInstruction))
}
}

impl ProcessingInstruction {
fn new_inherited(target: DOMString, data: DOMString, document: JSRef<Document>) -> ProcessingInstruction {
ProcessingInstruction {
characterdata: CharacterData::new_inherited(NodeTypeId::ProcessingInstruction, data, document),
characterdata: CharacterData::new_inherited(CharacterDataTypeId::ProcessingInstruction, data, document),
target: target
}
}
Expand Down

0 comments on commit f404853

Please sign in to comment.