diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2017-07-26 15:21:37 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-07-26 15:21:37 -0500 |
commit | 020188fdd77f0f0f2848e21eb9bcc28362d98506 (patch) | |
tree | 3d14537a207dabac8d35e06393034e1e95119db2 | |
parent | a15d13a6ec7b1f1ffeef86484ee483ec253ed0ba (diff) | |
parent | 048044f98b42a84cbd8fec5be7df8517b2a7e895 (diff) | |
download | servo-020188fdd77f0f0f2848e21eb9bcc28362d98506.tar.gz servo-020188fdd77f0f0f2848e21eb9bcc28362d98506.zip |
Auto merge of #17875 - bzbarsky:first-line-dual-inheritance, r=emilio
Add support for having two separate parent styles. Fixes gecko bug 1382806.
<!-- Please describe your changes on the following line: -->
This is needed for ::first-line support. See https://drafts.csswg.org/css-pseudo-4/#first-line-inheritance
This PR doesn't quite implement what the CSS spec draft says right now. It implements what Gecko does, which is what an earlier draft said.
---
<!-- 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 https://bugzilla.mozilla.org/show_bug.cgi?id=1382806
<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because servo doesn't support ::first-line yet
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17875)
<!-- Reviewable:end -->
-rw-r--r-- | components/style/animation.rs | 1 | ||||
-rw-r--r-- | components/style/properties/helpers/animated_properties.mako.rs | 4 | ||||
-rw-r--r-- | components/style/properties/longhand/color.mako.rs | 2 | ||||
-rw-r--r-- | components/style/properties/longhand/font.mako.rs | 24 | ||||
-rw-r--r-- | components/style/properties/longhand/inherited_text.mako.rs | 8 | ||||
-rw-r--r-- | components/style/properties/properties.mako.rs | 87 | ||||
-rw-r--r-- | components/style/style_adjuster.rs | 2 | ||||
-rw-r--r-- | components/style/style_resolver.rs | 1 | ||||
-rw-r--r-- | components/style/stylist.rs | 4 | ||||
-rw-r--r-- | components/style/values/computed/mod.rs | 7 | ||||
-rw-r--r-- | components/style/values/specified/length.rs | 2 |
11 files changed, 109 insertions, 33 deletions
diff --git a/components/style/animation.rs b/components/style/animation.rs index d90467c7eea..69b94edb580 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -499,6 +499,7 @@ fn compute_style_for_animation_step(context: &SharedStyleContext, iter, Some(previous_style), Some(previous_style), + Some(previous_style), /* cascade_info = */ None, /* visited_style = */ None, font_metrics_provider, diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 4e3a79f5e77..17c10809afc 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -605,8 +605,8 @@ impl AnimationValue { CSSWideKeyword::Unset | % endif CSSWideKeyword::Inherit => { - let inherit_struct = context.inherited_style() - .get_${prop.style_struct.name_lower}(); + let inherit_struct = context.builder + .get_parent_${prop.style_struct.name_lower}(); inherit_struct.clone_${prop.ident}() }, }; diff --git a/components/style/properties/longhand/color.mako.rs b/components/style/properties/longhand/color.mako.rs index d1fb5429261..9ac3bbfcfbc 100644 --- a/components/style/properties/longhand/color.mako.rs +++ b/components/style/properties/longhand/color.mako.rs @@ -22,7 +22,7 @@ #[inline] fn to_computed_value(&self, context: &Context) -> computed_value::T { self.0.to_computed_value(context) - .to_rgba(context.inherited_style().get_color().clone_color()) + .to_rgba(context.builder.get_parent_color().clone_color()) } #[inline] diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs index ca8d227537c..f6b78a91749 100644 --- a/components/style/properties/longhand/font.mako.rs +++ b/components/style/properties/longhand/font.mako.rs @@ -548,9 +548,9 @@ ${helpers.single_keyword_system("font-variant-caps", SpecifiedValue::Normal => computed_value::T::normal(), SpecifiedValue::Bold => computed_value::T::bold(), SpecifiedValue::Bolder => - context.inherited_style().get_font().clone_font_weight().bolder(), + context.builder.get_parent_font().clone_font_weight().bolder(), SpecifiedValue::Lighter => - context.inherited_style().get_font().clone_font_weight().lighter(), + context.builder.get_parent_font().clone_font_weight().lighter(), SpecifiedValue::System(_) => { <%self:nongecko_unreachable> context.cached_system_font.as_ref().unwrap().font_weight.clone() @@ -940,7 +940,7 @@ ${helpers.single_keyword_system("font-variant-caps", // recomputed from the base size for the keyword and the relative size. // // See bug 1355707 - if let Some((kw, fraction)) = context.builder.inherited_style().font_computation_data.font_size_keyword { + if let Some((kw, fraction)) = context.builder.inherited_font_computation_data().font_size_keyword { context.builder.font_size_keyword = Some((kw, fraction * ratio)); } else { context.builder.font_size_keyword = None; @@ -956,9 +956,9 @@ ${helpers.single_keyword_system("font-variant-caps", // if the language or generic changed, we need to recalculate // the font size from the stored font-size origin information. if context.builder.get_font().gecko().mLanguage.raw::<nsIAtom>() != - context.builder.inherited_style().get_font().gecko().mLanguage.raw::<nsIAtom>() || + context.builder.get_parent_font().gecko().mLanguage.raw::<nsIAtom>() || context.builder.get_font().gecko().mGenericID != - context.builder.inherited_style().get_font().gecko().mGenericID { + context.builder.get_parent_font().gecko().mGenericID { if let Some((kw, ratio)) = context.builder.font_size_keyword { computed = kw.to_computed_value(context).scale_by(ratio); } @@ -968,8 +968,7 @@ ${helpers.single_keyword_system("font-variant-caps", let device = context.builder.device; let mut font = context.builder.take_font(); let parent_unconstrained = { - let parent_style = context.builder.inherited_style(); - let parent_font = parent_style.get_font(); + let parent_font = context.builder.get_parent_font(); font.apply_font_size(computed, parent_font, device) }; context.builder.put_font(font); @@ -997,9 +996,8 @@ ${helpers.single_keyword_system("font-variant-caps", let device = context.builder.device; let mut font = context.builder.take_font(); let used_kw = { - let parent_style = context.builder.inherited_style(); - let parent_font = parent_style.get_font(); - parent_kw = parent_style.font_computation_data.font_size_keyword; + let parent_font = context.builder.get_parent_font(); + parent_kw = context.builder.inherited_font_computation_data().font_size_keyword; font.inherit_font_size_from(parent_font, kw_inherited_size, device) }; @@ -2279,8 +2277,8 @@ https://drafts.csswg.org/css-fonts-4/#low-level-font-variation-settings-control- let int = match *self { SpecifiedValue::Auto => { - let parent = cx.inherited_style().get_font().clone__moz_script_level() as i32; - let display = cx.inherited_style().get_font().clone__moz_math_display(); + let parent = cx.builder.get_parent_font().clone__moz_script_level() as i32; + let display = cx.builder.get_parent_font().clone__moz_math_display(); if display == DisplayValue::inline { parent + 1 } else { @@ -2288,7 +2286,7 @@ https://drafts.csswg.org/css-fonts-4/#low-level-font-variation-settings-control- } } SpecifiedValue::Relative(rel) => { - let parent = cx.inherited_style().get_font().clone__moz_script_level(); + let parent = cx.builder.get_parent_font().clone__moz_script_level(); parent as i32 + rel } SpecifiedValue::Absolute(abs) => abs, diff --git a/components/style/properties/longhand/inherited_text.mako.rs b/components/style/properties/longhand/inherited_text.mako.rs index 80f0624d55f..50df62db72a 100644 --- a/components/style/properties/longhand/inherited_text.mako.rs +++ b/components/style/properties/longhand/inherited_text.mako.rs @@ -230,8 +230,8 @@ ${helpers.single_keyword("text-align-last", if context.is_root_element { return get_initial_value(); } - let parent = context.inherited_style().get_inheritedtext().clone_text_align(); - let ltr = context.inherited_style().writing_mode.is_bidi_ltr(); + let parent = context.builder.get_parent_inheritedtext().clone_text_align(); + let ltr = context.builder.inherited_writing_mode().is_bidi_ltr(); match (parent, ltr) { (computed_value::T::start, true) => computed_value::T::left, (computed_value::T::start, false) => computed_value::T::right, @@ -241,7 +241,7 @@ ${helpers.single_keyword("text-align-last", } } SpecifiedValue::MozCenterOrInherit => { - let parent = context.inherited_style().get_inheritedtext().clone_text_align(); + let parent = context.builder.get_parent_inheritedtext().clone_text_align(); if parent == computed_value::T::start { computed_value::T::center } else { @@ -340,7 +340,7 @@ ${helpers.predefined_type("word-spacing", overline: None, line_through: None, }, - _ => context.inherited_style().get_inheritedtext().clone__servo_text_decorations_in_effect() + _ => context.builder.get_parent_inheritedtext().clone__servo_text_decorations_in_effect() }; result.underline = maybe(context.style().get_text().has_underline() diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 712b2c644ac..41e9b3e00ac 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -14,6 +14,7 @@ use servo_arc::{Arc, UniqueArc}; use std::borrow::Cow; use std::collections::HashSet; use std::{fmt, mem, ops}; +#[cfg(feature = "gecko")] use std::ptr; use app_units::Au; #[cfg(feature = "servo")] use cssparser::RGBA; @@ -2470,6 +2471,11 @@ pub struct StyleBuilder<'a> { /// `parent_style.unwrap_or(device.default_computed_values())`. inherited_style: &'a ComputedValues, + /// The style we're inheriting from for properties that don't inherit from + /// ::first-line. This is the same as inherited_style, unless + /// inherited_style is a ::first-line style. + inherited_style_ignoring_first_line: &'a ComputedValues, + /// The style we're getting reset structs from. reset_style: &'a ComputedValues, @@ -2507,6 +2513,7 @@ impl<'a> StyleBuilder<'a> { fn new( device: &'a Device, parent_style: Option<<&'a ComputedValues>, + parent_style_ignoring_first_line: Option<<&'a ComputedValues>, pseudo: Option<<&'a PseudoElement>, cascade_flags: CascadeFlags, rules: Option<StrongRuleNode>, @@ -2516,8 +2523,19 @@ impl<'a> StyleBuilder<'a> { flags: ComputedValueFlags, visited_style: Option<Arc<ComputedValues>>, ) -> Self { + debug_assert_eq!(parent_style.is_some(), parent_style_ignoring_first_line.is_some()); + #[cfg(feature = "gecko")] + debug_assert!(parent_style.is_none() || + ptr::eq(parent_style.unwrap(), + parent_style_ignoring_first_line.unwrap()) || + parent_style.unwrap().pseudo() == Some(PseudoElement::FirstLine)); let reset_style = device.default_computed_values(); let inherited_style = parent_style.unwrap_or(reset_style); + let inherited_style_ignoring_first_line = parent_style_ignoring_first_line.unwrap_or(reset_style); + // FIXME(bz): INHERIT_ALL seems like a fundamentally broken idea. I'm + // 99% sure it should give incorrect behavior for table anonymous box + // backgrounds, for example. This code doesn't attempt to make it play + // nice with inherited_style_ignoring_first_line. let reset_style = if cascade_flags.contains(INHERIT_ALL) { inherited_style } else { @@ -2528,6 +2546,7 @@ impl<'a> StyleBuilder<'a> { device, parent_style, inherited_style, + inherited_style_ignoring_first_line, reset_style, pseudo, rules, @@ -2556,10 +2575,15 @@ impl<'a> StyleBuilder<'a> { ) -> Self { let reset_style = device.default_computed_values(); let inherited_style = parent_style.unwrap_or(reset_style); + #[cfg(feature = "gecko")] + debug_assert!(parent_style.is_none() || + parent_style.unwrap().pseudo() != Some(PseudoElement::FirstLine)); StyleBuilder { device, parent_style, inherited_style, + // None of our callers pass in ::first-line parent styles. + inherited_style_ignoring_first_line: inherited_style, reset_style, pseudo, rules: None, // FIXME(emilio): Dubious... @@ -2581,8 +2605,13 @@ impl<'a> StyleBuilder<'a> { /// Inherit `${property.ident}` from our parent style. #[allow(non_snake_case)] pub fn inherit_${property.ident}(&mut self) { + % if property.style_struct.inherited: let inherited_struct = self.inherited_style.get_${property.style_struct.name_lower}(); + % else: + let inherited_struct = + self.inherited_style_ignoring_first_line.get_${property.style_struct.name_lower}(); + % endif self.${property.style_struct.ident}.mutate() .copy_${property.ident}_from( inherited_struct, @@ -2639,6 +2668,7 @@ impl<'a> StyleBuilder<'a> { Self::new( device, Some(parent), + Some(parent), pseudo, CascadeFlags::empty(), /* rules = */ None, @@ -2655,11 +2685,6 @@ impl<'a> StyleBuilder<'a> { self.visited_style.is_some() } - /// Returns the style we're inheriting from. - pub fn inherited_style(&self) -> &'a ComputedValues { - self.inherited_style - } - /// Returns the style we're getting reset properties from. pub fn default_style(&self) -> &'a ComputedValues { self.reset_style @@ -2752,6 +2777,42 @@ impl<'a> StyleBuilder<'a> { fn custom_properties(&self) -> Option<Arc<::custom_properties::CustomPropertiesMap>> { self.custom_properties.clone() } + + /// Access to various information about our inherited styles. We don't + /// expose an inherited ComputedValues directly, because in the + /// ::first-line case some of the inherited information needs to come from + /// one ComputedValues instance and some from a different one. + + /// Inherited font bits. + pub fn inherited_font_computation_data(&self) -> &FontComputationData { + &self.inherited_style.font_computation_data + } + + /// Inherited writing-mode. + pub fn inherited_writing_mode(&self) -> &WritingMode { + &self.inherited_style.writing_mode + } + + /// Inherited style flags. + pub fn inherited_flags(&self) -> &ComputedValueFlags { + &self.inherited_style.flags + } + + /// And access to inherited style structs. + % for style_struct in data.active_style_structs(): + /// Gets our inherited `${style_struct.name}`. We don't name these + /// accessors `inherited_${style_struct.name_lower}` because we already + /// have things like "box" vs "inherited_box" as struct names. Do the + /// next-best thing and call them `parent_${style_struct.name_lower}` + /// instead. + pub fn get_parent_${style_struct.name_lower}(&self) -> &style_structs::${style_struct.name} { + % if style_struct.inherited: + self.inherited_style.get_${style_struct.name_lower}() + % else: + self.inherited_style_ignoring_first_line.get_${style_struct.name_lower}() + % endif + } + % endfor } #[cfg(feature = "servo")] @@ -2867,6 +2928,7 @@ pub fn cascade( rule_node: &StrongRuleNode, guards: &StylesheetGuards, parent_style: Option<<&ComputedValues>, + parent_style_ignoring_first_line: Option<<&ComputedValues>, layout_parent_style: Option<<&ComputedValues>, visited_style: Option<Arc<ComputedValues>>, cascade_info: Option<<&mut CascadeInfo>, @@ -2874,6 +2936,12 @@ pub fn cascade( flags: CascadeFlags, quirks_mode: QuirksMode ) -> Arc<ComputedValues> { + debug_assert_eq!(parent_style.is_some(), parent_style_ignoring_first_line.is_some()); + #[cfg(feature = "gecko")] + debug_assert!(parent_style.is_none() || + ptr::eq(parent_style.unwrap(), + parent_style_ignoring_first_line.unwrap()) || + parent_style.unwrap().pseudo() == Some(PseudoElement::FirstLine)); let iter_declarations = || { rule_node.self_and_ancestors().flat_map(|node| { let cascade_level = node.cascade_level(); @@ -2919,6 +2987,7 @@ pub fn cascade( rule_node, iter_declarations, parent_style, + parent_style_ignoring_first_line, layout_parent_style, visited_style, cascade_info, @@ -2937,6 +3006,7 @@ pub fn apply_declarations<'a, F, I>( rules: &StrongRuleNode, iter_declarations: F, parent_style: Option<<&ComputedValues>, + parent_style_ignoring_first_line: Option<<&ComputedValues>, layout_parent_style: Option<<&ComputedValues>, visited_style: Option<Arc<ComputedValues>>, mut cascade_info: Option<<&mut CascadeInfo>, @@ -2949,6 +3019,12 @@ where I: Iterator<Item = (&'a PropertyDeclaration, CascadeLevel)>, { debug_assert!(layout_parent_style.is_none() || parent_style.is_some()); + debug_assert_eq!(parent_style.is_some(), parent_style_ignoring_first_line.is_some()); + #[cfg(feature = "gecko")] + debug_assert!(parent_style.is_none() || + ptr::eq(parent_style.unwrap(), + parent_style_ignoring_first_line.unwrap()) || + parent_style.unwrap().pseudo() == Some(PseudoElement::FirstLine)); let (inherited_style, layout_parent_style) = match parent_style { Some(parent_style) => { (parent_style, @@ -2983,6 +3059,7 @@ where builder: StyleBuilder::new( device, parent_style, + parent_style_ignoring_first_line, pseudo, flags, Some(rules.clone()), diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 8222634317e..022e5e54264 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -446,7 +446,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { let relevant_link_visited = if flags.contains(IS_LINK) { flags.contains(IS_VISITED_LINK) } else { - self.style.inherited_style().flags.contains(IS_RELEVANT_LINK_VISITED) + self.style.inherited_flags().contains(IS_RELEVANT_LINK_VISITED) }; if relevant_link_visited { diff --git a/components/style/style_resolver.rs b/components/style/style_resolver.rs index 71be0609a6e..cf512c4fbdd 100644 --- a/components/style/style_resolver.rs +++ b/components/style/style_resolver.rs @@ -479,6 +479,7 @@ where rules.unwrap_or(self.context.shared.stylist.rule_tree().root()), &self.context.shared.guards, parent_style, + parent_style, layout_parent_style, style_if_visited, Some(&mut cascade_info), diff --git a/components/style/stylist.rs b/components/style/stylist.rs index e5e61752be4..29633ae8509 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -637,6 +637,7 @@ impl Stylist { guards, parent, parent, + parent, None, None, font_metrics, @@ -753,6 +754,7 @@ impl Stylist { guards, Some(inherited_style), Some(inherited_style), + Some(inherited_style), None, None, font_metrics, @@ -778,6 +780,7 @@ impl Stylist { guards, Some(parent_style), Some(parent_style), + Some(parent_style), visited_values, None, font_metrics, @@ -1342,6 +1345,7 @@ impl Stylist { guards, Some(parent_style), Some(parent_style), + Some(parent_style), None, None, &metrics, diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 6a82b8cf8bb..8e01eda014c 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -119,11 +119,6 @@ impl<'a> Context<'a> { self.builder.device.au_viewport_size() } - /// The style we're inheriting from. - pub fn inherited_style(&self) -> &ComputedValues { - self.builder.inherited_style() - } - /// The default computed style we're getting our reset style from. pub fn default_style(&self) -> &ComputedValues { self.builder.default_style() @@ -411,7 +406,7 @@ impl ToComputedValue for specified::JustifyItems { // If the inherited value of `justify-items` includes the `legacy` keyword, `auto` computes // to the inherited value. if self.0 == align::ALIGN_AUTO { - let inherited = context.inherited_style().get_position().clone_justify_items(); + let inherited = context.builder.get_parent_position().clone_justify_items(); if inherited.0.contains(align::ALIGN_LEGACY) { return inherited } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index c3591bdf484..bf4b83b5293 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -93,7 +93,7 @@ impl FontBaseSize { match *self { FontBaseSize::Custom(size) => size, FontBaseSize::CurrentStyle => context.style().get_font().clone_font_size(), - FontBaseSize::InheritedStyle => context.inherited_style().get_font().clone_font_size(), + FontBaseSize::InheritedStyle => context.style().get_parent_font().clone_font_size(), } } } |