From 96e9a73918d3ff46a8b3e7f620bad9b827b0c805 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] Bug 1507309 - Simplify background-repeat. r=heycam 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 --HG-- extra : moz-landing-system : lando --- .../style/values/computed/background.rs | 85 +------------------ .../style/values/specified/background.rs | 61 +++++++++---- 2 files changed, 46 insertions(+), 100 deletions(-) diff --git a/servo/components/style/values/computed/background.rs b/servo/components/style/values/computed/background.rs index fd1348a9e977..e3fbd17b241e 100644 --- a/servo/components/style/values/computed/background.rs +++ b/servo/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/servo/components/style/values/specified/background.rs b/servo/components/style/values/specified/background.rs index 9aa47fb522c2..0c96cf8b7073 100644 --- a/servo/components/style/values/specified/background.rs +++ b/servo/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))) } }