From c00045b0c9b34febc815ba7c727a253a4e04be62 Mon Sep 17 00:00:00 2001 From: alwu Date: Tue, 9 Jul 2019 01:42:45 +0000 Subject: [PATCH 01/23] style: set CSS properties directly on '::cue'. According to the spec [1], we have to set those CSS properties on the root node, and then this root node would have a child node, background box [2], which would contain all other child nodes. In our case, the background box is `cueDiv` [3]. In theory, all those properties set on the root node should be inherited by the background box. However, when the background box is a pseudo element `::cue`, they won't be directly inherit from the the background box's parent, inherited styles would acutally come from video instead. Therefore, we have to directly set these properties on the pseudo element and mark them as `!important` to avoid being overrided by user style script. [1] https://www.w3.org/TR/webvtt1/#ref-for-list-of-webvtt-node-objects-9 [2] https://www.w3.org/TR/webvtt1/#webvtt-cue-background-box [3] https://searchfox.org/mozilla-central/rev/11712bd3ce7454923e5931fa92eaf9c01ef35a0a/dom/media/webvtt/vtt.jsm#533-534 Differential Revision: https://phabricator.services.mozilla.com/D35694 --- components/style/properties/longhands/inherited_box.mako.rs | 1 + components/style/properties/longhands/inherited_text.mako.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/components/style/properties/longhands/inherited_box.mako.rs b/components/style/properties/longhands/inherited_box.mako.rs index 94ed265defa..f14c485b801 100644 --- a/components/style/properties/longhands/inherited_box.mako.rs +++ b/components/style/properties/longhands/inherited_box.mako.rs @@ -29,6 +29,7 @@ ${helpers.single_keyword( servo_pref="layout.writing-mode.enabled", animation_value_type="none", spec="https://drafts.csswg.org/css-writing-modes/#propdef-writing-mode", + flags="APPLIES_TO_CUE", servo_restyle_damage="rebuild_and_reflow", )} diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs index c81e8392f94..63596b7e653 100644 --- a/components/style/properties/longhands/inherited_text.mako.rs +++ b/components/style/properties/longhands/inherited_text.mako.rs @@ -78,6 +78,7 @@ ${helpers.predefined_type( "computed::OverflowWrap::Normal", animation_value_type="discrete", spec="https://drafts.csswg.org/css-text/#propdef-overflow-wrap", + flags="APPLIES_TO_CUE", alias="word-wrap", needs_context=False, servo_restyle_damage="rebuild_and_reflow", From 6cf87d23f809e2945d8d285bea00033fe361f01a Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Tue, 9 Jul 2019 08:59:27 +0000 Subject: [PATCH 02/23] style: Add an 'auto' value for the CSS 'quotes' property, and make it use language-dependent quote marks. Differential Revision: https://phabricator.services.mozilla.com/D36429 --- components/style/values/computed/list.rs | 20 ++---------- components/style/values/computed/mod.rs | 2 +- components/style/values/specified/list.rs | 40 ++++++++++++++++++++--- components/style/values/specified/mod.rs | 2 +- 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/components/style/values/computed/list.rs b/components/style/values/computed/list.rs index bdcf62ba2f8..2ae3776041b 100644 --- a/components/style/values/computed/list.rs +++ b/components/style/values/computed/list.rs @@ -7,28 +7,12 @@ #[cfg(feature = "gecko")] pub use crate::values::specified::list::ListStyleType; pub use crate::values::specified::list::MozListReversed; -pub use crate::values::specified::list::{QuotePair, Quotes}; - -lazy_static! { - static ref INITIAL_QUOTES: crate::ArcSlice = crate::ArcSlice::from_iter_leaked( - vec![ - QuotePair { - opening: "\u{201c}".to_owned().into(), - closing: "\u{201d}".to_owned().into(), - }, - QuotePair { - opening: "\u{2018}".to_owned().into(), - closing: "\u{2019}".to_owned().into(), - }, - ] - .into_iter() - ); -} +pub use crate::values::specified::list::Quotes; impl Quotes { /// Initial value for `quotes`. #[inline] pub fn get_initial_value() -> Quotes { - Quotes(INITIAL_QUOTES.clone()) + Quotes::Auto } } diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index cea768abeff..aa37189f051 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -64,7 +64,7 @@ pub use self::length::{NonNegativeLengthPercentage, NonNegativeLengthPercentageO #[cfg(feature = "gecko")] pub use self::list::ListStyleType; pub use self::list::MozListReversed; -pub use self::list::{QuotePair, Quotes}; +pub use self::list::Quotes; pub use self::motion::{OffsetPath, OffsetRotate}; pub use self::outline::OutlineStyle; pub use self::percentage::{NonNegativePercentage, Percentage}; diff --git a/components/style/values/specified/list.rs b/components/style/values/specified/list.rs index 0519053b902..f5bda33ce73 100644 --- a/components/style/values/specified/list.rs +++ b/components/style/values/specified/list.rs @@ -104,7 +104,7 @@ pub struct QuotePair { pub closing: crate::OwnedStr, } -/// Specified and computed `quotes` property. +/// List of quote pairs for the specified/computed value of `quotes` property. #[derive( Clone, Debug, @@ -117,23 +117,53 @@ pub struct QuotePair { ToResolvedValue, ToShmem, )] -#[repr(C)] -pub struct Quotes( +#[repr(transparent)] +pub struct QuoteList( #[css(iterable, if_empty = "none")] #[ignore_malloc_size_of = "Arc"] pub crate::ArcSlice, ); +/// Specified and computed `quotes` property: `auto`, `none`, or a list +/// of characters. +/// +/// cbindgen:derive-tagged-enum-copy-constructor=true +#[derive( + Clone, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(C)] +pub enum Quotes { + /// list of quote pairs + QuoteList(QuoteList), + /// auto (use lang-dependent quote marks) + Auto, +} + impl Parse for Quotes { fn parse<'i, 't>( _: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { + if input + .try(|input| input.expect_ident_matching("auto")) + .is_ok() + { + return Ok(Quotes::Auto); + } + if input .try(|input| input.expect_ident_matching("none")) .is_ok() { - return Ok(Self::default()); + return Ok(Quotes::QuoteList(QuoteList::default())) } let mut quotes = Vec::new(); @@ -150,7 +180,7 @@ impl Parse for Quotes { } if !quotes.is_empty() { - Ok(Quotes(crate::ArcSlice::from_iter(quotes.into_iter()))) + Ok(Quotes::QuoteList(QuoteList(crate::ArcSlice::from_iter(quotes.into_iter())))) } else { Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 14439d2eea0..b22292a9dce 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -65,7 +65,7 @@ pub use self::length::{NonNegativeLengthPercentage, NonNegativeLengthPercentageO #[cfg(feature = "gecko")] pub use self::list::ListStyleType; pub use self::list::MozListReversed; -pub use self::list::{QuotePair, Quotes}; +pub use self::list::Quotes; pub use self::motion::{OffsetPath, OffsetRotate}; pub use self::outline::OutlineStyle; pub use self::percentage::Percentage; From 145acbf8767398ff8895195294a88b22981c5e46 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 10 Jul 2019 20:25:25 +0000 Subject: [PATCH 03/23] style: Part 2: Retire the support for 3-valued syntax for position. According to this resolved spec issue: https://github.com/w3c/csswg-drafts/issues/2140, we retire the 3-valued on 1. `object-position` 2. `perspective-origin`, 3. `mask-position` 4. `circle()` and `ellipse()` , but still keep the support for `background-position`. Besides, I simply run this python script to generate the .ini file: ``` s = sys.argv[1] + ".ini" with open(s, "w") as f: f.write('[{}]\n'.format(sys.argv[1])) f.write(' expected: FAIL\n') f.write(' bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1559276\n') ``` Differential Revision: https://phabricator.services.mozilla.com/D37126 --- .../properties/shorthands/background.mako.rs | 8 +++--- components/style/values/specified/position.rs | 27 ++++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/components/style/properties/shorthands/background.mako.rs b/components/style/properties/shorthands/background.mako.rs index af98342bfe6..7986201e423 100644 --- a/components/style/properties/shorthands/background.mako.rs +++ b/components/style/properties/shorthands/background.mako.rs @@ -15,7 +15,7 @@ use crate::properties::longhands::background_clip; use crate::properties::longhands::background_clip::single_value::computed_value::T as Clip; use crate::properties::longhands::background_origin::single_value::computed_value::T as Origin; - use crate::values::specified::{Color, Position, PositionComponent}; + use crate::values::specified::{AllowQuirks, Color, Position, PositionComponent}; use crate::parser::Parse; // FIXME(emilio): Should be the same type! @@ -64,7 +64,9 @@ } } if position.is_none() { - if let Ok(value) = input.try(|input| Position::parse(context, input)) { + if let Ok(value) = input.try(|input| { + Position::parse_three_value_quirky(context, input, AllowQuirks::No) + }) { position = Some(value); // Parse background size, if applicable. @@ -211,7 +213,7 @@ let mut any = false; input.parse_comma_separated(|input| { - let value = Position::parse_quirky(context, input, AllowQuirks::Yes)?; + let value = Position::parse_three_value_quirky(context, input, AllowQuirks::Yes)?; position_x.push(value.horizontal); position_y.push(value.vertical); any = true; diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index 85516790c4f..e4ab6536a65 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -94,13 +94,17 @@ impl Parse for Position { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - Self::parse_quirky(context, input, AllowQuirks::No) + let position = Self::parse_three_value_quirky(context, input, AllowQuirks::No)?; + if position.is_three_value_syntax() { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); + } + Ok(position) } } impl Position { - /// Parses a ``, with quirks. - pub fn parse_quirky<'i, 't>( + /// Parses a ``, with quirks. + pub fn parse_three_value_quirky<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, allow_quirks: AllowQuirks, @@ -183,6 +187,12 @@ impl Position { pub fn center() -> Self { Self::new(PositionComponent::Center, PositionComponent::Center) } + + /// Returns true if this uses a 3 value syntax. + #[inline] + fn is_three_value_syntax(&self) -> bool { + self.horizontal.component_count() != self.vertical.component_count() + } } impl ToCss for Position { @@ -252,6 +262,17 @@ impl PositionComponent { pub fn zero() -> Self { PositionComponent::Length(LengthPercentage::Percentage(Percentage::zero())) } + + /// Returns the count of this component. + fn component_count(&self) -> usize { + match *self { + PositionComponent::Length(..) | + PositionComponent::Center => 1, + PositionComponent::Side(_, ref lp) => { + if lp.is_some() { 2 } else { 1 } + } + } + } } impl ToComputedValue for PositionComponent { From e3b57efc7eca991de9eee8a48b9e0a5e501366db Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Wed, 10 Jul 2019 22:43:24 +0000 Subject: [PATCH 04/23] style: Ignore pref-controlled properties in cached scrollbar style assertions. Now if you add a new inherited, pref-controlled property, you must declare whether it can have an effect on scrollbar styles. If no, then the property will be skipped in the assertions that check whether our cached styles are equal to those we would compute. Differential Revision: https://phabricator.services.mozilla.com/D37507 --- components/style/properties/data.py | 9 +++++++++ .../style/properties/longhands/font.mako.rs | 3 +++ .../properties/longhands/inherited_text.mako.rs | 5 ++++- .../properties/longhands/inherited_ui.mako.rs | 4 ++++ components/style/properties/properties.mako.rs | 16 ++++++++++++++++ 5 files changed, 36 insertions(+), 1 deletion(-) diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 0c9321d004c..9365b6e6227 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -175,6 +175,7 @@ class Longhand(object): predefined_type=None, servo_pref=None, gecko_pref=None, enabled_in="content", need_index=False, gecko_ffi_name=None, + has_effect_on_gecko_scrollbars=None, allowed_in_keyframe_block=True, cast_type='u8', logical=False, logical_group=None, alias=None, extra_prefixes=None, boxed=False, flags=None, allowed_in_page_rule=False, allow_quirks="No", @@ -192,6 +193,14 @@ class Longhand(object): self.style_struct = style_struct self.servo_pref = servo_pref self.gecko_pref = gecko_pref + self.has_effect_on_gecko_scrollbars = has_effect_on_gecko_scrollbars + assert ( + has_effect_on_gecko_scrollbars in [None, False, True] and + not style_struct.inherited or + (gecko_pref is None) == (has_effect_on_gecko_scrollbars is None)), ( + "Property " + name + ": has_effect_on_gecko_scrollbars must be " + + "specified, and must have a value of True or False, iff a " + + "property is inherited and is behind a Gecko pref") # For enabled_in, the setup is as follows: # It needs to be one of the four values: ["", "ua", "chrome", "content"] # * "chrome" implies "ua", and implies that they're explicitly diff --git a/components/style/properties/longhands/font.mako.rs b/components/style/properties/longhands/font.mako.rs index d65afad693d..0323b892115 100644 --- a/components/style/properties/longhands/font.mako.rs +++ b/components/style/properties/longhands/font.mako.rs @@ -192,6 +192,7 @@ ${helpers.predefined_type( "FontVariationSettings", products="gecko", gecko_pref="layout.css.font-variations.enabled", + has_effect_on_gecko_scrollbars=False, initial_value="computed::FontVariationSettings::normal()", initial_specified_value="specified::FontVariationSettings::normal()", animation_value_type="ComputedValue", @@ -216,6 +217,7 @@ ${helpers.single_keyword_system( "auto none", products="gecko", gecko_pref="layout.css.font-variations.enabled", + has_effect_on_gecko_scrollbars=False, gecko_ffi_name="mFont.opticalSizing", gecko_constant_prefix="NS_FONT_OPTICAL_SIZING", animation_value_type="discrete", @@ -512,6 +514,7 @@ ${helpers.single_keyword( gecko_constant_prefix="NS_FONT_SMOOTHING", gecko_ffi_name="mFont.smoothing", gecko_pref="layout.css.osx-font-smoothing.enabled", + has_effect_on_gecko_scrollbars=False, products="gecko", spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/font-smooth)", flags="APPLIES_TO_CUE APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER APPLIES_TO_MARKER", diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs index 63596b7e653..614685f38d5 100644 --- a/components/style/properties/longhands/inherited_text.mako.rs +++ b/components/style/properties/longhands/inherited_text.mako.rs @@ -103,7 +103,8 @@ ${helpers.predefined_type( gecko_enum_prefix="StyleTextJustify" animation_value_type="discrete" gecko_pref="layout.css.text-justify.enabled" - flags="APPLIES_TO_PLACEHOLDER", + has_effect_on_gecko_scrollbars="False" + flags="APPLIES_TO_PLACEHOLDER" spec="https://drafts.csswg.org/css-text/#propdef-text-justify" servo_restyle_damage="rebuild_and_reflow" > @@ -383,6 +384,7 @@ ${helpers.predefined_type( products="gecko", animation_value_type="ComputedValue", gecko_pref="layout.css.text-underline-offset.enabled", + has_effect_on_gecko_scrollbars=False, spec="https://drafts.csswg.org/css-text-decor-4/#underline-offset", )} @@ -395,5 +397,6 @@ ${helpers.predefined_type( needs_context=False, animation_value_type="discrete", gecko_pref="layout.css.text-decoration-skip-ink.enabled", + has_effect_on_gecko_scrollbars=False, spec="https://drafts.csswg.org/css-text-decor-4/#text-decoration-skip-ink-property", )} diff --git a/components/style/properties/longhands/inherited_ui.mako.rs b/components/style/properties/longhands/inherited_ui.mako.rs index 07264372995..2bdb267ffb2 100644 --- a/components/style/properties/longhands/inherited_ui.mako.rs +++ b/components/style/properties/longhands/inherited_ui.mako.rs @@ -74,6 +74,10 @@ ${helpers.predefined_type( "Default::default()", spec="https://drafts.csswg.org/css-scrollbars-1/#scrollbar-color", gecko_pref="layout.css.scrollbar-color.enabled", + # Surprisingly, yes the computed value of scrollbar-color has no effect on + # Gecko scrollbar elements, since the value only matters on the scrollable + # element itself. + has_effect_on_gecko_scrollbars=False, animation_value_type="ScrollbarColor", boxed=True, ignored_when_colors_disabled=True, diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 4f07d6feb84..5139d887bbb 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -812,6 +812,22 @@ impl LonghandIdSet { &IGNORED_WHEN_COLORS_DISABLED } + /// Returns the set of properties that are declared as having no effect on + /// Gecko elements or their descendant scrollbar parts. + #[cfg(debug_assertions)] + #[cfg(feature = "gecko")] + #[inline] + pub fn has_no_effect_on_gecko_scrollbars() -> &'static Self { + // data.py asserts that has_no_effect_on_gecko_scrollbars is True or + // False for properties that are inherited and Gecko pref controlled, + // and is None for all other properties. + ${static_longhand_id_set( + "HAS_NO_EFFECT_ON_SCROLLBARS", + lambda p: p.has_effect_on_gecko_scrollbars is False + )} + &HAS_NO_EFFECT_ON_SCROLLBARS + } + /// Iterate over the current longhand id set. pub fn iter(&self) -> LonghandIdSetIterator { LonghandIdSetIterator { longhands: self, cur: 0, } From 10cb9c07aab93d6428707f6a13b5101e5b730363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 11 Jul 2019 23:06:41 +0000 Subject: [PATCH 05/23] style: Don't apply property restrictions to pseudo-elements in UA stylesheets. And remove some of the ::placeholder and ::cue hacks where we need to use !important to make the property not apply for content but apply on UA sheets. The comment about the white-space property was wrong, we don't enforce it with !important in the UA stylesheets for (we do for