From 5c66290142608b94b32d742a0645dff76f47d3a1 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Tue, 18 Sep 2018 09:34:21 +0100 Subject: [PATCH 1/6] style: Use nsAtom instead of nsString for FontFamilyName. Bug: 1490997 Reviewed-by: lsalzman --- components/style/values/computed/font.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index 2c3d2853e5c..4cb8f7a7bdf 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -483,16 +483,19 @@ impl SingleFontFamily { FontFamilyType::eFamily_fantasy => SingleFontFamily::Generic(atom!("fantasy")), FontFamilyType::eFamily_moz_fixed => SingleFontFamily::Generic(atom!("-moz-fixed")), FontFamilyType::eFamily_named => { - let name = Atom::from(&*family.mName); + let name = unsafe { Atom::from_raw(family.mName.mRawPtr) }; SingleFontFamily::FamilyName(FamilyName { name, syntax: FamilyNameSyntax::Identifiers, }) }, - FontFamilyType::eFamily_named_quoted => SingleFontFamily::FamilyName(FamilyName { - name: (&*family.mName).into(), - syntax: FamilyNameSyntax::Quoted, - }), + FontFamilyType::eFamily_named_quoted => { + let name = unsafe { Atom::from_raw(family.mName.mRawPtr) }; + SingleFontFamily::FamilyName(FamilyName { + name, + syntax: FamilyNameSyntax::Quoted, + }) + }, _ => panic!("Found unexpected font FontFamilyType"), } } @@ -538,9 +541,15 @@ impl Hash for FontFamilyList { where H: Hasher, { + use string_cache::WeakAtom; + for name in self.0.mNames.iter() { name.mType.hash(state); - name.mName.hash(state); + if !name.mName.mRawPtr.is_null() { + unsafe { + WeakAtom::new(name.mName.mRawPtr).hash(state); + } + } } } } @@ -552,7 +561,7 @@ impl PartialEq for FontFamilyList { return false; } for (a, b) in self.0.mNames.iter().zip(other.0.mNames.iter()) { - if a.mType != b.mType || &*a.mName != &*b.mName { + if a.mType != b.mType || a.mName.mRawPtr != b.mName.mRawPtr { return false; } } From 0bcffa70947cea2ac5c70a52933bb0097ac4ba1a Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Wed, 19 Sep 2018 05:33:12 +0000 Subject: [PATCH 2/6] style: Merge the two scrollbar color properties into scrollbar-color. Differential Revision: https://phabricator.services.mozilla.com/D6115 --- components/style/properties/gecko.mako.rs | 44 ++++++++++++++++++- .../properties/longhands/inherited_ui.mako.rs | 21 +++------ components/style/values/computed/ui.rs | 3 ++ components/style/values/generics/ui.rs | 24 ++++++++++ components/style/values/specified/ui.rs | 18 ++++++++ 5 files changed, 94 insertions(+), 16 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 1de99ec16f1..2275316bf72 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -5304,7 +5304,7 @@ clip-path <%self:impl_trait style_struct_name="InheritedUI" - skip_longhands="cursor"> + skip_longhands="cursor scrollbar-color"> pub fn set_cursor(&mut self, v: longhands::cursor::computed_value::T) { use style_traits::cursor::CursorKind; @@ -5450,6 +5450,48 @@ clip-path longhands::cursor::computed_value::T { images, keyword } } + + pub fn set_scrollbar_color(&mut self, v: longhands::scrollbar_color::computed_value::T) { + use gecko_bindings::structs::StyleComplexColor; + use values::generics::ui::ScrollbarColor; + match v { + ScrollbarColor::Auto => { + self.gecko.mScrollbarFaceColor = StyleComplexColor::auto(); + self.gecko.mScrollbarTrackColor = StyleComplexColor::auto(); + } + ScrollbarColor::Colors { thumb, track } => { + self.gecko.mScrollbarFaceColor = thumb.into(); + self.gecko.mScrollbarTrackColor = track.into(); + } + } + } + + pub fn copy_scrollbar_color_from(&mut self, other: &Self) { + self.gecko.mScrollbarFaceColor = other.gecko.mScrollbarFaceColor; + self.gecko.mScrollbarTrackColor = other.gecko.mScrollbarTrackColor; + } + + pub fn reset_scrollbar_color(&mut self, other: &Self) { + self.copy_scrollbar_color_from(other); + } + + pub fn clone_scrollbar_color(&self) -> longhands::scrollbar_color::computed_value::T { + use gecko_bindings::structs::StyleComplexColor_Tag as Tag; + use values::generics::ui::ScrollbarColor; + debug_assert!( + (self.gecko.mScrollbarFaceColor.mTag == Tag::eAuto) == + (self.gecko.mScrollbarTrackColor.mTag == Tag::eAuto), + "Whether the two colors are `auto` should match", + ); + if self.gecko.mScrollbarFaceColor.mTag == Tag::eAuto { + ScrollbarColor::Auto + } else { + ScrollbarColor::Colors { + thumb: self.gecko.mScrollbarFaceColor.into(), + track: self.gecko.mScrollbarTrackColor.into(), + } + } + } <%self:impl_trait style_struct_name="Column" diff --git a/components/style/properties/longhands/inherited_ui.mako.rs b/components/style/properties/longhands/inherited_ui.mako.rs index 0b2b590cf7f..6bdd0ff51b5 100644 --- a/components/style/properties/longhands/inherited_ui.mako.rs +++ b/components/style/properties/longhands/inherited_ui.mako.rs @@ -67,24 +67,15 @@ ${helpers.predefined_type( products="gecko", )} -// Only scrollbar-face-color and scrollbar-track-color are added here. -// These two are the only common parts of scrollbar among Windows and -// macOS. We may or may not want to provide other properties to allow -// finer-grain control. -// -// NOTE The syntax in spec is currently `normal | `, but I think -// reusing `auto | ` as `caret-color` makes more sense. See -// https://github.com/w3c/csswg-drafts/issues/2660 -% for part in ["face", "track"]: ${helpers.predefined_type( - "scrollbar-%s-color" % part, - "ColorOrAuto", - "Either::Second(Auto)", - spec="https://drafts.csswg.org/css-scrollbars-1/#scrollbar-color-properties", + "scrollbar-color", + "ui::ScrollbarColor", + "Default::default()", + spec="https://drafts.csswg.org/css-scrollbars-1/#scrollbar-color", gecko_pref="layout.css.scrollbar-colors.enabled", - animation_value_type="ColorOrAuto", + animation_value_type="ScrollbarColor", + boxed=True, ignored_when_colors_disabled=True, enabled_in="chrome", products="gecko", )} -% endfor diff --git a/components/style/values/computed/ui.rs b/components/style/values/computed/ui.rs index 5307f79a38e..40f1a378cc9 100644 --- a/components/style/values/computed/ui.rs +++ b/components/style/values/computed/ui.rs @@ -20,3 +20,6 @@ pub type Cursor = generics::Cursor; /// A computed value for item of `image cursors`. pub type CursorImage = generics::CursorImage; + +/// A computed value for `scrollbar-color` property. +pub type ScrollbarColor = generics::ScrollbarColor; diff --git a/components/style/values/generics/ui.rs b/components/style/values/generics/ui.rs index bef1926bc90..73f050bb12a 100644 --- a/components/style/values/generics/ui.rs +++ b/components/style/values/generics/ui.rs @@ -67,3 +67,27 @@ impl ToCss for CursorImage { Ok(()) } } + +/// A generic value for `scrollbar-color` property. +/// +/// https://drafts.csswg.org/css-scrollbars-1/#scrollbar-color +#[derive(Animate, Clone, ComputeSquaredDistance, Copy, Debug, MallocSizeOf, PartialEq, + SpecifiedValueInfo, ToAnimatedValue, ToAnimatedZero, ToComputedValue, ToCss)] +pub enum ScrollbarColor { + /// `auto` + Auto, + /// `{2}` + Colors { + /// First ``, for color of the scrollbar thumb. + thumb: Color, + /// Second ``, for color of the scrollbar track. + track: Color, + } +} + +impl Default for ScrollbarColor { + #[inline] + fn default() -> Self { + ScrollbarColor::Auto + } +} diff --git a/components/style/values/specified/ui.rs b/components/style/values/specified/ui.rs index 7dc1e0fe1d3..c9c5c0a8414 100644 --- a/components/style/values/specified/ui.rs +++ b/components/style/values/specified/ui.rs @@ -122,3 +122,21 @@ impl From for u8 { } } } + +/// A specified value for `scrollbar-color` property +pub type ScrollbarColor = generics::ScrollbarColor; + +impl Parse for ScrollbarColor { + fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + if input.try(|i| i.expect_ident_matching("auto")).is_ok() { + return Ok(generics::ScrollbarColor::Auto); + } + Ok(generics::ScrollbarColor::Colors { + thumb: Color::parse(context, input)?, + track: Color::parse(context, input)?, + }) + } +} From 3c6be59d226af04ca6c67bc7d5a31f041cde7cde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 19 Sep 2018 22:02:01 +0200 Subject: [PATCH 3/6] style: Back out bug 1481866. The behavior the WG proposed is way more subtle than what that bug implements, including: * Implementing two logical overflow longhands. * Expanding the overflow shorthand to different longhands depending on the syntax of that. Meanwhile, Blink hasn't done the swap and will ship the same behavior that we shipped in Firefox 61 (bug 1453148), that is, overflow-x, then overflow-y. So I think lacking a clear way forward we should revert this change and preserve our shipped behavior. Differential Revision: https://phabricator.services.mozilla.com/D6317 --- components/style/properties/shorthands/box.mako.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/components/style/properties/shorthands/box.mako.rs b/components/style/properties/shorthands/box.mako.rs index 960e608cea7..3886c7d2d3c 100644 --- a/components/style/properties/shorthands/box.mako.rs +++ b/components/style/properties/shorthands/box.mako.rs @@ -7,7 +7,7 @@ <%helpers:shorthand name="overflow" flags="SHORTHAND_IN_GETCS" - sub_properties="overflow-y overflow-x" + sub_properties="overflow-x overflow-y" spec="https://drafts.csswg.org/css-overflow/#propdef-overflow" > use properties::longhands::overflow_x::parse as parse_overflow; @@ -52,9 +52,9 @@ } } % endif - let overflow_y = parse_overflow(context, input)?; - let overflow_x = - input.try(|i| parse_overflow(context, i)).unwrap_or(overflow_y); + let overflow_x = parse_overflow(context, input)?; + let overflow_y = + input.try(|i| parse_overflow(context, i)).unwrap_or(overflow_x); Ok(expanded! { overflow_x: overflow_x, overflow_y: overflow_y, @@ -63,10 +63,10 @@ impl<'a> ToCss for LonghandsToSerialize<'a> { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: fmt::Write { - self.overflow_y.to_css(dest)?; + self.overflow_x.to_css(dest)?; if self.overflow_x != self.overflow_y { dest.write_char(' ')?; - self.overflow_x.to_css(dest)?; + self.overflow_y.to_css(dest)?; } Ok(()) } From b55bfc49fbb3294b6bd90eff84dbdeed529c6725 Mon Sep 17 00:00:00 2001 From: Brad Werth Date: Wed, 12 Sep 2018 15:54:24 -0700 Subject: [PATCH 4/6] style: Provide a specialized parse_method for mask-image to use CORS. Differential Revision: https://phabricator.services.mozilla.com/D5714 --- components/style/properties/longhands/svg.mako.rs | 1 + components/style/values/specified/image.rs | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/components/style/properties/longhands/svg.mako.rs b/components/style/properties/longhands/svg.mako.rs index 36105247676..acdb809135f 100644 --- a/components/style/properties/longhands/svg.mako.rs +++ b/components/style/properties/longhands/svg.mako.rs @@ -183,6 +183,7 @@ ${helpers.predefined_type( "ImageLayer", "Either::First(None_)", initial_specified_value="Either::First(None_)", + parse_method="parse_with_cors_anonymous", spec="https://drafts.fxtf.org/css-masking/#propdef-mask-image", vector=True, products="gecko", diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index 1b080816e83..b1a611846a8 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -33,6 +33,20 @@ use values::specified::url::SpecifiedImageUrl; /// A specified image layer. pub type ImageLayer = Either; +impl ImageLayer { + /// This is a specialization of Either with an alternative parse + /// method to provide anonymous CORS headers for the Image url fetch. + pub fn parse_with_cors_anonymous<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + if let Ok(v) = input.try(|i| None_::parse(context, i)) { + return Ok(Either::First(v)); + } + Image::parse_with_cors_anonymous(context, input).map(Either::Second) + } +} + /// Specified values for an image according to CSS-IMAGES. /// pub type Image = generic::Image; From e5f8155d6c780107d5c3dc742bfa11102d247ff3 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Fri, 21 Sep 2018 18:39:41 +0000 Subject: [PATCH 5/6] style: Flip boolean half way for path interpolation. According to the new svg 2 spec update (#543), we flip the flag half way for path interpolation. Differential Revision: https://phabricator.services.mozilla.com/D6192 --- components/style/values/specified/svg_path.rs | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/components/style/values/specified/svg_path.rs b/components/style/values/specified/svg_path.rs index 5d397558d25..3f1fdc39d97 100644 --- a/components/style/values/specified/svg_path.rs +++ b/components/style/values/specified/svg_path.rs @@ -13,7 +13,7 @@ use std::slice; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; use style_traits::values::SequenceWriter; use values::CSSFloat; -use values::animated::{Animate, Procedure}; +use values::animated::{Animate, Procedure, ToAnimatedZero}; use values::distance::{ComputeSquaredDistance, SquaredDistance}; /// The SVG path data. @@ -198,9 +198,7 @@ pub enum PathCommand { rx: CSSFloat, ry: CSSFloat, angle: CSSFloat, - #[animation(constant)] large_arc_flag: ArcFlag, - #[animation(constant)] sweep_flag: ArcFlag, point: CoordPair, absolute: IsAbsolute, @@ -538,6 +536,34 @@ impl ToCss for ArcFlag { } } +impl Animate for ArcFlag { + #[inline] + fn animate(&self, other: &Self, procedure: Procedure) -> Result { + (self.0 as i32) + .animate(&(other.0 as i32), procedure) + .map(|v| ArcFlag(v > 0)) + } +} + +impl ComputeSquaredDistance for ArcFlag { + #[inline] + fn compute_squared_distance(&self, other: &Self) -> Result { + (self.0 as i32).compute_squared_distance(&(other.0 as i32)) + } +} + +impl ToAnimatedZero for ArcFlag { + #[inline] + fn to_animated_zero(&self) -> Result { + // The 2 ArcFlags in EllipticalArc determine which one of the 4 different arcs will be + // used. (i.e. From 4 combinations). In other words, if we change the flag, we get a + // different arc. Therefore, we return *self. + // https://svgwg.org/svg2-draft/paths.html#PathDataEllipticalArcCommands + Ok(*self) + } +} + + /// SVG Path parser. struct PathParser<'a> { chars: Peekable>>, From 285ea1fc5bd3b7fd9e29d4e4a6676383b1c95065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 22 Sep 2018 18:56:30 +0200 Subject: [PATCH 6/6] Update test expectations. --- tests/wpt/metadata/css/cssom/overflow-serialization.html.ini | 4 ---- tests/wpt/metadata/css/cssom/shorthand-values.html.ini | 3 --- 2 files changed, 7 deletions(-) delete mode 100644 tests/wpt/metadata/css/cssom/overflow-serialization.html.ini diff --git a/tests/wpt/metadata/css/cssom/overflow-serialization.html.ini b/tests/wpt/metadata/css/cssom/overflow-serialization.html.ini deleted file mode 100644 index 3a51b8caa02..00000000000 --- a/tests/wpt/metadata/css/cssom/overflow-serialization.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[overflow-serialization.html] - [CSSOM - Overflow shorthand serialization] - expected: FAIL - diff --git a/tests/wpt/metadata/css/cssom/shorthand-values.html.ini b/tests/wpt/metadata/css/cssom/shorthand-values.html.ini index 427f39aa3e7..e0b5a48bdb5 100644 --- a/tests/wpt/metadata/css/cssom/shorthand-values.html.ini +++ b/tests/wpt/metadata/css/cssom/shorthand-values.html.ini @@ -29,6 +29,3 @@ [The serialization of list-style-type: circle; list-style-position: inside; list-style-image: initial; should be canonical.] expected: FAIL - [The serialization of overflow-x: scroll; overflow-y: hidden; should be canonical.] - expected: FAIL -