Use empty pseudo to cascade only inheritable properties for text

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.
servo · Apr 1, 2017 · 951c050 · 951c050
1 parent 1677d47
commit 951c050
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 191 deletions.
diff --git a/components/layout/construct.rs b/components/layout/construct.rs
diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs
@@ -1173,14 +1173,13 @@ impl Fragment {
             _ => self.style().logical_border_width(),
         };
 
-        match self.inline_context {
-            None => style_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);
@@ -1193,7 +1192,8 @@ impl Fragment {
                     accumulator + this_border_width
                 })
             }
-        }
+        };
+        style_border_width + context_border
     }
 
     /// Returns the border width in given direction if this fragment has property
@@ -1216,32 +1216,13 @@ impl Fragment {
     /// Do not use this method if the inline direction margins are to be computed some other way
     /// (for example, via constraint solving for blocks).
     pub fn compute_inline_direction_margins(&mut self, containing_block_inline_size: Au) {
-        match self.specific {
-            SpecificFragmentInfo::Table |
-            SpecificFragmentInfo::TableCell |
-            SpecificFragmentInfo::TableRow |
-            SpecificFragmentInfo::TableColumn(_) |
-            SpecificFragmentInfo::InlineAbsoluteHypothetical(_) => {
-                self.margin.inline_start = Au(0);
-                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 =
-                    MaybeAuto::from_style(margin.inline_start,
-                                          containing_block_inline_size).specified_or_zero();
-                self.margin.inline_end =
-                    MaybeAuto::from_style(margin.inline_end,
-                                          containing_block_inline_size).specified_or_zero();
-            }
-        }
+        let margin = self.style().logical_margin();
+        self.margin.inline_start =
+            MaybeAuto::from_style(margin.inline_start,
+                                  containing_block_inline_size).specified_or_zero();
+        self.margin.inline_end =
+            MaybeAuto::from_style(margin.inline_end,
+                                  containing_block_inline_size).specified_or_zero();
 
         if let Some(ref inline_context) = self.inline_context {
             for node in &inline_context.nodes {

diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs
@@ -780,16 +780,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()

diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs
@@ -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 {
@@ -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()
         }
     }
 
@@ -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()
         }
     }
 

diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
@@ -2445,49 +2445,6 @@ pub fn modify_style_for_anonymous_flow(style: &mut Arc<ComputedValues>,
     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.
 ///
@@ -2544,44 +2501,6 @@ pub fn modify_border_style_for_inline_sides(style: &mut Arc<ComputedValues>,
     }
 }
 
-/// 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;
-    }
-}
-
 #[macro_export]
 macro_rules! css_properties_accessors {
     ($macro_name: ident) => {

diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs
@@ -32,6 +32,7 @@ pub enum PseudoElement {
     Selection,
     DetailsSummary,
     DetailsContent,
+    ServoText,
     ServoInputText,
     ServoTableWrapper,
     ServoAnonymousTableWrapper,
@@ -52,6 +53,7 @@ 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",
@@ -87,6 +89,7 @@ impl PseudoElement {
             PseudoElement::Selection => PseudoElementCascadeType::Eager,
             PseudoElement::DetailsSummary => PseudoElementCascadeType::Lazy,
             PseudoElement::DetailsContent |
+            PseudoElement::ServoText |
             PseudoElement::ServoInputText |
             PseudoElement::ServoTableWrapper |
             PseudoElement::ServoAnonymousTableWrapper |
@@ -284,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(())
@@ -370,6 +379,7 @@ impl SelectorImpl {
         fun(PseudoElement::DetailsContent);
         fun(PseudoElement::DetailsSummary);
         fun(PseudoElement::Selection);
+        fun(PseudoElement::ServoText);
         fun(PseudoElement::ServoInputText);
         fun(PseudoElement::ServoTableWrapper);
         fun(PseudoElement::ServoAnonymousTableWrapper);

diff --git a/components/style/stylist.rs b/components/style/stylist.rs
@@ -385,13 +385,14 @@ impl Stylist {
 
     /// Returns the style for an anonymous box of the given type.
     #[cfg(feature = "servo")]
-    pub fn style_for_anonymous_box(&self,
-                                   guards: &StylesheetGuards,
-                                   pseudo: &PseudoElement,
-                                   parent_style: &Arc<ComputedValues>)
-                                   -> Arc<ComputedValues> {
+    pub fn style_for_anonymous(&self,
+                               guards: &StylesheetGuards,
+                               pseudo: &PseudoElement,
+                               parent_style: &Arc<ComputedValues>)
+                               -> Arc<ComputedValues> {
         // For most (but not all) pseudo-elements, we inherit all values from the parent.
         let inherit_all = match *pseudo {
+            PseudoElement::ServoText |
             PseudoElement::ServoInputText => false,
             PseudoElement::ServoAnonymousBlock |
             PseudoElement::ServoAnonymousTable |

diff --git a/resources/servo.css b/resources/servo.css
@@ -168,6 +168,12 @@ svg > * {
   display: none;
 }
 
+/* style for text node. */
+*|*::-servo-text {
+    margin: 0;
+}
+
+/* style for text in input elements. */
 *|*::-servo-input-text {
     margin: 0;
 }