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

HTML List works #1 #1524

Closed
wants to merge 5 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

html list implementation

  • Loading branch information
aydin.kim authored and recrack committed Jan 27, 2014
commit 4ecbc9a64619d20cedd2c46cf21b546c92973120
@@ -120,6 +120,10 @@ impl BlockFlow {
self.float.is_some()
}

pub fn is_list(&self) -> bool {
self.base.listdata.is_some()
}

pub fn teardown(&mut self) {
for box_ in self.box_.iter() {
box_.teardown();
@@ -492,27 +496,41 @@ impl BlockFlow {
box_.position.set(position);
}

/// wrapper of build_display_list. For switching to html list.
pub fn build_display_list_wrapper<E:ExtraDisplayListData>(

This comment has been minimized.

Copy link
@metajack

metajack Jan 24, 2014

Contributor

It seems very odd that we'd need custom display list items for list markers. I don't think this is quite right. Can you explain the design decision here?

This comment has been minimized.

Copy link
@aydinkim

aydinkim Jan 27, 2014

Author

@metajack
There are 2 resons for making "build_display_list_wrapper"

  • To avoid code duplication
    The code below is used both block and float function.
    I think it is same code. Just purpose of refactoring.
// add box that starts block context
        for box_ in self.box_.iter() {
            box_.build_display_list(builder, dirty, offset, flow, list)
        }
  • To add display item which doesn't take effect on box calculation.
    To represent display list marker, we had needed a list item. I think that normal list item affects box dimension calculation. but as I checked Firefox& Chrome, list marker does not belong to its own box. It was just a display level item for showing and acts independently. @pcwalton said to me that I should not make any other flow, and use the block flow to make list. So I had added custom list maker at there for avoiding any influence to box calculation. I thought that only difference of block and list is just exist or not of marker for now. (in aspect of appearance)

If it is not proper to design of servo, please let me know.
I'll follow your advice.

&self,
builder: &DisplayListBuilder,
dirty: &Rect<Au>,
offset: Point2D<Au>,
flow: &Flow,
list: &RefCell<DisplayList<E>>) {
if self.is_list() {
for box_ in self.box_.iter() {
box_.add_marker_to_display_list(builder, dirty, flow, list)
}
}

// add box that starts block context
for box_ in self.box_.iter() {
box_.build_display_list(builder, dirty, offset, flow, list)
}
}

pub fn build_display_list_block<E:ExtraDisplayListData>(
&mut self,
builder: &DisplayListBuilder,
dirty: &Rect<Au>,
list: &RefCell<DisplayList<E>>)
-> bool {
if self.is_float() {
return self.build_display_list_float(builder, dirty, list);
}

let abs_rect = Rect(self.base.abs_position, self.base.position.size);
if !abs_rect.intersects(dirty) {
return true;
}

debug!("build_display_list_block: adding display element");

// add box that starts block context
for box_ in self.box_.iter() {
box_.build_display_list(builder, dirty, self.base.abs_position, (&*self) as &Flow, list)
}
self.build_display_list_wrapper(builder, dirty, self.base.abs_position, (&*self) as &Flow, list);

// TODO: handle any out-of-flow elements
let this_position = self.base.abs_position;

@@ -536,11 +554,8 @@ impl BlockFlow {
}

let offset = self.base.abs_position + self.float.get_ref().rel_pos;
// add box that starts block context
for box_ in self.box_.iter() {
box_.build_display_list(builder, dirty, offset, (&*self) as &Flow, list)
}


self.build_display_list_wrapper(builder, dirty, offset, (&*self) as &Flow, list);

// TODO: handle any out-of-flow elements

@@ -1210,6 +1210,55 @@ impl Box {
self.paint_borders_if_applicable(list, &absolute_box_bounds);
}

/// Adds list marker into displaylist. This refers listdata in FlowData.
/// FIXME(aydin.kim) : Is it better that we add ListBox and handle in build_display_list?
pub fn add_marker_to_display_list<E:ExtraDisplayListData>(
&self,
builder: &DisplayListBuilder,
dirty: &Rect<Au>,
flow: &Flow,
list: &RefCell<DisplayList<E>>) {
if self.style().Box.visibility != visibility::visible {
return;
}

let color = self.style().Color.color.to_gfx_color();
let font_style = self.font_style();
let fontgroup = (builder.ctx.font_ctx).get_resolved_font_for_style(&font_style);
let text = RefCell::new(listdata.sequence.to_str() + ".\u2009");
let run = ~fontgroup.borrow().with(|fg| fg.create_textrun(text.get(), text_decoration::none));
let text_range = Range::new(0, text.get().len());
let text_bounds = run.metrics_for_range(&text_range).bounding_box;
let em_size = text_bounds.size.height;
let line_height = self.calculate_line_height(em_size);
let text_offset = (line_height - em_size).scale_by(0.5);
let marker_offset = &Point2D(-run.min_width_for_range(&text_range), text_offset);

let marker_bound = Rect(flow::base(flow).abs_position, text_bounds.size).translate(marker_offset);
if !marker_bound.intersects(dirty) {
return;
}


// Create the text box.
list.with_mut(|list| {
let text_display_item = ~TextDisplayItem {
base: BaseDisplayItem {
bounds: marker_bound,
extra: ExtraDisplayListData::new(self),
},
text_run: Arc::new(run.clone()),
range: text_range,
text_color: color,
overline_color: color,
underline_color: color,
line_through_color: color,
flags: TextDisplayItemFlags::new(),
};

list.append_item(TextDisplayItemClass(text_display_item))
});
}
/// Returns the *minimum width* and *preferred width* of this box as defined by CSS 2.1.
pub fn minimum_and_preferred_widths(&self) -> (Au, Au) {
let guessed_width = self.guess_width();
@@ -27,20 +27,25 @@ use layout::box_::{UnscannedTextBox, UnscannedTextBoxInfo, InlineInfo, InlinePar
use layout::context::LayoutContext;
use layout::float_context::FloatType;
use layout::flow::{BaseFlow, Flow, LeafSet, MutableOwnedFlowUtils};
use layout::flow;
use layout::inline::InlineFlow;
use layout::text::TextRunScanner;
use layout::util::{LayoutDataAccess, OpaqueNode};
use layout::wrapper::{LayoutNode, PostorderNodeMutTraversal};
use layout::wrapper::{LayoutNode, PostorderNodeMutTraversal, LayoutElement};

use gfx::font_context::FontContext;
use script::dom::element::{HTMLIframeElementTypeId, HTMLImageElementTypeId};
use script::dom::element::{HTMLUListElementTypeId, HTMLOListElementTypeId, ElementTypeId};
use script::dom::node::{CommentNodeTypeId, DoctypeNodeTypeId, DocumentFragmentNodeTypeId};
use script::dom::node::{DocumentNodeTypeId, ElementNodeTypeId, TextNodeTypeId};
use style::computed_values::{display, position, float};
use style::TNode;
use style::TElement;

use std::cell::RefCell;
use std::util;
use std::num::Zero;
use servo_util::namespace;

/// The results of flow construction for a DOM node.
pub enum ConstructionResult {
@@ -295,6 +300,57 @@ impl<'fc> FlowConstructor<'fc> {
}
}

fn recursive_order_check(&mut self, flow: &mut ~Flow, mut list_cnt: int, is_ordered: bool, is_reversed: bool) -> int {
for child_flow in flow::child_iter(*flow) {
list_cnt = self.recursive_order_check(child_flow, list_cnt, is_ordered, is_reversed);
match child_flow.class() {
flow::BlockFlowClass => {
if child_flow.as_block().is_list() && !child_flow.as_block().base.listdata.get_mut_ref().sequenced() {
child_flow.as_block().base.listdata.get_mut_ref().from_list_tag(list_cnt, is_ordered);
if is_reversed { list_cnt -= 1; }
else { list_cnt += 1; }
}
},
_ => {},
}
}
list_cnt
}

fn parse_attr_for_list(&mut self, node: LayoutNode) -> (Option<ElementTypeId>, int, bool, bool) {
//FIXME(aydin.kim) : boost up the flow traverse. Sequence checking in time of parsing is needed?
let mut list_start_num: int = 1;
let mut is_ordered = false;
let mut is_reversed = false;
return match node.type_id() {
ElementNodeTypeId(typeid) if typeid == HTMLUListElementTypeId || typeid == HTMLOListElementTypeId=> {
match typeid {
HTMLUListElementTypeId => {
is_ordered = false;
},
HTMLOListElementTypeId => {
is_ordered = true;

match node.with_element(|e: &LayoutElement| {e.get_attr(&namespace::Null, "start")}) {
Some(s) => {
let conv_seq: Option<int> = from_str(s);
if conv_seq.is_some() { list_start_num = conv_seq.unwrap(); }
else { list_start_num = 1; }
},
None => {},
};
if node.with_element(|e: &LayoutElement| {e.get_attr(&namespace::Null, "reversed")}).is_some() {
is_reversed = true;
}
},
_ => {},
}
(Some(typeid), list_start_num, is_ordered, is_reversed)
},
_ =>(None, 1, false, false) //just a dummy value
}
}

/// Builds the children flows underneath a node with `display: block`. After this call,
/// other `BlockFlow`s or `InlineFlow`s will be populated underneath this node, depending on
/// whether {ib} splits needed to happen.
@@ -304,17 +360,37 @@ impl<'fc> FlowConstructor<'fc> {
// Gather up boxes for the inline flows we might need to create.
let mut opt_boxes_for_inline_flow = None;
let mut first_box = true;

let (list_type, list_start_num, is_ordered, is_reversed) = self.parse_attr_for_list(node);
let mut list_cnt = list_start_num;

// Children..
for kid in node.children() {
match kid.swap_out_construction_result() {
NoConstructionResult => {}
FlowConstructionResult(kid_flow) => {
FlowConstructionResult(mut kid_flow) => {
// Strip ignorable whitespace from the start of this flow per CSS 2.1 §
// 9.2.1.1.
if first_box {
strip_ignorable_whitespace_from_start(&mut opt_boxes_for_inline_flow);
first_box = false
}

//Processing html list tag for backward compatibility.
if list_type.is_some() {
if kid_flow.as_block().is_list() && !kid_flow.as_block().base.listdata.get_mut_ref().sequenced() {
kid_flow.as_block().base.listdata.get_mut_ref().from_list_tag(list_cnt, is_ordered);
if is_reversed { list_cnt -= 1; }
else { list_cnt += 1; }
}

if (kid.type_id() != ElementNodeTypeId(HTMLUListElementTypeId))
&& (kid.type_id() != ElementNodeTypeId(HTMLOListElementTypeId)) {
//FIXME(aydin.kim): Actually we don't need sequence and reverse check in the case of UL.
list_cnt = self.recursive_order_check(&mut kid_flow, list_cnt, is_ordered, is_reversed);
}
}

// Flush any inline boxes that we were gathering up. This allows us to handle
// {ib} splits.
debug!("flushing {} inline box(es) to flow A",
@@ -388,6 +464,18 @@ impl<'fc> FlowConstructor<'fc> {
node);
}

///Builds a flow for a node. In here, we can switch the flow type to make.
fn build_block_flow(&mut self, node: LayoutNode, float_value: float::T, is_fixed: bool) -> ~Flow {
let flow = match float_value {
float::none => self.build_flow_for_block(node, is_fixed),
_ => {
let float_type = FloatType::from_property(float_value);
self.build_flow_for_floated_block(node, float_type)
}
};
flow
}

/// Builds a flow for a node with `display: block`. This yields a `BlockFlow` with possibly
/// other `BlockFlow`s or `InlineFlow`s underneath it, depending on whether {ib} splits needed
/// to happen.
@@ -651,15 +739,10 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
let flow = self.build_flow_for_block(node, true);
node.set_flow_construction_result(FlowConstructionResult(flow))
}
(_, float::none, _) => {
let flow = self.build_flow_for_block(node, false);
node.set_flow_construction_result(FlowConstructionResult(flow))
}

// Floated flows contribute float flow construction results.
(_, float_value, _) => {
let float_type = FloatType::from_property(float_value);
let flow = self.build_flow_for_floated_block(node, float_type);
let flow = self.build_block_flow(node, float_value, false);
node.set_flow_construction_result(FlowConstructionResult(flow))
}
}
@@ -37,6 +37,7 @@ impl ExtraDisplayListData for Nothing {
/// support display-list-based hit testing and so forth.
pub struct DisplayListBuilder<'a> {
ctx: &'a LayoutContext,
numbers: &'a converter::Numbers,
}

//
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.