From fcada8a71e3f1608617331bfa1e5e5df75dbc4e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 12 Nov 2018 11:19:24 +0100 Subject: [PATCH 01/23] style: Remove outdated comment. --- components/style/values/specified/box.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index a5761d3f9b2..b8e38da37a7 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -52,9 +52,6 @@ fn moz_box_display_values_enabled(context: &ParserContext) -> bool { /// If you change it, make sure to take a look at the /// FrameConstructionDataByDisplay stuff (both the XUL and non-XUL version), and /// ensure it's still correct! -/// -/// Also, when you change this from Gecko you may need to regenerate the -/// C++-side bindings (see components/style/cbindgen.toml). #[allow(missing_docs)] #[derive( Clone, From 1aa6725f9a15a36cacb4814f089d0dc957e073a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 11 Nov 2018 17:57:41 +0100 Subject: [PATCH 02/23] style: Move user-select outside of mako. This is the first step to unprefix user-select. This has no behavior change, it's just a nicer way to do the same thing which allows us to unship individual values more easily using parse(condition). Differential Revision: https://phabricator.services.mozilla.com/D11580 --- components/style/cbindgen.toml | 1 + components/style/properties/data.py | 1 + components/style/properties/gecko.mako.rs | 1 + .../style/properties/longhands/ui.mako.rs | 11 +++--- components/style/values/computed/mod.rs | 2 +- components/style/values/computed/ui.rs | 2 +- components/style/values/specified/mod.rs | 2 +- components/style/values/specified/ui.rs | 36 +++++++++++++++++++ 8 files changed, 47 insertions(+), 9 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 06c1730622a..d9efc9d9e18 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -53,5 +53,6 @@ include = [ "TimingFunction", "PathCommand", "UnicodeRange", + "UserSelect", ] item_types = ["enums", "structs", "typedefs"] diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 10f64484693..97e2f7f0d84 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -336,6 +336,7 @@ class Longhand(object): "TextEmphasisPosition", "TouchAction", "TransformStyle", + "UserSelect", "XSpan", "XTextZoom", "ZIndex", diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index b845d382623..2cc8cf2bcd3 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1429,6 +1429,7 @@ impl Clone for ${style_struct.gecko_struct_name} { "SVGWidth": impl_svg_length, "Transform": impl_transform, "TransformOrigin": impl_transform_origin, + "UserSelect": impl_simple, "url::UrlOrNone": impl_css_url, } diff --git a/components/style/properties/longhands/ui.mako.rs b/components/style/properties/longhands/ui.mako.rs index b8a188d896b..b73792104cb 100644 --- a/components/style/properties/longhands/ui.mako.rs +++ b/components/style/properties/longhands/ui.mako.rs @@ -31,16 +31,15 @@ ${helpers.single_keyword( spec="https://drafts.csswg.org/css-scrollbars-1/#scrollbar-width" )} -${helpers.single_keyword( +${helpers.predefined_type( "-moz-user-select", - "auto text none all element elements toggle tri-state -moz-all -moz-text", + "UserSelect", + "computed::UserSelect::Auto", products="gecko", - alias="-webkit-user-select", gecko_ffi_name="mUserSelect", - gecko_enum_prefix="StyleUserSelect", - gecko_strip_moz_prefix=False, - aliases="-moz-none=none", + alias="-webkit-user-select", animation_value_type="discrete", + needs_context=False, spec="https://drafts.csswg.org/css-ui-4/#propdef-user-select", )} diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index e24472a8068..b8484a291ab 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -84,7 +84,7 @@ pub use self::transform::{Rotate, Scale, Transform, TransformOperation}; pub use self::transform::{TransformOrigin, TransformStyle, Translate}; #[cfg(feature = "gecko")] pub use self::ui::CursorImage; -pub use self::ui::{ColorOrAuto, Cursor, MozForceBrokenImageIcon}; +pub use self::ui::{ColorOrAuto, Cursor, MozForceBrokenImageIcon, UserSelect}; pub use super::specified::{BorderStyle, TextDecorationLine}; pub use super::{Auto, Either, None_}; pub use app_units::Au; diff --git a/components/style/values/computed/ui.rs b/components/style/values/computed/ui.rs index 57990d84d79..7d4a5c6a1ce 100644 --- a/components/style/values/computed/ui.rs +++ b/components/style/values/computed/ui.rs @@ -10,7 +10,7 @@ use crate::values::computed::Number; use crate::values::generics::ui as generics; use crate::values::{Auto, Either}; -pub use crate::values::specified::ui::MozForceBrokenImageIcon; +pub use crate::values::specified::ui::{MozForceBrokenImageIcon, UserSelect}; /// auto | pub type ColorOrAuto = Either; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 3b7a3d0e04e..bcca8ed8f49 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -82,7 +82,7 @@ pub use self::transform::{Rotate, Scale, Transform}; pub use self::transform::{TransformOrigin, TransformStyle, Translate}; #[cfg(feature = "gecko")] pub use self::ui::CursorImage; -pub use self::ui::{ColorOrAuto, Cursor, MozForceBrokenImageIcon}; +pub use self::ui::{ColorOrAuto, Cursor, MozForceBrokenImageIcon, UserSelect}; pub use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent; #[cfg(feature = "gecko")] diff --git a/components/style/values/specified/ui.rs b/components/style/values/specified/ui.rs index 5adfcaa7f70..f11aae73bc3 100644 --- a/components/style/values/specified/ui.rs +++ b/components/style/values/specified/ui.rs @@ -140,3 +140,39 @@ impl Parse for ScrollbarColor { }) } } + +/// The specified value for the `user-select` property. +/// +/// https://drafts.csswg.org/css-ui-4/#propdef-user-select +#[allow(missing_docs)] +#[derive( + Clone, + Copy, + Debug, + Eq, + MallocSizeOf, + Parse, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, +)] +#[repr(u8)] +pub enum UserSelect { + Auto, + Text, + #[parse(aliases = "-moz-none")] + None, + Element, + Elements, + All, + Toggle, + TriState, + /// Force selection of all children, unless an ancestor has `none` set. + /// + /// See bug 48096. + MozAll, + /// Like `text`, except that it won't get overridden by ancestors having + /// `all`. + MozText, +} From 0c8b1a90047b9faff9ba92ae026708d8356b5d6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 11 Nov 2018 18:00:49 +0100 Subject: [PATCH 03/23] style: Remove unimplemented values of the user-select property. Can't believe we literally had no code to handle them. Differential Revision: https://phabricator.services.mozilla.com/D11581 --- components/style/values/specified/ui.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/components/style/values/specified/ui.rs b/components/style/values/specified/ui.rs index f11aae73bc3..37dd4ebc9f2 100644 --- a/components/style/values/specified/ui.rs +++ b/components/style/values/specified/ui.rs @@ -163,11 +163,7 @@ pub enum UserSelect { Text, #[parse(aliases = "-moz-none")] None, - Element, - Elements, All, - Toggle, - TriState, /// Force selection of all children, unless an ancestor has `none` set. /// /// See bug 48096. From 4bc9bc11e142507291d26cd31989b3637bdce490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 11 Nov 2018 18:05:49 +0100 Subject: [PATCH 04/23] style: Remove user-select: -moz-all. It's an attempt of an alias to `all`, except it doesn't get handled in all places. Seems unused both in comm-central and mozilla-central, and all external usage I could find is followed by -webkit-user-select: all. Differential Revision: https://phabricator.services.mozilla.com/D11582 --- components/style/values/specified/ui.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/components/style/values/specified/ui.rs b/components/style/values/specified/ui.rs index 37dd4ebc9f2..6dbf910a655 100644 --- a/components/style/values/specified/ui.rs +++ b/components/style/values/specified/ui.rs @@ -163,11 +163,8 @@ pub enum UserSelect { Text, #[parse(aliases = "-moz-none")] None, - All, /// Force selection of all children, unless an ancestor has `none` set. - /// - /// See bug 48096. - MozAll, + All, /// Like `text`, except that it won't get overridden by ancestors having /// `all`. MozText, From 4c3646eff00f370ae8d9d32c7310da711873ed31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 11 Nov 2018 18:16:00 +0100 Subject: [PATCH 05/23] style: Allow user-select: -moz-text on user-agent stylesheets only. It's only used in contenteditable.css, and same usage in comm-central. That sheet is loaded as a ua sheet so let's restrict it to that. No relevant external usage either. This value was introduced in bug 1181130. Differential Revision: https://phabricator.services.mozilla.com/D11584 --- components/style/properties/longhands/ui.mako.rs | 1 - components/style/values/specified/ui.rs | 11 +++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/components/style/properties/longhands/ui.mako.rs b/components/style/properties/longhands/ui.mako.rs index b73792104cb..c077d4d8ba8 100644 --- a/components/style/properties/longhands/ui.mako.rs +++ b/components/style/properties/longhands/ui.mako.rs @@ -39,7 +39,6 @@ ${helpers.predefined_type( gecko_ffi_name="mUserSelect", alias="-webkit-user-select", animation_value_type="discrete", - needs_context=False, spec="https://drafts.csswg.org/css-ui-4/#propdef-user-select", )} diff --git a/components/style/values/specified/ui.rs b/components/style/values/specified/ui.rs index 6dbf910a655..067ce312102 100644 --- a/components/style/values/specified/ui.rs +++ b/components/style/values/specified/ui.rs @@ -141,6 +141,11 @@ impl Parse for ScrollbarColor { } } +fn in_ua_sheet(context: &ParserContext) -> bool { + use crate::stylesheets::Origin; + context.stylesheet_origin == Origin::UserAgent +} + /// The specified value for the `user-select` property. /// /// https://drafts.csswg.org/css-ui-4/#propdef-user-select @@ -167,5 +172,11 @@ pub enum UserSelect { All, /// Like `text`, except that it won't get overridden by ancestors having /// `all`. + /// + /// FIXME(emilio): This only has one use in contenteditable.css, can we find + /// a better way to do this? + /// + /// See bug 1181130. + #[parse(condition = "in_ua_sheet")] MozText, } From 6b5117d82d194223837591d70b3abc1bf3ea7fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 14 Nov 2018 12:15:38 +0100 Subject: [PATCH 06/23] style: Remove outdated comment. --- components/style/values/specified/box.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index b8e38da37a7..6a207d0a90c 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -943,9 +943,6 @@ pub enum Resize { /// The value for the `appearance` property. /// /// https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-appearance -/// -/// NOTE(emilio): When changing this you may want to regenerate the C++ bindings -/// (see components/style/cbindgen.toml) #[allow(missing_docs)] #[derive( Clone, From c75a3e4db1af000c61a51ad066d9346a4bca4702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 12 Nov 2018 09:21:17 +0000 Subject: [PATCH 07/23] style: Remove serialize_basicshape_position. Per https://github.com/w3c/csswg-drafts/issues/2274. There's a whole lot of new failures, but those need to be updated along with the spec changes in that issue. We did resolve that position serialized the same everywhere though. Differential Revision: https://phabricator.services.mozilla.com/D1933 --- .../style/values/specified/basic_shape.rs | 85 +------------------ 1 file changed, 3 insertions(+), 82 deletions(-) diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index d394d8881dd..b371d709b80 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -8,20 +8,17 @@ //! [basic-shape]: https://drafts.csswg.org/css-shapes/#typedef-basic-shape use crate::parser::{Parse, ParserContext}; -use crate::values::computed::Percentage; use crate::values::generics::basic_shape as generic; use crate::values::generics::basic_shape::{GeometryBox, Path, PolygonCoord}; use crate::values::generics::basic_shape::{ShapeBox, ShapeSource}; use crate::values::generics::rect::Rect; use crate::values::specified::border::BorderRadius; use crate::values::specified::image::Image; -use crate::values::specified::position::{HorizontalPosition, Position, PositionComponent}; -use crate::values::specified::position::{Side, VerticalPosition}; +use crate::values::specified::position::{HorizontalPosition, VerticalPosition, Position}; use crate::values::specified::url::SpecifiedUrl; use crate::values::specified::LengthOrPercentage; use crate::values::specified::SVGPathData; use cssparser::Parser; -use std::borrow::Cow; use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; @@ -249,7 +246,7 @@ impl ToCss for Circle { } dest.write_str("at ")?; - serialize_basicshape_position(&self.position, dest)?; + self.position.to_css(dest)?; dest.write_str(")") } } @@ -305,7 +302,7 @@ impl ToCss for Ellipse { } dest.write_str("at ")?; - serialize_basicshape_position(&self.position, dest)?; + self.position.to_css(dest)?; dest.write_str(")") } } @@ -326,82 +323,6 @@ impl Parse for ShapeRadius { } } -/// -/// -/// Positions get serialized differently with basic shapes. Keywords -/// are converted to percentages where possible. Only the two or four -/// value forms are used. In case of two keyword-percentage pairs, -/// the keywords are folded into the percentages -fn serialize_basicshape_position(position: &Position, dest: &mut CssWriter) -> fmt::Result -where - W: Write, -{ - fn to_keyword_and_lop(component: &PositionComponent) -> (S, Cow) - where - S: Copy + Side, - { - match *component { - PositionComponent::Center => ( - S::start(), - Cow::Owned(LengthOrPercentage::Percentage(Percentage(0.5))), - ), - PositionComponent::Side(keyword, None) => { - // left | top => 0% - // right | bottom => 100% - let p = if keyword.is_start() { 0. } else { 1. }; - ( - S::start(), - Cow::Owned(LengthOrPercentage::Percentage(Percentage(p))), - ) - }, - PositionComponent::Side(keyword, Some(ref lop)) if !keyword.is_start() => { - if let LengthOrPercentage::Percentage(p) = *to_non_zero_length(lop) { - ( - S::start(), - Cow::Owned(LengthOrPercentage::Percentage(Percentage(1. - p.0))), - ) - } else { - (keyword, Cow::Borrowed(lop)) - } - }, - PositionComponent::Length(ref lop) | PositionComponent::Side(_, Some(ref lop)) => { - (S::start(), to_non_zero_length(lop)) - }, - } - } - - fn to_non_zero_length(lop: &LengthOrPercentage) -> Cow { - match *lop { - LengthOrPercentage::Length(ref l) if l.is_zero() => { - Cow::Owned(LengthOrPercentage::Percentage(Percentage(0.))) - }, - _ => Cow::Borrowed(lop), - } - } - - fn write_pair(a: &A, b: &B, dest: &mut CssWriter) -> fmt::Result - where - A: ToCss, - B: ToCss, - W: Write, - { - a.to_css(dest)?; - dest.write_str(" ")?; - b.to_css(dest) - } - - let (x_pos, x_lop) = to_keyword_and_lop(&position.horizontal); - let (y_pos, y_lop) = to_keyword_and_lop(&position.vertical); - - if x_pos.is_start() && y_pos.is_start() { - return write_pair(&*x_lop, &*y_lop, dest); - } - - write_pair(&x_pos, &*x_lop, dest)?; - dest.write_str(" ")?; - write_pair(&y_pos, &*y_lop, dest) -} - impl Parse for Polygon { fn parse<'i, 't>( context: &ParserContext, From 0b9ecbccfeb15e0ee73f7bbf218550d6cfbc2d28 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Mon, 12 Nov 2018 23:17:19 +0000 Subject: [PATCH 08/23] style: Rewrite the interpolation of Rotate to return correct type. The original implementation always returns Rotate::Rotate3D, but it is not correct, so we have to rewrite it: 1. If both from value and to value are none, we don't have to convert it into identity value, so just return None. 2. If one of the value is none, we replace it with an identity value based on the other one's rotate axis. 3. If we only have 2D rotation, we just animate the . 4. Otherwise, we do interpolation by 3D rotation. Differential Revision: https://phabricator.services.mozilla.com/D11247 --- .../helpers/animated_properties.mako.rs | 83 +++++++++++-------- 1 file changed, 50 insertions(+), 33 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 5ff4a10c3c0..4efab0bc60d 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -2108,7 +2108,7 @@ impl ComputedRotate { // // If the axis is unspecified, it defaults to "0 0 1" match *self { - Rotate::None => (0., 0., 1., Angle::zero()), + Rotate::None => unreachable!("None is handled by the caller"), Rotate::Rotate3D(rx, ry, rz, angle) => (rx, ry, rz, angle), Rotate::Rotate(angle) => (0., 0., 1., angle), } @@ -2122,41 +2122,58 @@ impl Animate for ComputedRotate { other: &Self, procedure: Procedure, ) -> Result { - let (from, to) = (self.resolve(), other.resolve()); + match (self, other) { + (&Rotate::None, &Rotate::None) => Ok(Rotate::None), + (&Rotate::Rotate3D(fx, fy, fz, fa), &Rotate::None) => { + // No need to normalize `none`, so animate angle directly. + Ok(Rotate::Rotate3D(fx, fy, fz, fa.animate(&Angle::zero(), procedure)?)) + }, + (&Rotate::None, &Rotate::Rotate3D(tx, ty, tz, ta)) => { + // No need to normalize `none`, so animate angle directly. + Ok(Rotate::Rotate3D(tx, ty, tz, Angle::zero().animate(&ta, procedure)?)) + }, + (&Rotate::Rotate3D(_, ..), _) | (_, &Rotate::Rotate3D(_, ..)) => { + let (from, to) = (self.resolve(), other.resolve()); + let (mut fx, mut fy, mut fz, fa) = + transform::get_normalized_vector_and_angle(from.0, from.1, from.2, from.3); + let (mut tx, mut ty, mut tz, ta) = + transform::get_normalized_vector_and_angle(to.0, to.1, to.2, to.3); - let (mut fx, mut fy, mut fz, fa) = - transform::get_normalized_vector_and_angle(from.0, from.1, from.2, from.3); - let (mut tx, mut ty, mut tz, ta) = - transform::get_normalized_vector_and_angle(to.0, to.1, to.2, to.3); + if fa == Angle::from_degrees(0.) { + fx = tx; + fy = ty; + fz = tz; + } else if ta == Angle::from_degrees(0.) { + tx = fx; + ty = fy; + tz = fz; + } - if fa == Angle::from_degrees(0.) { - fx = tx; - fy = ty; - fz = tz; - } else if ta == Angle::from_degrees(0.) { - tx = fx; - ty = fy; - tz = fz; + if (fx, fy, fz) == (tx, ty, tz) { + return Ok(Rotate::Rotate3D(fx, fy, fz, fa.animate(&ta, procedure)?)); + } + + let fv = DirectionVector::new(fx, fy, fz); + let tv = DirectionVector::new(tx, ty, tz); + let fq = Quaternion::from_direction_and_angle(&fv, fa.radians64()); + let tq = Quaternion::from_direction_and_angle(&tv, ta.radians64()); + + let rq = Quaternion::animate(&fq, &tq, procedure)?; + let (x, y, z, angle) = transform::get_normalized_vector_and_angle( + rq.0 as f32, + rq.1 as f32, + rq.2 as f32, + rq.3.acos() as f32 * 2.0, + ); + + Ok(Rotate::Rotate3D(x, y, z, Angle::from_radians(angle))) + }, + (&Rotate::Rotate(_), _) | (_, &Rotate::Rotate(_)) => { + // If this is a 2D rotation, we just animate the + let (from, to) = (self.resolve().3, other.resolve().3); + Ok(Rotate::Rotate(from.animate(&to, procedure)?)) + }, } - - if (fx, fy, fz) == (tx, ty, tz) { - return Ok(Rotate::Rotate3D(fx, fy, fz, fa.animate(&ta, procedure)?)); - } - - let fv = DirectionVector::new(fx, fy, fz); - let tv = DirectionVector::new(tx, ty, tz); - let fq = Quaternion::from_direction_and_angle(&fv, fa.radians64()); - let tq = Quaternion::from_direction_and_angle(&tv, ta.radians64()); - - let rq = Quaternion::animate(&fq, &tq, procedure)?; - let (x, y, z, angle) = transform::get_normalized_vector_and_angle( - rq.0 as f32, - rq.1 as f32, - rq.2 as f32, - rq.3.acos() as f32 * 2.0, - ); - - Ok(Rotate::Rotate3D(x, y, z, Angle::from_radians(angle))) } } From d8bd29292e32f8398d90ce942d23c363f9adef43 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Mon, 12 Nov 2018 19:21:32 +0000 Subject: [PATCH 09/23] style: Parse any order of number and angle for Rotate. Rotate accepts rotate axis and angle in any order (i.e. {3} or {3}), so we rewrite the parser. Differential Revision: https://phabricator.services.mozilla.com/D11417 --- .../style/values/specified/transform.rs | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs index c1ffe202a79..38ac07edf63 100644 --- a/components/style/values/specified/transform.rs +++ b/components/style/values/specified/transform.rs @@ -357,17 +357,28 @@ impl Parse for Rotate { return Ok(generic::Rotate::None); } - if let Ok(rx) = input.try(|i| Number::parse(context, i)) { - // 'rotate: {3} ' - let ry = Number::parse(context, input)?; - let rz = Number::parse(context, input)?; - let angle = specified::Angle::parse(context, input)?; - return Ok(generic::Rotate::Rotate3D(rx, ry, rz, angle)); - } + // Parse or [ x | y | z | {3} ] && . + // TODO: Bug 1504327: Parse [x|y|z] keywords. + // + // The rotate axis and angle could be in any order, so we parse angle twice to cover + // two cases. i.e. `{3} ` or ` {3}` + let angle = input.try(|i| specified::Angle::parse(context, i)).ok(); + let axis = input.try(|i| -> Result<_, ParseError> { + Ok(( + Number::parse(context, i)?, + Number::parse(context, i)?, + Number::parse(context, i)?, + )) + }).ok(); + let angle = match angle { + Some(a) => a, + None => specified::Angle::parse(context, input)?, + }; - // 'rotate: ' - let angle = specified::Angle::parse(context, input)?; - Ok(generic::Rotate::Rotate(angle)) + Ok(match axis { + Some((x, y, z)) => generic::Rotate::Rotate3D(x, y, z, angle), + None => generic::Rotate::Rotate(angle), + }) } } From f486ef7e47b7262a7ab0146f510aa7a3de602213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 13 Nov 2018 12:47:40 +0000 Subject: [PATCH 10/23] style: Add an atom bit to know whether we're ascii lowercase. And thus massively speed up ascii-case-insensitive atom comparisons when both atoms are lowercase (which is the common case by far). This removes almost all the slow selector-matching in this page, and it seems an easier fix than storing the lowercased version of all class-names in quirks mode in elements and selectors... Differential Revision: https://phabricator.services.mozilla.com/D10945 --- components/style/gecko/regen_atoms.py | 4 +- components/style/gecko_string_cache/mod.rs | 78 +++++++++++++--------- 2 files changed, 47 insertions(+), 35 deletions(-) diff --git a/components/style/gecko/regen_atoms.py b/components/style/gecko/regen_atoms.py index 36ec0f70434..18c28a13b49 100755 --- a/components/style/gecko/regen_atoms.py +++ b/components/style/gecko/regen_atoms.py @@ -16,8 +16,8 @@ sys.path.insert(0, os.path.join(os.path.dirname(GECKO_DIR), "properties")) import build -# Matches lines like `GK_ATOM(foo, "foo", 0x12345678, nsStaticAtom, PseudoElementAtom)`. -PATTERN = re.compile('^GK_ATOM\(([^,]*),[^"]*"([^"]*)",\s*(0x[0-9a-f]+),\s*([^,]*),\s*([^)]*)\)', +# Matches lines like `GK_ATOM(foo, "foo", 0x12345678, true, nsStaticAtom, PseudoElementAtom)`. +PATTERN = re.compile('^GK_ATOM\(([^,]*),[^"]*"([^"]*)",\s*(0x[0-9a-f]+),\s*[^,]*,\s*([^,]*),\s*([^)]*)\)', re.MULTILINE) FILE = "include/nsGkAtomList.h" diff --git a/components/style/gecko_string_cache/mod.rs b/components/style/gecko_string_cache/mod.rs index 6d9e5c60f80..ed0cc19dae2 100644 --- a/components/style/gecko_string_cache/mod.rs +++ b/components/style/gecko_string_cache/mod.rs @@ -175,13 +175,19 @@ impl WeakAtom { /// Returns whether this atom is static. #[inline] pub fn is_static(&self) -> bool { - unsafe { (*self.as_ptr()).mIsStatic() != 0 } + self.0.mIsStatic() != 0 + } + + /// Returns whether this atom is ascii lowercase. + #[inline] + fn is_ascii_lowercase(&self) -> bool { + self.0.mIsAsciiLowercase() != 0 } /// Returns the length of the atom string. #[inline] pub fn len(&self) -> u32 { - unsafe { (*self.as_ptr()).mLength() } + self.0.mLength() } /// Returns whether this atom is the empty string. @@ -199,41 +205,54 @@ impl WeakAtom { /// Convert this atom to ASCII lower-case pub fn to_ascii_lowercase(&self) -> Atom { - let slice = self.as_slice(); - match slice - .iter() - .position(|&char16| (b'A' as u16) <= char16 && char16 <= (b'Z' as u16)) - { - None => self.clone(), - Some(i) => { - let mut buffer: [u16; 64] = unsafe { mem::uninitialized() }; - let mut vec; - let mutable_slice = if let Some(buffer_prefix) = buffer.get_mut(..slice.len()) { - buffer_prefix.copy_from_slice(slice); - buffer_prefix - } else { - vec = slice.to_vec(); - &mut vec - }; - for char16 in &mut mutable_slice[i..] { - if *char16 <= 0x7F { - *char16 = (*char16 as u8).to_ascii_lowercase() as u16 - } - } - Atom::from(&*mutable_slice) - }, + if self.is_ascii_lowercase() { + return self.clone(); } + + let slice = self.as_slice(); + let mut buffer: [u16; 64] = unsafe { mem::uninitialized() }; + let mut vec; + let mutable_slice = if let Some(buffer_prefix) = buffer.get_mut(..slice.len()) { + buffer_prefix.copy_from_slice(slice); + buffer_prefix + } else { + vec = slice.to_vec(); + &mut vec + }; + for char16 in &mut *mutable_slice { + if *char16 <= 0x7F { + *char16 = (*char16 as u8).to_ascii_lowercase() as u16 + } + } + Atom::from(&*mutable_slice) } /// Return whether two atoms are ASCII-case-insensitive matches + #[inline] pub fn eq_ignore_ascii_case(&self, other: &Self) -> bool { if self == other { return true; } + // If we know both atoms are ascii-lowercase, then we can stick with + // pointer equality. + if self.is_ascii_lowercase() && other.is_ascii_lowercase() { + debug_assert!(!self.eq_ignore_ascii_case_slow(other)); + return false; + } + + self.eq_ignore_ascii_case_slow(other) + } + + fn eq_ignore_ascii_case_slow(&self, other: &Self) -> bool { let a = self.as_slice(); let b = other.as_slice(); - a.len() == b.len() && a.iter().zip(b).all(|(&a16, &b16)| { + + if a.len() != b.len() { + return false; + } + + a.iter().zip(b).all(|(&a16, &b16)| { if a16 <= 0x7F && b16 <= 0x7F { (a16 as u8).eq_ignore_ascii_case(&(b16 as u8)) } else { @@ -241,13 +260,6 @@ impl WeakAtom { } }) } - - /// Return whether this atom is an ASCII-case-insensitive match for the given string - pub fn eq_str_ignore_ascii_case(&self, other: &str) -> bool { - self.chars() - .map(|r| r.map(|c: char| c.to_ascii_lowercase())) - .eq(other.chars().map(|c: char| Ok(c.to_ascii_lowercase()))) - } } impl fmt::Debug for WeakAtom { From 41d2f7f3a2766952826d708aa542fc244ea91cd9 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Tue, 13 Nov 2018 18:37:14 +0000 Subject: [PATCH 11/23] style: Support keywords [x|y|z] on rotate. Update the parser and the serialization to support the keywords, [x|y|z]. Differential Revision: https://phabricator.services.mozilla.com/D11531 --- components/style/values/computed/mod.rs | 12 +++++ components/style/values/generics/transform.rs | 48 ++++++++++++++++++- components/style/values/specified/mod.rs | 12 +++++ .../style/values/specified/transform.rs | 21 +++++--- 4 files changed, 85 insertions(+), 8 deletions(-) diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index b8484a291ab..75c95e1a956 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -8,6 +8,7 @@ use super::animated::ToAnimatedValue; use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent; use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth}; use super::generics::grid::{TrackList as GenericTrackList, TrackSize as GenericTrackSize}; +use super::generics::transform::IsParallelTo; use super::generics::{GreaterThanOrEqualToOne, NonNegative}; use super::specified; use super::{CSSFloat, CSSInteger}; @@ -22,6 +23,7 @@ use crate::Atom; #[cfg(feature = "servo")] use crate::Prefix; use euclid::Size2D; +use self::transform::DirectionVector; use std::cell::RefCell; use std::cmp; use std::f32; @@ -459,6 +461,16 @@ trivial_to_computed_value!(Box); /// A `` value. pub type Number = CSSFloat; +impl IsParallelTo for (Number, Number, Number) { + fn is_parallel_to(&self, vector: &DirectionVector) -> bool { + use euclid::approxeq::ApproxEq; + // If a and b is parallel, the angle between them is 0deg, so + // a x b = |a|*|b|*sin(0)*n = 0 * n, |a x b| == 0. + let self_vector = DirectionVector::new(self.0, self.1, self.2); + self_vector.cross(*vector).square_length().approx_eq(&0.0f32) + } +} + /// A wrapper of Number, but the value >= 0. pub type NonNegativeNumber = NonNegative; diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index 12cc97fb10f..ef54c511f63 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -538,7 +538,6 @@ pub fn get_normalized_vector_and_angle( SpecifiedValueInfo, ToAnimatedZero, ToComputedValue, - ToCss, )] /// A value of the `Rotate` property /// @@ -552,6 +551,53 @@ pub enum Rotate { Rotate3D(Number, Number, Number, Angle), } +/// A trait to check if the current 3D vector is parallel to the DirectionVector. +/// This is especially for serialization on Rotate. +pub trait IsParallelTo { + /// Returns true if this is parallel to the vector. + fn is_parallel_to(&self, vector: &computed::transform::DirectionVector) -> bool; +} + +impl ToCss for Rotate +where + Number: Copy + ToCss, + Angle: ToCss, + (Number, Number, Number): IsParallelTo, +{ + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: fmt::Write, + { + use crate::values::computed::transform::DirectionVector; + match *self { + Rotate::None => dest.write_str("none"), + Rotate::Rotate(ref angle) => angle.to_css(dest), + Rotate::Rotate3D(x, y, z, ref angle) => { + // If a 3d rotation is specified, the property must serialize with an axis + // specified. If the axis is parallel with the x, y, or z axises, it must + // serialize as the appropriate keyword. + // https://drafts.csswg.org/css-transforms-2/#individual-transform-serialization + let v = (x, y, z); + if v.is_parallel_to(&DirectionVector::new(1., 0., 0.)) { + dest.write_char('x')?; + } else if v.is_parallel_to(&DirectionVector::new(0., 1., 0.)) { + dest.write_char('y')?; + } else if v.is_parallel_to(&DirectionVector::new(0., 0., 1.)) { + dest.write_char('z')?; + } else { + x.to_css(dest)?; + dest.write_char(' ')?; + y.to_css(dest)?; + dest.write_char(' ')?; + z.to_css(dest)?; + } + dest.write_char(' ')?; + angle.to_css(dest) + }, + } + } +} + #[derive( Clone, ComputeSquaredDistance, diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index bcca8ed8f49..22bdc233e5d 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -6,9 +6,11 @@ //! //! TODO(emilio): Enhance docs. +use super::computed::transform::DirectionVector; use super::computed::{Context, ToComputedValue}; use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth}; use super::generics::grid::{TrackList as GenericTrackList, TrackSize as GenericTrackSize}; +use super::generics::transform::IsParallelTo; use super::generics::{GreaterThanOrEqualToOne, NonNegative}; use super::{Auto, CSSFloat, CSSInteger, Either}; use crate::context::QuirksMode; @@ -292,6 +294,16 @@ impl ToCss for Number { } } +impl IsParallelTo for (Number, Number, Number) { + fn is_parallel_to(&self, vector: &DirectionVector) -> bool { + use euclid::approxeq::ApproxEq; + // If a and b is parallel, the angle between them is 0deg, so + // a x b = |a|*|b|*sin(0)*n = 0 * n, |a x b| == 0. + let self_vector = DirectionVector::new(self.0.get(), self.1.get(), self.2.get()); + self_vector.cross(*vector).square_length().approx_eq(&0.0f32) + } +} + impl SpecifiedValueInfo for Number {} impl From for f32 { diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs index 38ac07edf63..5f455c733de 100644 --- a/components/style/values/specified/transform.rs +++ b/components/style/values/specified/transform.rs @@ -358,17 +358,24 @@ impl Parse for Rotate { } // Parse or [ x | y | z | {3} ] && . - // TODO: Bug 1504327: Parse [x|y|z] keywords. // // The rotate axis and angle could be in any order, so we parse angle twice to cover // two cases. i.e. `{3} ` or ` {3}` let angle = input.try(|i| specified::Angle::parse(context, i)).ok(); - let axis = input.try(|i| -> Result<_, ParseError> { - Ok(( - Number::parse(context, i)?, - Number::parse(context, i)?, - Number::parse(context, i)?, - )) + let axis = input.try(|i| { + Ok(try_match_ident_ignore_ascii_case! { i, + "x" => (Number::new(1.), Number::new(0.), Number::new(0.)), + "y" => (Number::new(0.), Number::new(1.), Number::new(0.)), + "z" => (Number::new(0.), Number::new(0.), Number::new(1.)), + }) + }).or_else(|_: ParseError| -> Result<_, ParseError> { + input.try(|i| { + Ok(( + Number::parse(context, i)?, + Number::parse(context, i)?, + Number::parse(context, i)?, + )) + }) }).ok(); let angle = match angle { Some(a) => a, From 1576dd4c5b8d986620f8d882cb37485b2a5f1725 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Tue, 13 Nov 2018 12:51:05 +0000 Subject: [PATCH 12/23] style: Store variable references with a boxed slice rather than a PrecomputedHashSet. Once we've parsed the variable references, there is no need to keep an entire HashSet object around, as all we do is iterate over the values. Differential Revision: https://phabricator.services.mozilla.com/D11735 --- components/style/custom_properties.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 31c3dc207f5..c1d337ac2fa 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -109,7 +109,7 @@ pub struct VariableValue { references_environment: bool, /// Custom property names in var() functions. - references: PrecomputedHashSet, + references: Box<[Name]>, } impl ToCss for SpecifiedValue { @@ -278,7 +278,7 @@ impl VariableValue { css: String::new(), last_token_type: TokenSerializationType::nothing(), first_token_type: TokenSerializationType::nothing(), - references: PrecomputedHashSet::default(), + references: Default::default(), references_environment: false, } } @@ -335,11 +335,16 @@ impl VariableValue { let (first_token_type, css, last_token_type) = parse_self_contained_declaration_value(input, Some(&mut references))?; + let custom_property_references = references.custom_property_references + .into_iter() + .collect::>() + .into_boxed_slice(); + Ok(Arc::new(VariableValue { css: css.into_owned(), first_token_type, last_token_type, - references: references.custom_property_references, + references: custom_property_references, references_environment: references.references_environment, })) } From dad9ad6b31205ddd11733e674d2c569dbfe40694 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Wed, 14 Nov 2018 21:33:01 +0000 Subject: [PATCH 13/23] style: Store MediaFeatureDescription references as an index into the MEDIA_FEATURES array. The current use of a static reference is incompatible with sharing style sheet data across processes. Differential Revision: https://phabricator.services.mozilla.com/D11845 --- .../media_queries/media_feature_expression.rs | 45 +++++++++++-------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/components/style/media_queries/media_feature_expression.rs b/components/style/media_queries/media_feature_expression.rs index b6bfc03179d..122b7ca05ff 100644 --- a/components/style/media_queries/media_feature_expression.rs +++ b/components/style/media_queries/media_feature_expression.rs @@ -10,8 +10,12 @@ use super::media_feature::{KeywordDiscriminant, ParsingRequirements}; use super::Device; use crate::context::QuirksMode; #[cfg(feature = "gecko")] +use crate::gecko::media_features::MEDIA_FEATURES; +#[cfg(feature = "gecko")] use crate::gecko_bindings::structs; use crate::parser::{Parse, ParserContext}; +#[cfg(feature = "servo")] +use crate::servo::media_queries::MEDIA_FEATURES; use crate::str::{starts_with_ignore_ascii_case, string_as_ascii_lowercase}; use crate::stylesheets::Origin; use crate::values::computed::{self, ToComputedValue}; @@ -150,14 +154,14 @@ impl RangeOrOperator { /// the media query contained, and the range to evaluate. #[derive(Clone, Debug, MallocSizeOf)] pub struct MediaFeatureExpression { - feature: &'static MediaFeatureDescription, + feature_index: usize, value: Option, range_or_operator: Option, } impl PartialEq for MediaFeatureExpression { fn eq(&self, other: &Self) -> bool { - self.feature as *const _ == other.feature as *const _ && + self.feature_index == other.feature_index && self.value == other.value && self.range_or_operator == other.range_or_operator } @@ -170,8 +174,9 @@ impl ToCss for MediaFeatureExpression { { dest.write_str("(")?; - if self - .feature + let feature = self.feature(); + + if feature .requirements .contains(ParsingRequirements::WEBKIT_PREFIX) { @@ -186,7 +191,7 @@ impl ToCss for MediaFeatureExpression { } // NB: CssStringWriter not needed, feature names are under control. - write!(dest, "{}", self.feature.name)?; + write!(dest, "{}", feature.name)?; if let Some(RangeOrOperator::Operator(op)) = self.range_or_operator { dest.write_char(' ')?; @@ -240,17 +245,22 @@ fn consume_operation_or_colon(input: &mut Parser) -> Result, () impl MediaFeatureExpression { fn new( - feature: &'static MediaFeatureDescription, + feature_index: usize, value: Option, range_or_operator: Option, ) -> Self { + debug_assert!(feature_index < MEDIA_FEATURES.len()); Self { - feature, + feature_index, value, range_or_operator, } } + fn feature(&self) -> &'static MediaFeatureDescription { + &MEDIA_FEATURES[self.feature_index] + } + /// Parse a media expression of the form: /// /// ``` @@ -270,12 +280,8 @@ impl MediaFeatureExpression { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - #[cfg(feature = "gecko")] - use crate::gecko::media_features::MEDIA_FEATURES; - #[cfg(feature = "servo")] - use crate::servo::media_queries::MEDIA_FEATURES; - // FIXME: remove extra indented block when lifetimes are non-lexical + let feature_index; let feature; let range; { @@ -319,14 +325,15 @@ impl MediaFeatureExpression { }; let atom = Atom::from(string_as_ascii_lowercase(feature_name)); - match MEDIA_FEATURES.iter().find(|f| f.name == atom) { - Some(f) => Ok((f, range)), + match MEDIA_FEATURES.iter().enumerate().find(|(_, f)| f.name == atom) { + Some((i, f)) => Ok((i, f, range)), None => Err(()), } }; match result { - Ok((f, r)) => { + Ok((i, f, r)) => { + feature_index = i; feature = f; range = r; }, @@ -365,7 +372,7 @@ impl MediaFeatureExpression { ); } - return Ok(Self::new(feature, None, None)); + return Ok(Self::new(feature_index, None, None)); }, Ok(operator) => operator, }; @@ -396,7 +403,7 @@ impl MediaFeatureExpression { .new_custom_error(StyleParseErrorKind::MediaQueryExpectedFeatureValue) })?; - Ok(Self::new(feature, Some(value), range_or_operator)) + Ok(Self::new(feature_index, Some(value), range_or_operator)) } /// Returns whether this media query evaluates to true for the given device. @@ -412,7 +419,7 @@ impl MediaFeatureExpression { }; } - match self.feature.evaluator { + match self.feature().evaluator { Evaluator::Length(eval) => { let computed = expect!(Length).map(|specified| { computed::Context::for_media_query_evaluation(device, quirks_mode, |context| { @@ -492,7 +499,7 @@ impl MediaExpressionValue { MediaExpressionValue::IntRatio(ratio) => ratio.to_css(dest), MediaExpressionValue::Resolution(ref r) => r.to_css(dest), MediaExpressionValue::Ident(ref ident) => serialize_atom_identifier(ident, dest), - MediaExpressionValue::Enumerated(value) => match for_expr.feature.evaluator { + MediaExpressionValue::Enumerated(value) => match for_expr.feature().evaluator { Evaluator::Enumerated { serializer, .. } => dest.write_str(&*serializer(value)), _ => unreachable!(), }, From 3977854ee28b580d6aeeb38fe949b3431e671f00 Mon Sep 17 00:00:00 2001 From: Timothy Guan-tin Chien Date: Thu, 15 Nov 2018 06:51:07 +0000 Subject: [PATCH 14/23] Remove dom.webcomponents.shadowdom.enabled. This patch removes the dom.webcomponents.shadowdom.enabled pref and all its references, including the following functions: * nsContentUtils::IsShadowDOMEnabled() * nsIDocument::IsShadowDOMEnabled() * nsDocument::IsShadowDOMEnabled(JSContext* aCx, JSObject* aGlobal) * nsDocument::IsShadowDOMEnabled(const nsINode* aNode) * nsTextNode::IsShadowDOMEnabled(JSContext* aCx, JSObject* aObject) This function is renamed and updated to nsDocument::IsCallerChromeOrAddon(): * nsDocument::IsShadowDOMEnabledAndCallerIsChromeOrAddon(JSContext* aCx, JSObject* aObject) I didn't change the tests that load Shadow DOM tests in an iframe, in the interest of keeping hg annotation history. Differential Revision: https://phabricator.services.mozilla.com/D11183 --- components/style/gecko/selector_parser.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index b8fd780b716..039c069d2bb 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -347,10 +347,7 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> { #[inline] fn parse_slotted(&self) -> bool { - // NOTE(emilio): Slot assignment and such works per-document, but - // getting a document around here is not trivial, and it's not worth - // anyway to handle this in a per-doc basis. - unsafe { structs::nsContentUtils_sIsShadowDOMEnabled } + true } #[inline] From 9f28242def7718a5c3e04a61093890d504743010 Mon Sep 17 00:00:00 2001 From: Timothy Guan-tin Chien Date: Thu, 15 Nov 2018 06:52:01 +0000 Subject: [PATCH 15/23] Remove dom.webcomponents.customelements.enabled pref. This patch removes the following functions: * nsContentUtils::IsCustomElementsEnabled() * CustomElementRegistry::IsCustomElementEnabled(JSContext* aCx, JSObject* aObject) * CustomElementRegistry::IsCustomElementEnabled(nsIDocument* aDoc) and all references of the pref. Depends on D11183 Differential Revision: https://phabricator.services.mozilla.com/D11249 --- components/style/gecko/non_ts_pseudo_class_list.rs | 2 +- components/style/gecko/selector_parser.rs | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/components/style/gecko/non_ts_pseudo_class_list.rs b/components/style/gecko/non_ts_pseudo_class_list.rs index f7d8be418de..cf21cbf8999 100644 --- a/components/style/gecko/non_ts_pseudo_class_list.rs +++ b/components/style/gecko/non_ts_pseudo_class_list.rs @@ -37,7 +37,7 @@ macro_rules! apply_non_ts_list { ("visited", Visited, visited, IN_VISITED_STATE, _), ("active", Active, active, IN_ACTIVE_STATE, _), ("checked", Checked, checked, IN_CHECKED_STATE, _), - ("defined", Defined, defined, IN_DEFINED_STATE, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS_AND_CHROME), + ("defined", Defined, defined, IN_DEFINED_STATE, _), ("disabled", Disabled, disabled, IN_DISABLED_STATE, _), ("enabled", Enabled, enabled, IN_ENABLED_STATE, _), ("focus", Focus, focus, IN_FOCUS_STATE, _), diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index 039c069d2bb..1871d60a0b2 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -178,9 +178,6 @@ impl NonTSPseudoClass { NonTSPseudoClass::Fullscreen => unsafe { mozilla::StaticPrefs_sVarCache_full_screen_api_unprefix_enabled }, - NonTSPseudoClass::Defined => unsafe { - structs::nsContentUtils_sIsCustomElementsEnabled - }, // Otherwise, a pseudo-class is enabled in content when it // doesn't have any enabled flag. _ => !self From d9de68ad6a703b60215b64f40f90ddd19d86797a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 15 Nov 2018 08:15:13 +0000 Subject: [PATCH 16/23] style: Move the page-break-{before,after} properties to not use mako. And respect the computed value of `left` / `right` / etc. Differential Revision: https://phabricator.services.mozilla.com/D11872 --- components/style/cbindgen.toml | 1 + components/style/properties/data.py | 1 + components/style/properties/gecko.mako.rs | 33 ++----------------- .../style/properties/longhands/box.mako.rs | 13 +++++--- components/style/values/computed/box.rs | 10 +++--- components/style/values/computed/mod.rs | 2 +- components/style/values/specified/box.rs | 26 +++++++++++++++ components/style/values/specified/mod.rs | 2 +- 8 files changed, 44 insertions(+), 44 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index d9efc9d9e18..098022cf13d 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -39,6 +39,7 @@ derive_helper_methods = true prefix = "Style" include = [ "Appearance", + "BreakBetween", "ComputedFontStretchRange", "ComputedFontStyleDescriptor", "ComputedFontWeightRange", diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 97e2f7f0d84..c0be33177f2 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -291,6 +291,7 @@ class Longhand(object): "AlignItems", "AlignSelf", "Appearance", + "BreakBetween", "BackgroundRepeat", "BorderImageRepeat", "BorderStyle", diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 2cc8cf2bcd3..15d4c40773b 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1396,6 +1396,7 @@ impl Clone for ${style_struct.gecko_struct_name} { # Types used with predefined_type()-defined properties that we can auto-generate. predefined_types = { + "BreakBetween": impl_simple, "Color": impl_color, "ColorOrAuto": impl_color, "GreaterThanOrEqualToOneNumber": impl_simple, @@ -3029,8 +3030,7 @@ fn static_assert() { animation-iteration-count animation-timing-function transition-duration transition-delay transition-timing-function transition-property - page-break-before page-break-after rotate - scroll-snap-points-x scroll-snap-points-y + rotate scroll-snap-points-x scroll-snap-points-y scroll-snap-type-x scroll-snap-type-y scroll-snap-coordinate perspective-origin -moz-binding will-change offset-path overscroll-behavior-x overscroll-behavior-y @@ -3149,35 +3149,6 @@ fn static_assert() { <%call expr="impl_coord_copy('vertical_align', 'mVerticalAlign')"> - % for kind in ["before", "after"]: - // Temp fix for Bugzilla bug 24000. - // Map 'auto' and 'avoid' to false, and 'always', 'left', and 'right' to true. - // "A conforming user agent may interpret the values 'left' and 'right' - // as 'always'." - CSS2.1, section 13.3.1 - pub fn set_page_break_${kind}(&mut self, v: longhands::page_break_${kind}::computed_value::T) { - use crate::computed_values::page_break_${kind}::T; - - let result = match v { - T::Auto => false, - T::Always => true, - T::Avoid => false, - T::Left => true, - T::Right => true - }; - self.gecko.mBreak${kind.title()} = result; - } - - ${impl_simple_copy('page_break_' + kind, 'mBreak' + kind.title())} - - // Temp fix for Bugzilla bug 24000. - // See set_page_break_before/after for detail. - pub fn clone_page_break_${kind}(&self) -> longhands::page_break_${kind}::computed_value::T { - use crate::computed_values::page_break_${kind}::T; - - if self.gecko.mBreak${kind.title()} { T::Always } else { T::Auto } - } - % endfor - ${impl_style_coord("scroll_snap_points_x", "mScrollSnapPointsX")} ${impl_style_coord("scroll_snap_points_y", "mScrollSnapPointsY")} diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index 685e97aa3b0..da0e919f784 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -437,18 +437,21 @@ ${helpers.single_keyword( animation_value_type="discrete", )} -// TODO add support for logical values recto and verso -${helpers.single_keyword( +${helpers.predefined_type( "page-break-after", - "auto always avoid left right", + "BreakBetween", + "computed::BreakBetween::Auto", + needs_context=False, products="gecko", spec="https://drafts.csswg.org/css2/page.html#propdef-page-break-after", animation_value_type="discrete", )} -${helpers.single_keyword( +${helpers.predefined_type( "page-break-before", - "auto always avoid left right", + "BreakBetween", + "computed::BreakBetween::Auto", + needs_context=False, products="gecko", spec="https://drafts.csswg.org/css2/page.html#propdef-page-break-before", animation_value_type="discrete", diff --git a/components/style/values/computed/box.rs b/components/style/values/computed/box.rs index 0cf6ab3e39c..dd168b72b39 100644 --- a/components/style/values/computed/box.rs +++ b/components/style/values/computed/box.rs @@ -11,13 +11,11 @@ use crate::values::generics::box_::Perspective as GenericPerspective; use crate::values::generics::box_::VerticalAlign as GenericVerticalAlign; use crate::values::specified::box_ as specified; -pub use crate::values::specified::box_::{ - AnimationName, Appearance, Contain, Display, OverflowClipBox, -}; +pub use crate::values::specified::box_::{AnimationName, Appearance, BreakBetween}; +pub use crate::values::specified::box_::{Contain, Display, OverflowClipBox}; pub use crate::values::specified::box_::{Clear as SpecifiedClear, Float as SpecifiedFloat}; -pub use crate::values::specified::box_::{ - OverscrollBehavior, ScrollSnapType, TouchAction, TransitionProperty, WillChange, -}; +pub use crate::values::specified::box_::{OverscrollBehavior, ScrollSnapType}; +pub use crate::values::specified::box_::{TouchAction, TransitionProperty, WillChange}; /// A computed value for the `vertical-align` property. pub type VerticalAlign = GenericVerticalAlign; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 75c95e1a956..3f3b29189ab 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -42,7 +42,7 @@ pub use self::border::{BorderCornerRadius, BorderRadius, BorderSpacing}; pub use self::border::{BorderImageRepeat, BorderImageSideWidth}; pub use self::border::{BorderImageSlice, BorderImageWidth}; pub use self::box_::{AnimationIterationCount, AnimationName, Contain}; -pub use self::box_::{Appearance, Clear, Float}; +pub use self::box_::{Appearance, BreakBetween, Clear, Float}; pub use self::box_::{Display, TransitionProperty}; pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize}; pub use self::box_::{ScrollSnapType, TouchAction, VerticalAlign, WillChange}; diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 6a207d0a90c..8d0c9f38be9 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -1289,3 +1289,29 @@ pub enum Appearance { #[css(skip)] Count, } + +/// A kind of break between two boxes. +/// +/// https://drafts.csswg.org/css-break/#break-between +#[allow(missing_docs)] +#[derive( + Clone, + Copy, + Debug, + Eq, + Hash, + MallocSizeOf, + Parse, + PartialEq, + SpecifiedValueInfo, + ToCss, + ToComputedValue, +)] +#[repr(u8)] +pub enum BreakBetween { + Auto, + Always, + Avoid, + Left, + Right, +} diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 22bdc233e5d..6fa8dbb4101 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -36,7 +36,7 @@ pub use self::border::{BorderCornerRadius, BorderImageSlice, BorderImageWidth}; pub use self::border::{BorderImageRepeat, BorderImageSideWidth}; pub use self::border::{BorderRadius, BorderSideWidth, BorderSpacing}; pub use self::box_::{AnimationIterationCount, AnimationName, Contain, Display}; -pub use self::box_::{Appearance, Clear, Float}; +pub use self::box_::{Appearance, BreakBetween, Clear, Float}; pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize}; pub use self::box_::{ScrollSnapType, TouchAction, TransitionProperty, VerticalAlign, WillChange}; pub use self::color::{Color, ColorPropertyValue, RGBAColor}; From a5f0eb9205edf154dcb50419398a87a459058fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 15 Nov 2018 08:16:23 +0000 Subject: [PATCH 17/23] style: Also move page-break-inside outside of mako. Differential Revision: https://phabricator.services.mozilla.com/D11876 --- components/style/cbindgen.toml | 1 + components/style/properties/data.py | 1 + components/style/properties/gecko.mako.rs | 1 + .../style/properties/longhands/box.mako.rs | 9 ++++---- components/style/values/computed/box.rs | 2 +- components/style/values/computed/mod.rs | 2 +- components/style/values/specified/box.rs | 23 +++++++++++++++++++ components/style/values/specified/mod.rs | 2 +- 8 files changed, 34 insertions(+), 7 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 098022cf13d..84bf2eb4301 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -40,6 +40,7 @@ prefix = "Style" include = [ "Appearance", "BreakBetween", + "BreakWithin", "ComputedFontStretchRange", "ComputedFontStyleDescriptor", "ComputedFontWeightRange", diff --git a/components/style/properties/data.py b/components/style/properties/data.py index c0be33177f2..22d5337fe67 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -292,6 +292,7 @@ class Longhand(object): "AlignSelf", "Appearance", "BreakBetween", + "BreakWithin", "BackgroundRepeat", "BorderImageRepeat", "BorderStyle", diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 15d4c40773b..f0b8cef22ac 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1397,6 +1397,7 @@ impl Clone for ${style_struct.gecko_struct_name} { # Types used with predefined_type()-defined properties that we can auto-generate. predefined_types = { "BreakBetween": impl_simple, + "BreakWithin": impl_simple, "Color": impl_color, "ColorOrAuto": impl_color, "GreaterThanOrEqualToOneNumber": impl_simple, diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index da0e919f784..f83b7928dc9 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -457,12 +457,13 @@ ${helpers.predefined_type( animation_value_type="discrete", )} -${helpers.single_keyword( +${helpers.predefined_type( "page-break-inside", - "auto avoid", - products="gecko", + "BreakWithin", + "computed::BreakWithin::Auto", gecko_ffi_name="mBreakInside", - gecko_constant_prefix="NS_STYLE_PAGE_BREAK", + needs_context=False, + products="gecko", spec="https://drafts.csswg.org/css2/page.html#propdef-page-break-inside", animation_value_type="discrete", )} diff --git a/components/style/values/computed/box.rs b/components/style/values/computed/box.rs index dd168b72b39..1af0efff85c 100644 --- a/components/style/values/computed/box.rs +++ b/components/style/values/computed/box.rs @@ -11,7 +11,7 @@ use crate::values::generics::box_::Perspective as GenericPerspective; use crate::values::generics::box_::VerticalAlign as GenericVerticalAlign; use crate::values::specified::box_ as specified; -pub use crate::values::specified::box_::{AnimationName, Appearance, BreakBetween}; +pub use crate::values::specified::box_::{AnimationName, Appearance, BreakBetween, BreakWithin}; pub use crate::values::specified::box_::{Contain, Display, OverflowClipBox}; pub use crate::values::specified::box_::{Clear as SpecifiedClear, Float as SpecifiedFloat}; pub use crate::values::specified::box_::{OverscrollBehavior, ScrollSnapType}; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 3f3b29189ab..cae5519f2e7 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -42,7 +42,7 @@ pub use self::border::{BorderCornerRadius, BorderRadius, BorderSpacing}; pub use self::border::{BorderImageRepeat, BorderImageSideWidth}; pub use self::border::{BorderImageSlice, BorderImageWidth}; pub use self::box_::{AnimationIterationCount, AnimationName, Contain}; -pub use self::box_::{Appearance, BreakBetween, Clear, Float}; +pub use self::box_::{Appearance, BreakBetween, BreakWithin, Clear, Float}; pub use self::box_::{Display, TransitionProperty}; pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize}; pub use self::box_::{ScrollSnapType, TouchAction, VerticalAlign, WillChange}; diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 8d0c9f38be9..5229f839eaf 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -1315,3 +1315,26 @@ pub enum BreakBetween { Left, Right, } + +/// A kind of break within a box. +/// +/// https://drafts.csswg.org/css-break/#break-within +#[allow(missing_docs)] +#[derive( + Clone, + Copy, + Debug, + Eq, + Hash, + MallocSizeOf, + Parse, + PartialEq, + SpecifiedValueInfo, + ToCss, + ToComputedValue, +)] +#[repr(u8)] +pub enum BreakWithin { + Auto, + Avoid, +} diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 6fa8dbb4101..0a1c323481e 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -36,7 +36,7 @@ pub use self::border::{BorderCornerRadius, BorderImageSlice, BorderImageWidth}; pub use self::border::{BorderImageRepeat, BorderImageSideWidth}; pub use self::border::{BorderRadius, BorderSideWidth, BorderSpacing}; pub use self::box_::{AnimationIterationCount, AnimationName, Contain, Display}; -pub use self::box_::{Appearance, BreakBetween, Clear, Float}; +pub use self::box_::{Appearance, BreakBetween, BreakWithin, Clear, Float}; pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize}; pub use self::box_::{ScrollSnapType, TouchAction, TransitionProperty, VerticalAlign, WillChange}; pub use self::color::{Color, ColorPropertyValue, RGBAColor}; From 0e7adcf18af72b09088526d5d71489049f624554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 15 Nov 2018 03:23:39 +0000 Subject: [PATCH 18/23] style: Simplify background-repeat. This way we always serialize in the shortest form, and take less space. This is useful because when serializing uncomputed values we'd like to compare to the initial value to avoid serializing parts of a shorthand, but with the existing implementation we would generate always a second keyword, which means that we'll never match it. This also matches Chrome and WebKit, incidentally, so I'm pretty confident the behavior change when serializing specified style is web-compatible. Differential Revision: https://phabricator.services.mozilla.com/D11941 --- .../style/values/computed/background.rs | 85 +------------------ .../style/values/specified/background.rs | 61 +++++++++---- 2 files changed, 46 insertions(+), 100 deletions(-) diff --git a/components/style/values/computed/background.rs b/components/style/values/computed/background.rs index fd1348a9e97..e3fbd17b241 100644 --- a/components/style/values/computed/background.rs +++ b/components/style/values/computed/background.rs @@ -5,12 +5,9 @@ //! Computed types for CSS values related to backgrounds. use crate::values::computed::length::NonNegativeLengthOrPercentageOrAuto; -use crate::values::computed::{Context, ToComputedValue}; use crate::values::generics::background::BackgroundSize as GenericBackgroundSize; -use crate::values::specified::background::BackgroundRepeat as SpecifiedBackgroundRepeat; -use crate::values::specified::background::BackgroundRepeatKeyword; -use std::fmt::{self, Write}; -use style_traits::{CssWriter, ToCss}; + +pub use crate::values::specified::background::BackgroundRepeat; /// A computed value for the `background-size` property. pub type BackgroundSize = GenericBackgroundSize; @@ -24,81 +21,3 @@ impl BackgroundSize { } } } - -/// The computed value of the `background-repeat` property: -/// -/// https://drafts.csswg.org/css-backgrounds/#the-background-repeat -#[derive(Clone, Debug, MallocSizeOf, PartialEq)] -pub struct BackgroundRepeat(pub BackgroundRepeatKeyword, pub BackgroundRepeatKeyword); - -impl BackgroundRepeat { - /// Returns the `repeat repeat` value. - pub fn repeat() -> Self { - BackgroundRepeat( - BackgroundRepeatKeyword::Repeat, - BackgroundRepeatKeyword::Repeat, - ) - } -} - -impl ToCss for BackgroundRepeat { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - match (self.0, self.1) { - (BackgroundRepeatKeyword::Repeat, BackgroundRepeatKeyword::NoRepeat) => { - dest.write_str("repeat-x") - }, - (BackgroundRepeatKeyword::NoRepeat, BackgroundRepeatKeyword::Repeat) => { - dest.write_str("repeat-y") - }, - (horizontal, vertical) => { - horizontal.to_css(dest)?; - if horizontal != vertical { - dest.write_str(" ")?; - vertical.to_css(dest)?; - } - Ok(()) - }, - } - } -} - -impl ToComputedValue for SpecifiedBackgroundRepeat { - type ComputedValue = BackgroundRepeat; - - #[inline] - fn to_computed_value(&self, _: &Context) -> Self::ComputedValue { - match *self { - SpecifiedBackgroundRepeat::RepeatX => BackgroundRepeat( - BackgroundRepeatKeyword::Repeat, - BackgroundRepeatKeyword::NoRepeat, - ), - SpecifiedBackgroundRepeat::RepeatY => BackgroundRepeat( - BackgroundRepeatKeyword::NoRepeat, - BackgroundRepeatKeyword::Repeat, - ), - SpecifiedBackgroundRepeat::Keywords(horizontal, vertical) => { - BackgroundRepeat(horizontal, vertical.unwrap_or(horizontal)) - }, - } - } - - #[inline] - fn from_computed_value(computed: &Self::ComputedValue) -> Self { - // FIXME(emilio): Why can't this just be: - // SpecifiedBackgroundRepeat::Keywords(computed.0, computed.1) - match (computed.0, computed.1) { - (BackgroundRepeatKeyword::Repeat, BackgroundRepeatKeyword::NoRepeat) => { - SpecifiedBackgroundRepeat::RepeatX - }, - (BackgroundRepeatKeyword::NoRepeat, BackgroundRepeatKeyword::Repeat) => { - SpecifiedBackgroundRepeat::RepeatY - }, - (horizontal, vertical) => { - SpecifiedBackgroundRepeat::Keywords(horizontal, Some(vertical)) - }, - } - } -} diff --git a/components/style/values/specified/background.rs b/components/style/values/specified/background.rs index 9aa47fb522c..0c96cf8b707 100644 --- a/components/style/values/specified/background.rs +++ b/components/style/values/specified/background.rs @@ -9,7 +9,8 @@ use crate::values::generics::background::BackgroundSize as GenericBackgroundSize use crate::values::specified::length::NonNegativeLengthOrPercentageOrAuto; use cssparser::Parser; use selectors::parser::SelectorParseErrorKind; -use style_traits::ParseError; +use std::fmt::{self, Write}; +use style_traits::{CssWriter, ParseError, ToCss}; /// A specified value for the `background-size` property. pub type BackgroundSize = GenericBackgroundSize; @@ -56,6 +57,7 @@ impl BackgroundSize { ToCss, )] #[allow(missing_docs)] +#[value_info(other_values = "repeat-x,repeat-y")] pub enum BackgroundRepeatKeyword { Repeat, Space, @@ -63,24 +65,45 @@ pub enum BackgroundRepeatKeyword { NoRepeat, } -/// The specified value for the `background-repeat` property. +/// The value of the `background-repeat` property, with `repeat-x` / `repeat-y` +/// represented as the combination of `no-repeat` and `repeat` in the opposite +/// axes. /// /// https://drafts.csswg.org/css-backgrounds/#the-background-repeat -#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)] -pub enum BackgroundRepeat { - /// `repeat-x` - RepeatX, - /// `repeat-y` - RepeatY, - /// `[repeat | space | round | no-repeat]{1,2}` - Keywords(BackgroundRepeatKeyword, Option), -} +#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue)] +pub struct BackgroundRepeat(pub BackgroundRepeatKeyword, pub BackgroundRepeatKeyword); impl BackgroundRepeat { - /// Returns the `repeat` value. - #[inline] + /// Returns the `repeat repeat` value. pub fn repeat() -> Self { - BackgroundRepeat::Keywords(BackgroundRepeatKeyword::Repeat, None) + BackgroundRepeat( + BackgroundRepeatKeyword::Repeat, + BackgroundRepeatKeyword::Repeat, + ) + } +} + +impl ToCss for BackgroundRepeat { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + match (self.0, self.1) { + (BackgroundRepeatKeyword::Repeat, BackgroundRepeatKeyword::NoRepeat) => { + dest.write_str("repeat-x") + }, + (BackgroundRepeatKeyword::NoRepeat, BackgroundRepeatKeyword::Repeat) => { + dest.write_str("repeat-y") + }, + (horizontal, vertical) => { + horizontal.to_css(dest)?; + if horizontal != vertical { + dest.write_str(" ")?; + vertical.to_css(dest)?; + } + Ok(()) + }, + } } } @@ -92,8 +115,12 @@ impl Parse for BackgroundRepeat { let ident = input.expect_ident_cloned()?; match_ignore_ascii_case! { &ident, - "repeat-x" => return Ok(BackgroundRepeat::RepeatX), - "repeat-y" => return Ok(BackgroundRepeat::RepeatY), + "repeat-x" => { + return Ok(BackgroundRepeat(BackgroundRepeatKeyword::Repeat, BackgroundRepeatKeyword::NoRepeat)); + }, + "repeat-y" => { + return Ok(BackgroundRepeat(BackgroundRepeatKeyword::NoRepeat, BackgroundRepeatKeyword::Repeat)); + }, _ => {}, } @@ -107,6 +134,6 @@ impl Parse for BackgroundRepeat { }; let vertical = input.try(BackgroundRepeatKeyword::parse).ok(); - Ok(BackgroundRepeat::Keywords(horizontal, vertical)) + Ok(BackgroundRepeat(horizontal, vertical.unwrap_or(horizontal))) } } From 60331f01d0674bcd4008e3ff00069bc3d454161a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 15 Nov 2018 08:22:31 +0000 Subject: [PATCH 19/23] style: Make the serialization of the mask shorthand more compact. This helps to preserve the old longhand form when possible (mask used to be a longhand), which will be relevant when we serialize this for the computed value. Differential Revision: https://phabricator.services.mozilla.com/D11943 --- .../style/properties/shorthands/svg.mako.rs | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/components/style/properties/shorthands/svg.mako.rs b/components/style/properties/shorthands/svg.mako.rs index e814a0404c4..b5888948cf8 100644 --- a/components/style/properties/shorthands/svg.mako.rs +++ b/components/style/properties/shorthands/svg.mako.rs @@ -148,21 +148,32 @@ % endfor image.to_css(dest)?; - dest.write_str(" ")?; - mode.to_css(dest)?; - dest.write_str(" ")?; - Position { - horizontal: position_x.clone(), - vertical: position_y.clone() - }.to_css(dest)?; - - if *size != mask_size::single_value::get_initial_specified_value() { - dest.write_str(" / ")?; - size.to_css(dest)?; + if *mode != mask_mode::single_value::get_initial_specified_value() { + dest.write_str(" ")?; + mode.to_css(dest)?; + } + + if *position_x != PositionComponent::zero() || + *position_y != PositionComponent::zero() || + *size != mask_size::single_value::get_initial_specified_value() + { + dest.write_str(" ")?; + Position { + horizontal: position_x.clone(), + vertical: position_y.clone() + }.to_css(dest)?; + + if *size != mask_size::single_value::get_initial_specified_value() { + dest.write_str(" / ")?; + size.to_css(dest)?; + } + } + + if *repeat != mask_repeat::single_value::get_initial_specified_value() { + dest.write_str(" ")?; + repeat.to_css(dest)?; } - dest.write_str(" ")?; - repeat.to_css(dest)?; if *origin != Origin::BorderBox || *clip != Clip::BorderBox { dest.write_str(" ")?; @@ -173,8 +184,10 @@ } } - dest.write_str(" ")?; - composite.to_css(dest)?; + if *composite != mask_composite::single_value::get_initial_specified_value() { + dest.write_str(" ")?; + composite.to_css(dest)?; + } } Ok(()) From ebb57eff22f102579c6e7ce261b4806e31f4efde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 15 Nov 2018 08:25:13 +0000 Subject: [PATCH 20/23] style: Add a mechanism to serialize shorthands for getComputedStyle(). This implements the mechanism reusing the animation machinery for now, so it asserts in a few cases that this wouldn't handle correctly. For shorthands that have colors and other bits we'd need a more sophisticated mechanism with a bit more code (that resolves colors and such), but it'd look something like this regardless, and we should have this in any case. Differential Revision: https://phabricator.services.mozilla.com/D11944 --- components/style/properties/declaration_block.rs | 4 +++- components/style/properties/properties.mako.rs | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index a6d736fb49f..dfe619fc79b 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -309,7 +309,9 @@ impl PropertyDeclarationBlock { .find(|(declaration, _)| declaration.id() == property) } - fn shorthand_to_css( + /// Tries to serialize a given shorthand from the declarations in this + /// block. + pub fn shorthand_to_css( &self, shorthand: ShorthandId, dest: &mut CssStringWriter, diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 072b78cf377..ad51e87fb2f 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1354,6 +1354,13 @@ impl ShorthandId { NonCustomPropertyId::from(self).to_nscsspropertyid() } + /// Converts from a nsCSSPropertyID to a ShorthandId. + #[cfg(feature = "gecko")] + #[inline] + pub fn from_nscsspropertyid(prop: nsCSSPropertyID) -> Result { + PropertyId::from_nscsspropertyid(prop)?.as_shorthand().map_err(|_| ()) + } + /// Get the longhand ids that form this shorthand. pub fn longhands(&self) -> NonCustomPropertyIterator { % for property in data.shorthands: From cd75cd6a86cbae850d8927c7c0772fb68582d25a Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Thu, 15 Nov 2018 16:42:54 +0000 Subject: [PATCH 21/23] style: fixed an unused import warning. Differential Revision: https://phabricator.services.mozilla.com/D12028 --- components/style/gecko/selector_parser.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index 1871d60a0b2..cde467b8335 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -5,7 +5,6 @@ //! Gecko-specific bits for selector-parsing. use crate::element_state::{DocumentState, ElementState}; -use crate::gecko_bindings::structs; use crate::gecko_bindings::structs::RawServoSelectorList; use crate::gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI}; use crate::invalidation::element::document_state::InvalidationMatchingData; From 06fe0a1fc04700b6473be0412c561d3de489d32c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 17 Nov 2018 09:48:01 +0100 Subject: [PATCH 22/23] style: Fix formatting. --- components/style/custom_properties.rs | 3 +- .../media_queries/media_feature_expression.rs | 6 +++- components/style/values/computed/box.rs | 2 +- components/style/values/computed/mod.rs | 7 +++-- .../style/values/specified/basic_shape.rs | 2 +- components/style/values/specified/mod.rs | 5 +++- .../style/values/specified/transform.rs | 29 ++++++++++--------- 7 files changed, 34 insertions(+), 20 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index c1d337ac2fa..c9d4ab0b753 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -335,7 +335,8 @@ impl VariableValue { let (first_token_type, css, last_token_type) = parse_self_contained_declaration_value(input, Some(&mut references))?; - let custom_property_references = references.custom_property_references + let custom_property_references = references + .custom_property_references .into_iter() .collect::>() .into_boxed_slice(); diff --git a/components/style/media_queries/media_feature_expression.rs b/components/style/media_queries/media_feature_expression.rs index 122b7ca05ff..0b093931fad 100644 --- a/components/style/media_queries/media_feature_expression.rs +++ b/components/style/media_queries/media_feature_expression.rs @@ -325,7 +325,11 @@ impl MediaFeatureExpression { }; let atom = Atom::from(string_as_ascii_lowercase(feature_name)); - match MEDIA_FEATURES.iter().enumerate().find(|(_, f)| f.name == atom) { + match MEDIA_FEATURES + .iter() + .enumerate() + .find(|(_, f)| f.name == atom) + { Some((i, f)) => Ok((i, f, range)), None => Err(()), } diff --git a/components/style/values/computed/box.rs b/components/style/values/computed/box.rs index 1af0efff85c..8cf2a63b3c1 100644 --- a/components/style/values/computed/box.rs +++ b/components/style/values/computed/box.rs @@ -12,8 +12,8 @@ use crate::values::generics::box_::VerticalAlign as GenericVerticalAlign; use crate::values::specified::box_ as specified; pub use crate::values::specified::box_::{AnimationName, Appearance, BreakBetween, BreakWithin}; -pub use crate::values::specified::box_::{Contain, Display, OverflowClipBox}; pub use crate::values::specified::box_::{Clear as SpecifiedClear, Float as SpecifiedFloat}; +pub use crate::values::specified::box_::{Contain, Display, OverflowClipBox}; pub use crate::values::specified::box_::{OverscrollBehavior, ScrollSnapType}; pub use crate::values::specified::box_::{TouchAction, TransitionProperty, WillChange}; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index cae5519f2e7..0dce1502e92 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -4,6 +4,7 @@ //! Computed values. +use self::transform::DirectionVector; use super::animated::ToAnimatedValue; use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent; use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth}; @@ -23,7 +24,6 @@ use crate::Atom; #[cfg(feature = "servo")] use crate::Prefix; use euclid::Size2D; -use self::transform::DirectionVector; use std::cell::RefCell; use std::cmp; use std::f32; @@ -467,7 +467,10 @@ impl IsParallelTo for (Number, Number, Number) { // If a and b is parallel, the angle between them is 0deg, so // a x b = |a|*|b|*sin(0)*n = 0 * n, |a x b| == 0. let self_vector = DirectionVector::new(self.0, self.1, self.2); - self_vector.cross(*vector).square_length().approx_eq(&0.0f32) + self_vector + .cross(*vector) + .square_length() + .approx_eq(&0.0f32) } } diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index b371d709b80..252542df794 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -14,7 +14,7 @@ use crate::values::generics::basic_shape::{ShapeBox, ShapeSource}; use crate::values::generics::rect::Rect; use crate::values::specified::border::BorderRadius; use crate::values::specified::image::Image; -use crate::values::specified::position::{HorizontalPosition, VerticalPosition, Position}; +use crate::values::specified::position::{HorizontalPosition, Position, VerticalPosition}; use crate::values::specified::url::SpecifiedUrl; use crate::values::specified::LengthOrPercentage; use crate::values::specified::SVGPathData; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 0a1c323481e..5ffa947ef35 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -300,7 +300,10 @@ impl IsParallelTo for (Number, Number, Number) { // If a and b is parallel, the angle between them is 0deg, so // a x b = |a|*|b|*sin(0)*n = 0 * n, |a x b| == 0. let self_vector = DirectionVector::new(self.0.get(), self.1.get(), self.2.get()); - self_vector.cross(*vector).square_length().approx_eq(&0.0f32) + self_vector + .cross(*vector) + .square_length() + .approx_eq(&0.0f32) } } diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs index 5f455c733de..650722b249f 100644 --- a/components/style/values/specified/transform.rs +++ b/components/style/values/specified/transform.rs @@ -362,21 +362,24 @@ impl Parse for Rotate { // The rotate axis and angle could be in any order, so we parse angle twice to cover // two cases. i.e. `{3} ` or ` {3}` let angle = input.try(|i| specified::Angle::parse(context, i)).ok(); - let axis = input.try(|i| { - Ok(try_match_ident_ignore_ascii_case! { i, - "x" => (Number::new(1.), Number::new(0.), Number::new(0.)), - "y" => (Number::new(0.), Number::new(1.), Number::new(0.)), - "z" => (Number::new(0.), Number::new(0.), Number::new(1.)), + let axis = input + .try(|i| { + Ok(try_match_ident_ignore_ascii_case! { i, + "x" => (Number::new(1.), Number::new(0.), Number::new(0.)), + "y" => (Number::new(0.), Number::new(1.), Number::new(0.)), + "z" => (Number::new(0.), Number::new(0.), Number::new(1.)), + }) }) - }).or_else(|_: ParseError| -> Result<_, ParseError> { - input.try(|i| { - Ok(( - Number::parse(context, i)?, - Number::parse(context, i)?, - Number::parse(context, i)?, - )) + .or_else(|_: ParseError| -> Result<_, ParseError> { + input.try(|i| { + Ok(( + Number::parse(context, i)?, + Number::parse(context, i)?, + Number::parse(context, i)?, + )) + }) }) - }).ok(); + .ok(); let angle = match angle { Some(a) => a, None => specified::Angle::parse(context, input)?, From 2b89e5258f538fff63ec6ad7931a79dd5d2d71d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 17 Nov 2018 13:30:34 +0100 Subject: [PATCH 23/23] Update WPT expectations. --- .../parsing/rotate-parsing-valid.html.ini | 31 ------------------- 1 file changed, 31 deletions(-) delete mode 100644 tests/wpt/metadata/css/css-transforms/parsing/rotate-parsing-valid.html.ini diff --git a/tests/wpt/metadata/css/css-transforms/parsing/rotate-parsing-valid.html.ini b/tests/wpt/metadata/css/css-transforms/parsing/rotate-parsing-valid.html.ini deleted file mode 100644 index bc107c5939d..00000000000 --- a/tests/wpt/metadata/css/css-transforms/parsing/rotate-parsing-valid.html.ini +++ /dev/null @@ -1,31 +0,0 @@ -[rotate-parsing-valid.html] - [e.style['rotate'\] = "400grad 100 200 300" should set the property value] - expected: FAIL - - [e.style['rotate'\] = "x 400grad" should set the property value] - expected: FAIL - - [e.style['rotate'\] = "400grad x" should set the property value] - expected: FAIL - - [e.style['rotate'\] = "y 400grad" should set the property value] - expected: FAIL - - [e.style['rotate'\] = "400grad y" should set the property value] - expected: FAIL - - [e.style['rotate'\] = "z 400grad" should set the property value] - expected: FAIL - - [e.style['rotate'\] = "400grad z" should set the property value] - expected: FAIL - - [e.style['rotate'\] = "1 0 0 400grad" should set the property value] - expected: FAIL - - [e.style['rotate'\] = "0 1 0 400grad" should set the property value] - expected: FAIL - - [e.style['rotate'\] = "0 0 1 400grad" should set the property value] - expected: FAIL -