Skip to content
This repository has been archived by the owner on Aug 4, 2022. It is now read-only.

Commit

Permalink
servo: Merge #16096 - Use Servo-specific pseudo elements for anonymou…
Browse files Browse the repository at this point in the history
…s box and text (from stshine:die-modify-style-die); r=emilio

<!-- Please describe your changes on the following line: -->

Use some fake pseudo elements to style servo-specific boxes in servo. Also, Since for nested inline elements non-inheritable properties are properly stored in the inline context of an inline fragment, so get
rid of them on the style using empty pseudo to do cascading.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #5625 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because refactoring

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 449758ef5dd399f7e1a5a9550dcdd614056cee9e
  • Loading branch information
stshine committed Apr 1, 2017
1 parent 364f833 commit 91c8e12
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 257 deletions.
107 changes: 46 additions & 61 deletions servo/components/layout/construct.rs

Large diffs are not rendered by default.

40 changes: 13 additions & 27 deletions servo/components/layout/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1167,20 +1167,15 @@ impl Fragment {
/// can be expensive to compute, so if possible use the `border_padding` field instead.
#[inline]
pub fn border_width(&self) -> LogicalMargin<Au> {
let style_border_width = match self.specific {
SpecificFragmentInfo::ScannedText(_) |
SpecificFragmentInfo::InlineBlock(_) => LogicalMargin::zero(self.style.writing_mode),
_ => self.style().logical_border_width(),
};

match self.inline_context {
None => style_border_width,
let style_border_width = self.style().logical_border_width();

// NOTE: We can have nodes with different writing mode inside
// the inline fragment context, so we need to overwrite the
// writing mode to compute the child logical sizes.
let writing_mode = self.style.writing_mode;
let context_border = match self.inline_context {
None => LogicalMargin::zero(writing_mode),
Some(ref inline_fragment_context) => {
// NOTE: We can have nodes with different writing mode inside
// the inline fragment context, so we need to overwrite the
// writing mode to compute the child logical sizes.
let writing_mode = self.style.writing_mode;

inline_fragment_context.nodes.iter().fold(style_border_width, |accumulator, node| {
let mut this_border_width =
node.style.border_width_for_writing_mode(writing_mode);
Expand All @@ -1193,7 +1188,8 @@ impl Fragment {
accumulator + this_border_width
})
}
}
};
style_border_width + context_border
}

/// Returns the border width in given direction if this fragment has property
Expand Down Expand Up @@ -1226,12 +1222,6 @@ impl Fragment {
self.margin.inline_end = Au(0);
return
}
SpecificFragmentInfo::InlineBlock(_) => {
// Inline-blocks do not take self margins into account but do account for margins
// from outer inline contexts.
self.margin.inline_start = Au(0);
self.margin.inline_end = Au(0);
}
_ => {
let margin = self.style().logical_margin();
self.margin.inline_start =
Expand Down Expand Up @@ -1259,8 +1249,8 @@ impl Fragment {
containing_block_inline_size).specified_or_zero()
};

self.margin.inline_start = self.margin.inline_start + this_inline_start_margin;
self.margin.inline_end = self.margin.inline_end + this_inline_end_margin;
self.margin.inline_start += this_inline_start_margin;
self.margin.inline_end += this_inline_end_margin;
}
}
}
Expand Down Expand Up @@ -1309,14 +1299,10 @@ impl Fragment {
};

// Compute padding from the fragment's style.
//
// This is zero in the case of `inline-block` because that padding is applied to the
// wrapped block, not the fragment.
let padding_from_style = match self.specific {
SpecificFragmentInfo::TableColumn(_) |
SpecificFragmentInfo::TableRow |
SpecificFragmentInfo::TableWrapper |
SpecificFragmentInfo::InlineBlock(_) => LogicalMargin::zero(self.style.writing_mode),
SpecificFragmentInfo::TableWrapper => LogicalMargin::zero(self.style.writing_mode),
_ => model::padding_from_style(self.style(), containing_block_inline_size, self.style().writing_mode),
};

Expand Down
11 changes: 1 addition & 10 deletions servo/components/script/layout_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -788,16 +788,7 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
self.node.type_id()
}

fn style_for_text_node(&self) -> Arc<ComputedValues> {
// Text nodes get a copy of the parent style. Inheriting all non-
// inherited properties into the text node is odd from a CSS
// perspective, but makes fragment construction easier (by making
// properties like vertical-align on fragments have values that
// match the parent element). This is an implementation detail of
// Servo layout that is not central to how fragment construction
// works, but would be difficult to change. (Text node style is
// also not visible to script.)
debug_assert!(self.is_text_node());
fn parent_style(&self) -> Arc<ComputedValues> {
let parent = self.node.parent_node().unwrap().as_element().unwrap();
let parent_data = parent.get_data().unwrap().borrow();
parent_data.styles().primary.values().clone()
Expand Down
9 changes: 6 additions & 3 deletions servo/components/script_layout_interface/wrapper_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo
/// it can be used to reach siblings and cousins. A simple immutable borrow
/// of the parent data is fine, since the bottom-up traversal will not process
/// the parent until all the children have been processed.
fn style_for_text_node(&self) -> Arc<ServoComputedValues>;
fn parent_style(&self) -> Arc<ServoComputedValues>;

#[inline]
fn is_element_or_elements_pseudo(&self) -> bool {
Expand Down Expand Up @@ -222,8 +222,10 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo
if let Some(el) = self.as_element() {
el.style(context)
} else {
// Text nodes are not styled during traversal,instead we simply
// return parent style here and do cascading during layout.
debug_assert!(self.is_text_node());
self.style_for_text_node()
self.parent_style()
}
}

Expand All @@ -232,7 +234,8 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo
el.selected_style()
} else {
debug_assert!(self.is_text_node());
self.style_for_text_node()
// TODO(stshine): What should the selected style be for text?
self.parent_style()
}
}

Expand Down
149 changes: 0 additions & 149 deletions servo/components/style/properties/properties.mako.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2398,96 +2398,6 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
style
}

/// Modifies the style for an anonymous flow so it resets all its non-inherited
/// style structs, and set their borders and outlines to zero.
///
/// Also, it gets a new display value, which is honored except when it's
/// `inline`.
#[cfg(feature = "servo")]
pub fn modify_style_for_anonymous_flow(style: &mut Arc<ComputedValues>,
new_display_value: longhands::display::computed_value::T) {
// The 'align-self' property needs some special treatment since
// its value depends on the 'align-items' value of its parent.
% if "align-items" in data.longhands_by_name:
use computed_values::align_self::T as align_self;
use computed_values::align_items::T as align_items;
let self_align =
match style.position.align_items {
align_items::stretch => align_self::stretch,
align_items::baseline => align_self::baseline,
align_items::flex_start => align_self::flex_start,
align_items::flex_end => align_self::flex_end,
align_items::center => align_self::center,
};
% endif
let inital_values = &*INITIAL_SERVO_VALUES;
let mut style = Arc::make_mut(style);
% for style_struct in data.active_style_structs():
% if not style_struct.inherited:
style.${style_struct.ident} = inital_values.clone_${style_struct.name_lower}();
% endif
% endfor
% if "align-items" in data.longhands_by_name:
let position = Arc::make_mut(&mut style.position);
position.align_self = self_align;
% endif
if new_display_value != longhands::display::computed_value::T::inline {
let new_box = Arc::make_mut(&mut style.box_);
new_box.display = new_display_value;
}
let border = Arc::make_mut(&mut style.border);
% for side in ["top", "right", "bottom", "left"]:
// Like calling to_computed_value, which wouldn't type check.
border.border_${side}_width = Au(0);
% endfor
// Initial value of outline-style is always none for anonymous box.
let outline = Arc::make_mut(&mut style.outline);
outline.outline_width = Au(0);
}

/// Alters the given style to accommodate replaced content. This is called in
/// flow construction. It handles cases like:
///
/// <div style="position: absolute">foo bar baz</div>
///
/// (in which `foo`, `bar`, and `baz` must not be absolutely-positioned) and
/// cases like `<sup>Foo</sup>` (in which the `vertical-align: top` style of
/// `sup` must not propagate down into `Foo`).
///
/// FIXME(#5625, pcwalton): It would probably be cleaner and faster to do this
/// in the cascade.
#[cfg(feature = "servo")]
#[inline]
pub fn modify_style_for_replaced_content(style: &mut Arc<ComputedValues>) {
// Reset `position` to handle cases like `<div style="position: absolute">foo bar baz</div>`.
if style.box_.display != longhands::display::computed_value::T::inline {
let mut style = Arc::make_mut(style);
Arc::make_mut(&mut style.box_).display = longhands::display::computed_value::T::inline;
Arc::make_mut(&mut style.box_).position =
longhands::position::computed_value::T::static_;
}

// Reset `vertical-align` to handle cases like `<sup>foo</sup>`.
if style.box_.vertical_align != longhands::vertical_align::computed_value::T::baseline {
let mut style = Arc::make_mut(style);
Arc::make_mut(&mut style.box_).vertical_align =
longhands::vertical_align::computed_value::T::baseline
}

// Reset margins.
if style.margin.margin_top != computed::LengthOrPercentageOrAuto::Length(Au(0)) ||
style.margin.margin_left != computed::LengthOrPercentageOrAuto::Length(Au(0)) ||
style.margin.margin_bottom != computed::LengthOrPercentageOrAuto::Length(Au(0)) ||
style.margin.margin_right != computed::LengthOrPercentageOrAuto::Length(Au(0)) {
let mut style = Arc::make_mut(style);
let margin = Arc::make_mut(&mut style.margin);
margin.margin_top = computed::LengthOrPercentageOrAuto::Length(Au(0));
margin.margin_left = computed::LengthOrPercentageOrAuto::Length(Au(0));
margin.margin_bottom = computed::LengthOrPercentageOrAuto::Length(Au(0));
margin.margin_right = computed::LengthOrPercentageOrAuto::Length(Au(0));
}
}

/// Adjusts borders as appropriate to account for a fragment's status as the
/// first or last fragment within the range of an element.
///
Expand Down Expand Up @@ -2544,65 +2454,6 @@ pub fn modify_border_style_for_inline_sides(style: &mut Arc<ComputedValues>,
}
}

/// Adjusts the `position` property as necessary for the outer fragment wrapper
/// of an inline-block.
#[cfg(feature = "servo")]
#[inline]
pub fn modify_style_for_outer_inline_block_fragment(style: &mut Arc<ComputedValues>) {
let mut style = Arc::make_mut(style);
let box_style = Arc::make_mut(&mut style.box_);
box_style.position = longhands::position::computed_value::T::static_
}

/// Adjusts the `position` and `padding` properties as necessary to account for
/// text.
///
/// Text is never directly relatively positioned; it's always contained within
/// an element that is itself relatively positioned.
#[cfg(feature = "servo")]
#[inline]
pub fn modify_style_for_text(style: &mut Arc<ComputedValues>) {
if style.box_.position == longhands::position::computed_value::T::relative {
// We leave the `position` property set to `relative` so that we'll still establish a
// containing block if needed. But we reset all position offsets to `auto`.
let mut style = Arc::make_mut(style);
let mut position = Arc::make_mut(&mut style.position);
position.top = computed::LengthOrPercentageOrAuto::Auto;
position.right = computed::LengthOrPercentageOrAuto::Auto;
position.bottom = computed::LengthOrPercentageOrAuto::Auto;
position.left = computed::LengthOrPercentageOrAuto::Auto;
}

if style.padding.padding_top != computed::LengthOrPercentage::Length(Au(0)) ||
style.padding.padding_right != computed::LengthOrPercentage::Length(Au(0)) ||
style.padding.padding_bottom != computed::LengthOrPercentage::Length(Au(0)) ||
style.padding.padding_left != computed::LengthOrPercentage::Length(Au(0)) {
let mut style = Arc::make_mut(style);
let mut padding = Arc::make_mut(&mut style.padding);
padding.padding_top = computed::LengthOrPercentage::Length(Au(0));
padding.padding_right = computed::LengthOrPercentage::Length(Au(0));
padding.padding_bottom = computed::LengthOrPercentage::Length(Au(0));
padding.padding_left = computed::LengthOrPercentage::Length(Au(0));
}

if style.effects.opacity != 1.0 {
let mut style = Arc::make_mut(style);
let mut effects = Arc::make_mut(&mut style.effects);
effects.opacity = 1.0;
}
}

/// Adjusts the `clip` property so that an inline absolute hypothetical fragment
/// doesn't clip its children.
#[cfg(feature = "servo")]
pub fn modify_style_for_inline_absolute_hypothetical_fragment(style: &mut Arc<ComputedValues>) {
if !style.get_effects().clip.is_auto() {
let mut style = Arc::make_mut(style);
let effects_style = Arc::make_mut(&mut style.effects);
effects_style.clip = Either::auto()
}
}

#[macro_export]
macro_rules! css_properties_accessors {
($macro_name: ident) => {
Expand Down
32 changes: 31 additions & 1 deletion servo/components/style/servo/selector_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,16 @@ pub enum PseudoElement {
Selection,
DetailsSummary,
DetailsContent,
ServoText,
ServoInputText,
ServoTableWrapper,
ServoAnonymousTableWrapper,
ServoAnonymousTable,
ServoAnonymousTableRow,
ServoAnonymousTableCell,
ServoAnonymousBlock,
ServoInlineBlockWrapper,
ServoInlineAbsolute,
}

impl ToCss for PseudoElement {
Expand All @@ -50,13 +53,16 @@ impl ToCss for PseudoElement {
Selection => "::selection",
DetailsSummary => "::-servo-details-summary",
DetailsContent => "::-servo-details-content",
ServoText => "::-servo-text",
ServoInputText => "::-servo-input-text",
ServoTableWrapper => "::-servo-table-wrapper",
ServoAnonymousTableWrapper => "::-servo-anonymous-table-wrapper",
ServoAnonymousTable => "::-servo-anonymous-table",
ServoAnonymousTableRow => "::-servo-anonymous-table-row",
ServoAnonymousTableCell => "::-servo-anonymous-table-cell",
ServoAnonymousBlock => "::-servo-anonymous-block",
ServoInlineBlockWrapper => "::-servo-inline-block-wrapper",
ServoInlineAbsolute => "::-servo-inline-absolute",
})
}
}
Expand All @@ -83,13 +89,16 @@ impl PseudoElement {
PseudoElement::Selection => PseudoElementCascadeType::Eager,
PseudoElement::DetailsSummary => PseudoElementCascadeType::Lazy,
PseudoElement::DetailsContent |
PseudoElement::ServoText |
PseudoElement::ServoInputText |
PseudoElement::ServoTableWrapper |
PseudoElement::ServoAnonymousTableWrapper |
PseudoElement::ServoAnonymousTable |
PseudoElement::ServoAnonymousTableRow |
PseudoElement::ServoAnonymousTableCell |
PseudoElement::ServoAnonymousBlock => PseudoElementCascadeType::Precomputed,
PseudoElement::ServoAnonymousBlock |
PseudoElement::ServoInlineBlockWrapper |
PseudoElement::ServoInlineAbsolute => PseudoElementCascadeType::Precomputed,
}
}
}
Expand Down Expand Up @@ -278,6 +287,12 @@ impl<'a> ::selectors::Parser for SelectorParser<'a> {
}
DetailsContent
},
"-servo-text" => {
if !self.in_user_agent_stylesheet() {
return Err(())
}
ServoText
},
"-servo-input-text" => {
if !self.in_user_agent_stylesheet() {
return Err(())
Expand Down Expand Up @@ -320,6 +335,18 @@ impl<'a> ::selectors::Parser for SelectorParser<'a> {
}
ServoAnonymousBlock
},
"-servo-inline-block-wrapper" => {
if !self.in_user_agent_stylesheet() {
return Err(())
}
ServoInlineBlockWrapper
},
"-servo-input-absolute" => {
if !self.in_user_agent_stylesheet() {
return Err(())
}
ServoInlineAbsolute
},
_ => return Err(())
};

Expand Down Expand Up @@ -352,13 +379,16 @@ impl SelectorImpl {
fun(PseudoElement::DetailsContent);
fun(PseudoElement::DetailsSummary);
fun(PseudoElement::Selection);
fun(PseudoElement::ServoText);
fun(PseudoElement::ServoInputText);
fun(PseudoElement::ServoTableWrapper);
fun(PseudoElement::ServoAnonymousTableWrapper);
fun(PseudoElement::ServoAnonymousTable);
fun(PseudoElement::ServoAnonymousTableRow);
fun(PseudoElement::ServoAnonymousTableCell);
fun(PseudoElement::ServoAnonymousBlock);
fun(PseudoElement::ServoInlineBlockWrapper);
fun(PseudoElement::ServoInlineAbsolute);
}

/// Returns the pseudo-class state flag for selector matching.
Expand Down
Loading

0 comments on commit 91c8e12

Please sign in to comment.