From 1b2fd3fe8536e4f73b0c31d89fa0d3bdd01f74df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 21 Jun 2017 12:16:43 +0200 Subject: [PATCH] style: Be more strict when setting the root font size. Before this commit, we assumed that if the element had no parent element, it was the root of the document, which is plain false, since we can arrive there from, let's say, getComputedStyle on a detached node. Bug: 1374062 Reviewed-By: heycam MozReview-Commit-ID: 65DxdzXgd0J --- components/style/animation.rs | 1 - components/style/matching.rs | 8 ++--- .../style/properties/properties.mako.rs | 34 +++++++++---------- components/style/style_adjuster.rs | 10 +++--- ports/geckolib/glue.rs | 2 +- 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/components/style/animation.rs b/components/style/animation.rs index f8fde047bf11..50735801c2aa 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -502,7 +502,6 @@ fn compute_style_for_animation_step(context: &SharedStyleContext, // as existing browsers don't appear to animate visited styles. let computed = properties::apply_declarations(context.stylist.device(), - /* is_root = */ false, iter, previous_style, previous_style, diff --git a/components/style/matching.rs b/components/style/matching.rs index f65f183e64bc..44b02daba484 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -17,8 +17,8 @@ use invalidation::element::restyle_hints::{RESTYLE_CSS_ANIMATIONS, RESTYLE_CSS_T use invalidation::element::restyle_hints::{RESTYLE_SMIL, RESTYLE_STYLE_ATTRIBUTE}; use invalidation::element::restyle_hints::RestyleHint; use log::LogLevel::Trace; -use properties::{ALLOW_SET_ROOT_FONT_SIZE, PROHIBIT_DISPLAY_CONTENTS, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP}; use properties::{AnimationRules, CascadeFlags, ComputedValues}; +use properties::{IS_ROOT_ELEMENT, PROHIBIT_DISPLAY_CONTENTS, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP}; use properties::{VISITED_DEPENDENT_ONLY, cascade}; use properties::longhands::display::computed_value as display; use rule_tree::{CascadeLevel, StrongRuleNode}; @@ -33,7 +33,7 @@ use stylist::RuleInclusion; /// /// Controls where we inherit styles from, and whether display:contents is /// prohibited. -#[derive(PartialEq, Copy, Clone)] +#[derive(PartialEq, Copy, Clone, Debug)] enum CascadeTarget { /// Inherit from the parent element, as normal CSS dictates, _or_ from the /// closest non-Native Anonymous element in case this is Native Anonymous @@ -273,8 +273,8 @@ trait PrivateMatchMethods: TElement { } if self.is_native_anonymous() || cascade_target == CascadeTarget::EagerPseudo { cascade_flags.insert(PROHIBIT_DISPLAY_CONTENTS); - } else { - cascade_flags.insert(ALLOW_SET_ROOT_FONT_SIZE); + } else if self.is_root() { + cascade_flags.insert(IS_ROOT_ELEMENT); } // Grab the inherited values. diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 9fcb87a03b1f..1df4615722a5 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -2472,20 +2472,24 @@ bitflags! { /// Whether to inherit all styles from the parent. If this flag is not /// present, non-inherited styles are reset to their initial values. const INHERIT_ALL = 0x01, + /// Whether to skip any root element and flex/grid item display style /// fixup. const SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP = 0x02, + /// Whether to only cascade properties that are visited dependent. const VISITED_DEPENDENT_ONLY = 0x04, - /// Should we modify the device's root font size - /// when computing the root? + + /// Whether the given element we're styling is the document element, + /// that is, matches :root. /// - /// Not set for native anonymous content since some NAC - /// form their own root, but share the device. + /// Not set for native anonymous content since some NAC form their own + /// root, but share the device. /// - /// ::backdrop and all NAC will resolve rem units against - /// the toplevel root element now. - const ALLOW_SET_ROOT_FONT_SIZE = 0x08, + /// This affects some style adjustments, like blockification, and means + /// that it may affect global state, like the Device's root font-size. + const IS_ROOT_ELEMENT = 0x08, + /// Whether to convert display:contents into display:inline. This /// is used by Gecko to prevent display:contents on generated /// content. @@ -2520,15 +2524,13 @@ pub fn cascade(device: &Device, quirks_mode: QuirksMode) -> ComputedValues { debug_assert_eq!(parent_style.is_some(), layout_parent_style.is_some()); - let (is_root_element, inherited_style, layout_parent_style) = match parent_style { + let (inherited_style, layout_parent_style) = match parent_style { Some(parent_style) => { - (false, - parent_style, + (parent_style, layout_parent_style.unwrap()) }, None => { - (true, - device.default_computed_values(), + (device.default_computed_values(), device.default_computed_values()) } }; @@ -2558,7 +2560,6 @@ pub fn cascade(device: &Device, }) }; apply_declarations(device, - is_root_element, iter_declarations, inherited_style, layout_parent_style, @@ -2574,7 +2575,6 @@ pub fn cascade(device: &Device, /// first. #[allow(unused_mut)] // conditionally compiled code for "position" pub fn apply_declarations<'a, F, I>(device: &Device, - is_root_element: bool, iter_declarations: F, inherited_style: &ComputedValues, layout_parent_style: &ComputedValues, @@ -2629,7 +2629,7 @@ pub fn apply_declarations<'a, F, I>(device: &Device, }; let mut context = computed::Context { - is_root_element: is_root_element, + is_root_element: flags.contains(IS_ROOT_ELEMENT), device: device, inherited_style: inherited_style, layout_parent_style: layout_parent_style, @@ -2853,7 +2853,7 @@ pub fn apply_declarations<'a, F, I>(device: &Device, % endif } - if is_root_element && flags.contains(ALLOW_SET_ROOT_FONT_SIZE) { + if context.is_root_element { let s = context.style.get_font().clone_font_size(); context.device.set_root_font_size(s); } @@ -2863,7 +2863,7 @@ pub fn apply_declarations<'a, F, I>(device: &Device, let mut style = context.style; { - StyleAdjuster::new(&mut style, is_root_element) + StyleAdjuster::new(&mut style) .adjust(context.layout_parent_style, flags); } diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 4c4a45187484..6d0d84c34f46 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -7,7 +7,7 @@ use app_units::Au; use properties::{self, CascadeFlags, ComputedValues}; -use properties::{SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, StyleBuilder}; +use properties::{IS_ROOT_ELEMENT, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, StyleBuilder}; use properties::longhands::display::computed_value::T as display; use properties::longhands::float::computed_value::T as float; use properties::longhands::overflow_x::computed_value::T as overflow; @@ -17,15 +17,13 @@ use properties::longhands::position::computed_value::T as position; /// An unsized struct that implements all the adjustment methods. pub struct StyleAdjuster<'a, 'b: 'a> { style: &'a mut StyleBuilder<'b>, - is_root_element: bool, } impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { /// Trivially constructs a new StyleAdjuster. - pub fn new(style: &'a mut StyleBuilder<'b>, is_root_element: bool) -> Self { + pub fn new(style: &'a mut StyleBuilder<'b>) -> Self { StyleAdjuster { style: style, - is_root_element: is_root_element, } } @@ -66,7 +64,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } if !flags.contains(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP) { - blockify_if!(self.is_root_element); + blockify_if!(flags.contains(IS_ROOT_ELEMENT)); blockify_if!(layout_parent_style.get_box().clone_display().is_item_container()); } @@ -81,7 +79,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { let display = self.style.get_box().clone_display(); let blockified_display = - display.equivalent_block_display(self.is_root_element); + display.equivalent_block_display(flags.contains(IS_ROOT_ELEMENT)); if display != blockified_display { self.style.mutate_box().set_adjusted_display(blockified_display, is_item_or_root); diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 2ae1ea05f927..dbc1624527b9 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -1535,7 +1535,7 @@ pub extern "C" fn Servo_ComputedValues_Inherit( StyleBuilder::for_inheritance(reference, &data.default_computed_values()); if for_text { - StyleAdjuster::new(&mut style, /* is_root = */ false) + StyleAdjuster::new(&mut style) .adjust_for_text(); }