diff --git a/components/style/animation.rs b/components/style/animation.rs index 8ce3077f2d2..aa115527d57 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -430,6 +430,7 @@ fn compute_style_for_animation_step(context: &SharedStyleContext, /* is_root = */ false, iter, previous_style, + previous_style, &context.default_computed_values, /* cascade_info = */ None, context.error_reporter.clone(), diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index dd9f5a19643..f7265efafd0 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -492,6 +492,7 @@ impl Expression { is_root_element: false, viewport_size: device.au_viewport_size(), inherited_style: default_values, + layout_parent_style: default_values, // This cloning business is kind of dumb.... It's because Context // insists on having an actual ComputedValues inside itself. style: default_values.clone(), diff --git a/components/style/matching.rs b/components/style/matching.rs index d710d558263..129b4884cbe 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -443,6 +443,33 @@ struct CascadeBooleans { } trait PrivateMatchMethods: TElement { + /// Returns the closest parent element that doesn't have a display: contents + /// style (and thus generates a box). + /// + /// This is needed to correctly handle blockification of flex and grid + /// items. + /// + /// Returns itself if the element has no parent. In practice this doesn't + /// happen because the root element is blockified per spec, but it could + /// happen if we decide to not blockify for roots of disconnected subtrees, + /// which is a kind of dubious beahavior. + fn layout_parent(&self) -> Self { + let mut current = self.clone(); + loop { + current = match current.parent_element() { + Some(el) => el, + None => return current, + }; + + let is_display_contents = + current.borrow_data().unwrap().styles().primary.values().is_display_contents(); + + if !is_display_contents { + return current; + } + } + } + fn cascade_internal(&self, context: &StyleContext, primary_style: &ComputedStyle, @@ -467,7 +494,7 @@ trait PrivateMatchMethods: TElement { let parent_data; let inherited_values_ = if pseudo_style.is_none() { parent_el = self.parent_element(); - parent_data = parent_el.as_ref().and_then(|x| x.borrow_data()); + parent_data = parent_el.as_ref().and_then(|e| e.borrow_data()); let parent_values = parent_data.as_ref().map(|d| { // Sometimes Gecko eagerly styles things without processing // pending restyles first. In general we'd like to avoid this, @@ -479,25 +506,44 @@ trait PrivateMatchMethods: TElement { d.styles().primary.values() }); - // Propagate the "can be fragmented" bit. It would be nice to - // encapsulate this better. - if let Some(ref p) = parent_values { - let can_be_fragmented = - p.is_multicol() || parent_el.unwrap().as_node().can_be_fragmented(); - unsafe { self.as_node().set_can_be_fragmented(can_be_fragmented); } - } - parent_values } else { + parent_el = Some(self.clone()); Some(primary_style.values()) }; + + let mut layout_parent_el = parent_el.clone(); + let layout_parent_data; + let mut layout_parent_style = inherited_values_; + if inherited_values_.map_or(false, |s| s.is_display_contents()) { + layout_parent_el = Some(layout_parent_el.unwrap().layout_parent()); + layout_parent_data = layout_parent_el.as_ref().unwrap().borrow_data().unwrap(); + layout_parent_style = Some(layout_parent_data.styles().primary.values()) + } + let inherited_values = inherited_values_.map(|x| &**x); + let layout_parent_style = layout_parent_style.map(|x| &**x); + + // Propagate the "can be fragmented" bit. It would be nice to + // encapsulate this better. + // + // Note that this is not needed for pseudos since we already do that + // when we resolve the non-pseudo style. + if pseudo_style.is_none() { + if let Some(ref p) = layout_parent_style { + let can_be_fragmented = + p.is_multicol() || + layout_parent_el.as_ref().unwrap().as_node().can_be_fragmented(); + unsafe { self.as_node().set_can_be_fragmented(can_be_fragmented); } + } + } // Invoke the cascade algorithm. let values = Arc::new(cascade(shared_context.viewport_size, rule_node, inherited_values, + layout_parent_style, &shared_context.default_computed_values, Some(&mut cascade_info), shared_context.error_reporter.clone(), diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index ba834f0141a..901779eba59 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -130,6 +130,11 @@ impl ComputedValues { }) } + #[inline] + pub fn is_display_contents(&self) -> bool { + self.get_box().clone_display() == longhands::display::computed_value::T::contents + } + % for style_struct in data.style_structs: #[inline] pub fn clone_${style_struct.name_lower}(&self) -> Arc { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index c3b60b7df0e..b844f73a685 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1424,6 +1424,11 @@ impl ComputedValues { /// Servo for obvious reasons. pub fn has_moz_binding(&self) -> bool { false } + /// Returns whether this style's display value is equal to contents. + /// + /// Since this isn't supported in Servo, this is always false for Servo. + pub fn is_display_contents(&self) -> bool { false } + /// Get the root font size. fn root_font_size(&self) -> Au { self.root_font_size } @@ -1761,14 +1766,16 @@ bitflags! { pub fn cascade(viewport_size: Size2D, rule_node: &StrongRuleNode, parent_style: Option<<&ComputedValues>, + layout_parent_style: Option<<&ComputedValues>, default_style: &Arc, cascade_info: Option<<&mut CascadeInfo>, error_reporter: StdBox, flags: CascadeFlags) -> ComputedValues { - let (is_root_element, inherited_style) = match parent_style { - Some(parent_style) => (false, parent_style), - None => (true, &**default_style), + debug_assert_eq!(parent_style.is_some(), layout_parent_style.is_some()); + let (is_root_element, inherited_style, layout_parent_style) = match parent_style { + Some(parent_style) => (false, parent_style, layout_parent_style.unwrap()), + None => (true, &**default_style, &**default_style), }; // Hold locks until after the apply_declarations() call returns. // Use filter_map because the root node has no style source. @@ -1793,6 +1800,7 @@ pub fn cascade(viewport_size: Size2D, is_root_element, iter_declarations, inherited_style, + layout_parent_style, default_style, cascade_info, error_reporter, @@ -1806,6 +1814,7 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D, is_root_element: bool, iter_declarations: F, inherited_style: &ComputedValues, + layout_parent_style: &ComputedValues, default_style: &Arc, mut cascade_info: Option<<&mut CascadeInfo>, mut error_reporter: StdBox, @@ -1861,6 +1870,7 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D, is_root_element: is_root_element, viewport_size: viewport_size, inherited_style: inherited_style, + layout_parent_style: layout_parent_style, style: starting_style, font_metrics_provider: font_metrics_provider, }; @@ -1943,10 +1953,7 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D, longhands::position::SpecifiedValue::absolute | longhands::position::SpecifiedValue::fixed); let floated = style.get_box().clone_float() != longhands::float::computed_value::T::none; - // FIXME(heycam): We should look past any display:contents ancestors to - // determine if we are a flex or grid item, but we don't have access to - // grandparent or higher style here. - let is_item = matches!(context.inherited_style.get_box().clone_display(), + let is_item = matches!(context.layout_parent_style.get_box().clone_display(), % if product == "gecko": computed_values::display::T::grid | computed_values::display::T::inline_grid | @@ -2023,7 +2030,7 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D, use computed_values::align_items::T as align_items; if style.get_position().clone_align_self() == computed_values::align_self::T::auto && !positioned { let self_align = - match context.inherited_style.get_position().clone_align_items() { + match context.layout_parent_style.get_position().clone_align_items() { align_items::stretch => align_self::stretch, align_items::baseline => align_self::baseline, align_items::flex_start => align_self::flex_start, diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs index d6c5f281946..b4d6a495cec 100644 --- a/components/style/servo/media_queries.rs +++ b/components/style/servo/media_queries.rs @@ -180,6 +180,7 @@ impl Range { is_root_element: false, viewport_size: viewport_size, inherited_style: default_values, + layout_parent_style: default_values, // This cloning business is kind of dumb.... It's because Context // insists on having an actual ComputedValues inside itself. style: default_values.clone(), diff --git a/components/style/stylist.rs b/components/style/stylist.rs index f5127d5b15b..5f7aef1666f 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -313,10 +313,25 @@ impl Stylist { flags.insert(INHERIT_ALL) } + // NOTE(emilio): We skip calculating the proper layout parent style + // here. + // + // It'd be fine to assert that this isn't called with a parent style + // where display contents is in effect, but in practice this is hard to + // do for stuff like :-moz-fieldset-content with a + //
. That is, the computed value of + // display for the fieldset is "contents", even though it's not the used + // value, so we don't need to adjust in a different way anyway. + // + // In practice, I don't think any anonymous content can be a direct + // descendant of a display: contents element where display: contents is + // the actual used value, and the computed value of it would need + // blockification. let computed = properties::cascade(self.device.au_viewport_size(), &rule_node, parent.map(|p| &**p), + parent.map(|p| &**p), default, None, Box::new(StdoutErrorReporter), @@ -389,10 +404,15 @@ impl Stylist { self.rule_tree.insert_ordered_rules( declarations.into_iter().map(|a| (a.source, a.level))); + // Read the comment on `precomputed_values_for_pseudo` to see why it's + // difficult to assert that display: contents nodes never arrive here + // (tl;dr: It doesn't apply for replaced elements and such, but the + // computed value is still "contents"). let computed = properties::cascade(self.device.au_viewport_size(), &rule_node, Some(&**parent), + Some(&**parent), default, None, Box::new(StdoutErrorReporter), diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 7a52ebd1d7f..3652af99484 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -43,6 +43,12 @@ pub struct Context<'a> { /// The style we're inheriting from. pub inherited_style: &'a ComputedValues, + /// The style of the layout parent node. This will almost always be + /// `inherited_style`, except when `display: contents` is at play, in which + /// case it's the style of the last ancestor with a `display` value that + /// isn't `contents`. + pub layout_parent_style: &'a ComputedValues, + /// Values access through this need to be in the properties "computed /// early": color, text-decoration, font-size, display, position, float, /// border-*-style, outline-style, font-family, writing-mode... diff --git a/components/style/viewport.rs b/components/style/viewport.rs index 884a52611ca..f15f4553c0e 100644 --- a/components/style/viewport.rs +++ b/components/style/viewport.rs @@ -689,6 +689,7 @@ impl MaybeNew for ViewportConstraints { is_root_element: false, viewport_size: initial_viewport, inherited_style: device.default_values(), + layout_parent_style: device.default_values(), style: device.default_values().clone(), font_metrics_provider: None, // TODO: Should have! }; diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 9791ff7837d..4cfba96bd4a 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -645,7 +645,10 @@ fn get_pseudo_style(element: GeckoElement, pseudo_tag: *mut nsIAtom, PseudoElementCascadeType::Lazy => { let d = doc_data.borrow_mut(); let base = styles.primary.values(); - d.stylist.lazily_compute_pseudo_element_style(&element, &pseudo, base, &d.default_computed_values()) + d.stylist.lazily_compute_pseudo_element_style(&element, + &pseudo, + base, + &d.default_computed_values()) .map(|s| s.values().clone()) }, } @@ -1348,6 +1351,7 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis // FIXME (bug 1303229): Use the actual viewport size here viewport_size: Size2D::new(Au(0), Au(0)), inherited_style: parent_style.unwrap_or(&init), + layout_parent_style: parent_style.unwrap_or(&init), style: (**style).clone(), font_metrics_provider: None, }; diff --git a/tests/unit/style/parsing/image.rs b/tests/unit/style/parsing/image.rs index cb4dfac001e..b68d61fb2ed 100644 --- a/tests/unit/style/parsing/image.rs +++ b/tests/unit/style/parsing/image.rs @@ -49,6 +49,7 @@ fn test_linear_gradient() { is_root_element: true, viewport_size: container, inherited_style: initial_style, + layout_parent_style: initial_style, style: initial_style.clone(), font_metrics_provider: None, };