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

Pseudo element build flow and box #1496

Closed
Next

Create before_parent_node and before_node

Conflicts:

	src/components/main/layout/construct.rs
	src/components/main/layout/wrapper.rs
	src/components/script/dom/node.rs
	src/components/util/slot.rs
  • Loading branch information
parkjaeman authored and sammykim committed Feb 10, 2014
commit 66f325a9d609ca1acd3262db97db1e1a7c46a379
@@ -14,6 +14,8 @@ use style::ComputedValues;
/// Node mixin providing `style` method that returns a `NodeStyle`
pub trait StyledNode {
fn style<'a>(&'a self) -> &'a Arc<ComputedValues>;
fn before_style<'a>(&'a self) -> &'a Arc<ComputedValues>;
fn after_style<'a>(&'a self) -> &'a Arc<ComputedValues>;
fn restyle_damage(&self) -> RestyleDamage;
}

@@ -23,6 +25,16 @@ impl<'ln> StyledNode for ThreadSafeLayoutNode<'ln> {
self.get_css_select_results()
}

#[inline]
fn before_style<'a>(&'a self) -> &'a Arc<ComputedValues> {
self.get_before_css_select_results()
}

#[inline]
fn after_style<'a>(&'a self) -> &'a Arc<ComputedValues> {
self.get_before_css_select_results()
}

fn restyle_damage(&self) -> RestyleDamage {
self.get_restyle_damage()
}
@@ -12,6 +12,8 @@ use style::ComputedValues;

pub trait NodeUtil {
fn get_css_select_results<'a>(&'a self) -> &'a Arc<ComputedValues>;
fn get_before_css_select_results<'a>(&'a self) -> &'a Arc<ComputedValues>;
fn get_after_css_select_results<'a>(&'a self) -> &'a Arc<ComputedValues>;
fn have_css_select_results(self) -> bool;

fn get_restyle_damage(self) -> RestyleDamage;
@@ -35,6 +37,22 @@ impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
}
}

#[inline]
fn get_before_css_select_results<'a>(&'a self) -> &'a Arc<ComputedValues> {
unsafe {
let layout_data_ref = self.borrow_layout_data();
cast::transmute_region(layout_data_ref.get().as_ref().unwrap().data.before_style.as_ref().unwrap())
}
}

#[inline]
fn get_after_css_select_results<'a>(&'a self) -> &'a Arc<ComputedValues> {
unsafe {
let layout_data_ref = self.borrow_layout_data();
cast::transmute_region(layout_data_ref.get().as_ref().unwrap().data.after_style.as_ref().unwrap())
}
}

/// Does this node have a computed style yet?
fn have_css_select_results(self) -> bool {
let layout_data_ref = self.borrow_layout_data();
@@ -32,13 +32,23 @@ use layout::inline::InlineFlow;
use layout::text::TextRunScanner;
use layout::util::{LayoutDataAccess, OpaqueNode};
use layout::wrapper::{PostorderNodeMutTraversal, TLayoutNode, ThreadSafeLayoutNode};
use layout::util::PrivateLayoutData;
use layout::wrapper::{LayoutNode, PostorderNodeMutTraversal};
use layout::wrapper::{LayoutPseudoNode};
use layout::extra::LayoutAuxMethods;

use gfx::font_context::FontContext;
use script::dom::element::{HTMLIframeElementTypeId, HTMLImageElementTypeId};
use script::dom::element::Element;
use script::dom::node::{CommentNodeTypeId, DoctypeNodeTypeId, DocumentFragmentNodeTypeId};
use script::dom::node::{DocumentNodeTypeId, ElementNodeTypeId, ProcessingInstructionNodeTypeId};
use script::dom::node::{TextNodeTypeId};
use script::dom::node::AbstractNode;
use script::dom::text::Text;
use style::computed_values::{display, position, float, white_space};
use style::computed_values::content;
use style::TNode;
use style::{PseudoElement, Before, After};
use style::ComputedValues;

use extra::arc::Arc;
@@ -371,8 +381,6 @@ impl<'fc> FlowConstructor<'fc> {
}
}
}

// Add the boxes to the list we're maintaining.
opt_boxes_for_inline_flow.push_all_move(boxes)
}
ConstructionItemConstructionResult(WhitespaceConstructionItem(..)) => {
@@ -402,6 +410,7 @@ impl<'fc> FlowConstructor<'fc> {
fn build_flow_for_block(&mut self, node: ThreadSafeLayoutNode, is_fixed: bool) -> ~Flow {
let mut flow = ~BlockFlow::from_node(self, node, is_fixed) as ~Flow;
self.build_children_of_block_flow(&mut flow, node);

flow
}

@@ -617,6 +626,136 @@ impl<'fc> FlowConstructor<'fc> {
self.build_boxes_for_replaced_inline_content(node)
}
}

fn build_flow_or_boxes_for_pseudo_element(&mut self, node: LayoutNode, pseudo_element: PseudoElement) {
let p = node.parent_node().expect("Text node should have its parent");
let mut content = ~"";

// Create pseudo parent_node
let pseudo_parent_element = match pseudo_element {
Before => ~Element::new_layout_pseudo(~"before"),
After => ~Element::new_layout_pseudo(~"after"),
};
let pseudo_parent_abstract_node = unsafe { AbstractNode::from_layout_pseudo(pseudo_parent_element) };
let mut pseudo_parent_node = unsafe { p.new_with_this_lifetime(pseudo_parent_abstract_node.clone()) };

// Create layout data for pseudo parent node
let layout_data_ref = p.borrow_layout_data();
match *layout_data_ref.get() {
Some(ref ldw) => {

This comment has been minimized.

@jdm

jdm Feb 3, 2014

Member

get_ref instead of matching.

match ldw.chan {
Some(ref chan) => pseudo_parent_node.initialize_layout_data(chan.clone()),

This comment has been minimized.

@jdm

jdm Feb 3, 2014

Member

and_then instead of matching.

This comment has been minimized.

@hyunjunekim

hyunjunekim Feb 4, 2014

Contributor

pseudo_parent_ldw.chan.and_then(|ref chan| Some(pseudo_parent_node.initialize_layout_data(chan.clone())) );
When use and_then, I found move out error!

None => {}
}
if pseudo_element == Before {
insert_layout_data(&pseudo_parent_node, ~PrivateLayoutData::new_with_style(ldw.data.before_style.clone()));
match ldw.data.before_style {

This comment has been minimized.

@jdm

jdm Feb 3, 2014

Member

Either and_then or get_ref, depending on whether this should ever be None in a valid case.

Some(ref before_style) => content = FlowConstructor::get_content(&before_style.get().Box.content),
None() => {}
}
} else if pseudo_element == After {
insert_layout_data(&pseudo_parent_node, ~PrivateLayoutData::new_with_style(ldw.data.after_style.clone()));
match ldw.data.after_style {
Some(ref after_style) => content = FlowConstructor::get_content(&after_style.get().Box.content),
None() => {}
}
}
}
None => {}
}
let pseudo_parent_display = pseudo_parent_node.style().get().Box.display;

// Create pseudo node
let pseudo_text = ~Text::new_layout_pseudo(content);
let pseudo_abstract_node = unsafe { AbstractNode::from_layout_pseudo(pseudo_text) };
let mut pseudo_node = unsafe { node.new_with_this_lifetime(pseudo_abstract_node) };
let mut layout_data_ref = node.mutate_layout_data();
match *layout_data_ref.get() {
Some(ref mut ldw) => {

This comment has been minimized.

@jdm

jdm Feb 3, 2014

Member

get_ref instead of matching.

This comment has been minimized.

@hyunjunekim

hyunjunekim Feb 4, 2014

Contributor

we will use get_mut_ref instead of get_ref.

match ldw.chan {

This comment has been minimized.

@jdm

jdm Feb 3, 2014

Member

and_then instead of matching.

Some(ref chan) => {
pseudo_node.initialize_layout_data(chan.clone());
}
None => {}
}

insert_layout_data(&pseudo_node, ~PrivateLayoutData::new());

// Store pseudo_parent_node & pseudo_node in node
if pseudo_element == Before {
ldw.data.before_parent_node = Some(LayoutPseudoNode::from_layout_pseudo(pseudo_parent_abstract_node));
match ldw.data.before_parent_node {
Some(ref mut before_parent_node) => before_parent_node.set_display(pseudo_parent_display),

This comment has been minimized.

@jdm

jdm Feb 3, 2014

Member

This is a bit silly. Just have from_layout_pseudo take a display argument and set it in the constructor.

None => {}
}
ldw.data.before_node = Some(LayoutPseudoNode::from_layout_pseudo(pseudo_abstract_node));
} else if pseudo_element == After {
ldw.data.after_parent_node = Some(LayoutPseudoNode::from_layout_pseudo(pseudo_parent_abstract_node));
match ldw.data.after_parent_node {
Some(ref mut after_parent_node) => after_parent_node.set_display(pseudo_parent_display),
None => {}
}
ldw.data.after_node = Some(LayoutPseudoNode::from_layout_pseudo(pseudo_abstract_node));
}
}
None => {}
}

// Set relation between pseudo_node and pseudo_parent_node
pseudo_parent_node.set_first_child(&mut pseudo_node);
pseudo_node.set_parent_node(&mut pseudo_parent_node);

// Build block flow or inline boxes for pseudo_parent_node
if pseudo_element == Before {
if pseudo_parent_display == display::block {
let flow = self.build_flow_for_block(pseudo_parent_node, false);
pseudo_parent_node.set_flow_construction_result(FlowConstructionResult(flow));
} else if pseudo_parent_display == display::inline {
let construction_result = self.build_boxes_for_inline(pseudo_parent_node);
pseudo_parent_node.set_flow_construction_result(construction_result);
}
}

// Set relation with dom nodes
if pseudo_parent_display == display::inline {

This comment has been minimized.

@jdm

jdm Feb 3, 2014

Member

These two blocks are very similar. I think we can refactor this code to reduce duplication.

if pseudo_element == Before {
pseudo_node.set_next_sibling(&node);
} else if pseudo_element == After {
pseudo_parent_node.set_prev_sibling(&node);
match node.next_pseudo_sibling() {
Some(next_sibling) => {
pseudo_parent_node.set_next_sibling(&next_sibling);
}
None => {}
}
}
} else if pseudo_parent_display == display::block {
if pseudo_element == Before {
pseudo_parent_node.set_next_sibling(&p);
} else if pseudo_element == After {
pseudo_parent_node.set_prev_sibling(&p);
match p.next_pseudo_sibling() {
Some(next_sibling) => {
pseudo_parent_node.set_next_sibling(&next_sibling);
}
None => {}
}
}
}
}

fn get_content(content_list: &content::T) -> ~str{
match *content_list {
content::Content(ref value) => {
let iter = &mut value.clone().move_iter().peekable();
match iter.next() {
Some(content::StringContent(str)) => str,
_ => ~"",
}
}
_ => ~"",
}
}
}

impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
@@ -653,6 +792,12 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {

// Inline items contribute inline box construction results.
(display::inline, float::none, _) => {
if node.is_text() {
let pseudo_elements = node.necessary_pseudo_elements();
for pseudo_element in pseudo_elements.iter() {
self.build_flow_or_boxes_for_pseudo_element(node, *pseudo_element);
}
}
let construction_result = self.build_boxes_for_inline(node);
node.set_flow_construction_result(construction_result)
}
@@ -812,3 +957,10 @@ fn strip_ignorable_whitespace_from_end(opt_boxes: &mut Option<~[Box]>) {
}
}

fn insert_layout_data(node: &LayoutNode, new_layout_data: ~PrivateLayoutData) {
let mut layout_data = node.mutate_layout_data();
match *layout_data.get() {
Some(ref mut layout_data_wrapper) => layout_data_wrapper.data = new_layout_data,
None => {}

This comment has been minimized.

@jdm

jdm Feb 3, 2014

Member

Every other caller of mutate_layout_data calls fail! if they get None.

}
}
@@ -6,6 +6,7 @@ use layout::box_::Box;
use layout::construct::{ConstructionResult, NoConstructionResult};
use layout::parallel::DomParallelInfo;
use layout::wrapper::{LayoutNode, TLayoutNode, ThreadSafeLayoutNode};
use layout::wrapper::LayoutPseudoNode;

use extra::arc::Arc;
use script::dom::bindings::utils::Reflectable;
@@ -144,6 +145,14 @@ pub struct PrivateLayoutData {

after_style: Option<Arc<ComputedValues>>,

before_parent_node: Option<LayoutPseudoNode>,

before_node: Option<LayoutPseudoNode>,

after_parent_node: Option<LayoutPseudoNode>,

after_node: Option<LayoutPseudoNode>,

This comment has been minimized.

@jdm

jdm Feb 5, 2014

Member

After reading all the and_then calls when dealing with these, why don't we do:

struct PseudoElement {
  parent: LayoutPseudoNode,
  element: LayoutPseudoNode
}
...
before: Option<PseudoElement>,
after: Option<PseudoElement>

That seems like it will result in more readable and logical code.

This comment has been minimized.

@jdm

jdm Feb 5, 2014

Member

In fact, if we make this change I don't think we need (or want) a macro. We can just have a variable that's a reference to a PseudoElement (or method taking one).


/// Description of how to account for recent style changes.
restyle_damage: Option<int>,

@@ -165,6 +174,27 @@ impl PrivateLayoutData {
before_style: None,
style: None,
after_style: None,
before_parent_node: None,
before_node: None,
after_parent_node: None,
after_node: None,
restyle_damage: None,
flow_construction_result: NoConstructionResult,
}
}

pub fn new_with_style(style: Option<Arc<ComputedValues>>) -> PrivateLayoutData {
PrivateLayoutData {
applicable_declarations: ~[],
before_applicable_declarations: ~[],
after_applicable_declarations: ~[],
before_style: None,
style: style,
after_style: None,
before_parent_node: None,
before_node: None,
after_parent_node: None,
after_node: None,
restyle_damage: None,
flow_construction_result: NoConstructionResult,
parallel: DomParallelInfo::new(),
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.