Bug 1618997 - Omit center positions in conic/radial gradient serialization. r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D67461

--HG--
extra : moz-landing-system : lando

layout/style/test/test_computed_style.html

@@ -206,10 +206,10 @@ var noframe_container = document.getElementById("content");
       "radial-gradient(at 10% 100%, rgb(255, 255, 255), rgb(0, 0, 0))",
       "radial gradient 1" ],
     [ "radial-gradient(#ffffff, black)",
-      "radial-gradient(at 50% 50%, rgb(255, 255, 255), rgb(0, 0, 0))",
+      "radial-gradient(rgb(255, 255, 255), rgb(0, 0, 0))",
       "radial gradient 2" ],
     [ "radial-gradient(farthest-corner, #ffffff, black)",
-      "radial-gradient(at 50% 50%, rgb(255, 255, 255), rgb(0, 0, 0))",
+      "radial-gradient(rgb(255, 255, 255), rgb(0, 0, 0))",
       "radial gradient 3" ],
     [ "linear-gradient(red, blue)",
       "linear-gradient(rgb(255, 0, 0), rgb(0, 0, 255))",
@@ -628,7 +628,7 @@ var noframe_container = document.getElementById("content");
 
     // Radial gradients (should be serialized using modern unprefixed style):
     [ "-webkit-radial-gradient(contain, red, blue)",
-      "-webkit-radial-gradient(50% 50%, closest-side, rgb(255, 0, 0), rgb(0, 0, 255))",
+      "-webkit-radial-gradient(closest-side, rgb(255, 0, 0), rgb(0, 0, 255))",
       "-webkit-radial-gradient with legacy 'contain' keyword" ],
   ];
 

layout/style/test/test_specified_value_serialization.html

@@ -125,7 +125,7 @@
 
     // Servo keeps the original prefix form which is closer to other impls.
     [ "-webkit-radial-gradient(contain, red, blue)",
-      "-webkit-radial-gradient(center center, closest-side, red, blue)",
+      "-webkit-radial-gradient(closest-side, red, blue)",
       "-webkit-radial-gradient with legacy 'contain' keyword" ],
   ];
 

servo/components/style/values/computed/position.rs

@@ -9,6 +9,7 @@
 
 use crate::values::computed::{Integer, LengthPercentage, Percentage};
 use crate::values::generics::position::Position as GenericPosition;
+use crate::values::generics::position::PositionComponent as GenericPositionComponent;
 use crate::values::generics::position::PositionOrAuto as GenericPositionOrAuto;
 use crate::values::generics::position::ZIndex as GenericZIndex;
 pub use crate::values::specified::position::{GridAutoFlow, GridTemplateAreas};
@@ -56,5 +57,14 @@ impl ToCss for Position {
     }
 }
 
+impl GenericPositionComponent for LengthPercentage {
+    fn is_center(&self) -> bool {
+        match self.to_percentage() {
+            Some(Percentage(per)) => per == 0.5,
+            _ => false
+        }
+    }
+}
+
 /// A computed value for the `z-index` property.
 pub type ZIndex = GenericZIndex<Integer>;

servo/components/style/values/generics/image.rs

@@ -13,6 +13,7 @@ use crate::Zero;
 use servo_arc::Arc;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
+use values::generics::position::PositionComponent;
 
 /// An `<image> | none` value.
 ///
@@ -330,7 +331,7 @@ where
     LP: ToCss,
     NL: ToCss,
     NLP: ToCss,
-    P: ToCss,
+    P: PositionComponent + ToCss,
     A: ToCss,
     AoP: ToCss,
     C: ToCss,
@@ -379,36 +380,59 @@ where
                     EndingShape::Ellipse(Ellipse::Extent(ShapeExtent::FarthestCorner)) => true,
                     _ => false,
                 };
+                let omit_position = position.is_center();
                 if compat_mode == GradientCompatMode::Modern {
                     if !omit_shape {
                         shape.to_css(dest)?;
-                        dest.write_str(" ")?;
+                        if !omit_position {
+                            dest.write_str(" ")?;
+                        }
+                    }
+                    if !omit_position {
+                        dest.write_str("at ")?;
+                        position.to_css(dest)?;
                     }
-                    dest.write_str("at ")?;
-                    position.to_css(dest)?;
                 } else {
-                    position.to_css(dest)?;
+                    if !omit_position {
+                        position.to_css(dest)?;
+                        if !omit_shape {
+                            dest.write_str(", ")?;
+                        }
+                    }
                     if !omit_shape {
-                        dest.write_str(", ")?;
                         shape.to_css(dest)?;
                     }
                 }
+                let mut skip_comma = omit_shape && omit_position;
                 for item in &**items {
-                    dest.write_str(", ")?;
+                    if !skip_comma {
+                        dest.write_str(", ")?;
+                    }
+                    skip_comma = false;
                     item.to_css(dest)?;
                 }
             },
             Gradient::Conic { ref angle, ref position, ref items, .. } => {
                 dest.write_str("conic-gradient(")?;
-                if !angle.is_zero() {
+                let omit_angle = angle.is_zero();
+                let omit_position = position.is_center();
+                if !omit_angle {
                     dest.write_str("from ")?;
                     angle.to_css(dest)?;
-                    dest.write_str(" ")?;
+                    if !omit_position {
+                        dest.write_str(" ")?;
+                    }
                 }
-                dest.write_str("at ")?;
-                position.to_css(dest)?;
+                if !omit_position {
+                    dest.write_str("at ")?;
+                    position.to_css(dest)?;
+                }
+                let mut skip_comma = omit_angle && omit_position;
                 for item in &**items {
-                    dest.write_str(", ")?;
+                    if !skip_comma {
+                        dest.write_str(", ")?;
+                    }
+                    skip_comma = false;
                     item.to_css(dest)?;
                 }
             },

servo/components/style/values/generics/position.rs

@@ -31,6 +31,17 @@ pub struct GenericPosition<H, V> {
     pub vertical: V,
 }
 
+impl<H, V> PositionComponent for Position<H, V>
+where
+    H: PositionComponent,
+    V: PositionComponent,
+{
+    #[inline]
+    fn is_center(&self) -> bool {
+        self.horizontal.is_center() && self.vertical.is_center()
+    }
+}
+
 pub use self::GenericPosition as Position;
 
 impl<H, V> Position<H, V> {
@@ -43,6 +54,13 @@ impl<H, V> Position<H, V> {
     }
 }
 
+/// Implements a method that checks if the position is centered.
+pub trait PositionComponent {
+    /// Returns if the position component is 50% or center.
+    /// For pixel lengths, it always returns false.
+    fn is_center(&self) -> bool;
+}
+
 /// A generic type for representing an `Auto | <position>`.
 /// This is used by <offset-anchor> for now.
 /// https://drafts.fxtf.org/motion-1/#offset-anchor-property

servo/components/style/values/specified/position.rs

@@ -13,6 +13,7 @@ use crate::str::HTML_SPACE_CHARACTERS;
 use crate::values::computed::LengthPercentage as ComputedLengthPercentage;
 use crate::values::computed::{Context, Percentage, ToComputedValue};
 use crate::values::generics::position::Position as GenericPosition;
+use crate::values::generics::position::PositionComponent as GenericPositionComponent;
 use crate::values::generics::position::PositionOrAuto as GenericPositionOrAuto;
 use crate::values::generics::position::ZIndex as GenericZIndex;
 use crate::values::specified::{AllowQuirks, Integer, LengthPercentage};
@@ -262,6 +263,18 @@ impl<S: Parse> PositionComponent<S> {
     }
 }
 
+impl<S> GenericPositionComponent for PositionComponent<S> {
+    fn is_center(&self) -> bool {
+        match *self {
+            PositionComponent::Center => true,
+            PositionComponent::Length(LengthPercentage::Percentage(ref per)) => per.0 == 0.5,
+            // 50% from any side is still the center.
+            PositionComponent::Side(_, Some(LengthPercentage::Percentage(ref per))) => per.0 == 0.5,
+            _ => false,
+        }
+    }
+}
+
 impl<S> PositionComponent<S> {
     /// `0%`
     pub fn zero() -> Self {

testing/web-platform/meta/css/css-backgrounds/__dir__.ini

@@ -1 +1,2 @@
 leak-threshold: [default:51200]
+prefs: [layout.css.conic-gradient.enabled:true]

testing/web-platform/tests/css/css-backgrounds/parsing/background-image-computed.sub.html

@@ -26,10 +26,34 @@ test_computed_value("background-image", 'none, url("http://{{host}}/")');
 
 test_computed_value('background-image', 'linear-gradient(to left bottom, red, blue)', 'linear-gradient(to left bottom, rgb(255, 0, 0), rgb(0, 0, 255))');
 
+test_computed_value('background-image', 'radial-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'radial-gradient(at center, red, blue)', 'radial-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'radial-gradient(at 50%, red, blue)', 'radial-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'radial-gradient(at 10px 10px, rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'radial-gradient(farthest-side, rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'radial-gradient(farthest-side at 10px 10px, rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'radial-gradient(farthest-corner, red, blue)', 'radial-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'radial-gradient(farthest-corner at center, red, blue)', 'radial-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'radial-gradient(farthest-corner at 50%, red, blue)', 'radial-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'radial-gradient(farthest-corner at 10px 10px, red, blue)', 'radial-gradient(at 10px 10px, rgb(255, 0, 0), rgb(0, 0, 255))');
+
 test_computed_value('background-image', 'radial-gradient(10px at 20px 30px, rgb(255, 0, 0), rgb(0, 0, 255))');
 test_computed_value('background-image', 'radial-gradient(circle calc(-0.5em + 10px) at calc(-1em + 10px) calc(-2em + 10px), red, blue)', 'radial-gradient(0px at -30px -70px, rgb(255, 0, 0), rgb(0, 0, 255))');
 test_computed_value('background-image', 'radial-gradient(ellipse calc(-0.5em + 10px) calc(0.5em + 10px) at 20px 30px, red, blue)', 'radial-gradient(0px 30px at 20px 30px, rgb(255, 0, 0), rgb(0, 0, 255))');
 test_computed_value('background-image', 'radial-gradient(ellipse calc(0.5em + 10px) calc(-0.5em + 10px) at 20px 30px, red, blue)', 'radial-gradient(30px 0px at 20px 30px, rgb(255, 0, 0), rgb(0, 0, 255))');
+
+test_computed_value('background-image', 'conic-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(at center, red, blue)', 'conic-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(at 50%, red, blue)', 'conic-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(at 10px 10px, rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(from 0deg, red, blue)', 'conic-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(from 0deg at center, red, blue)', 'conic-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(from 0deg at 50%, red, blue)', 'conic-gradient(rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(from 0deg at 10px 10px, red, blue)', 'conic-gradient(at 10px 10px, rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(from 45deg, rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(from 45deg at center, red, blue)', 'conic-gradient(from 45deg, rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(from 45deg at 50%, red, blue)', 'conic-gradient(from 45deg, rgb(255, 0, 0), rgb(0, 0, 255))');
+test_computed_value('background-image', 'conic-gradient(from 45deg at 10px 10px, red, blue)', 'conic-gradient(from 45deg at 10px 10px, rgb(255, 0, 0), rgb(0, 0, 255))');
 </script>
 </body>
 </html>

testing/web-platform/tests/css/css-images/parsing/gradient-position-valid.html

@@ -13,6 +13,7 @@
 <body>
 <script>
 // Where two values are shown, the first serialization is being used by Blink/Firefox/WebKit and the second by Edge.
+// Where three values are shown, the first is used by Blink/Webkit, the second by Edge and the third by Firefox.
 
 test_valid_value("background-image", "radial-gradient(at 10%, red, blue)", ["radial-gradient(at 10% center, red, blue)", "radial-gradient(at 10%, red, blue)"]);
 test_valid_value("background-image", "radial-gradient(at 20% 30px, red, blue)");
@@ -20,10 +21,10 @@ test_valid_value("background-image", "radial-gradient(at 30px center, red, blue)
 test_valid_value("background-image", "radial-gradient(at 40px top, red, blue)");
 test_valid_value("background-image", "radial-gradient(at bottom 10% right 20%, red, blue)", "radial-gradient(at right 20% bottom 10%, red, blue)");
 test_valid_value("background-image", "radial-gradient(at bottom right, red, blue)", "radial-gradient(at right bottom, red, blue)");
-test_valid_value("background-image", "radial-gradient(at center, red, blue)", ["radial-gradient(at center center, red, blue)", "radial-gradient(at center, red, blue)"]);
+test_valid_value("background-image", "radial-gradient(at center, red, blue)", ["radial-gradient(at center center, red, blue)", "radial-gradient(at center, red, blue)", "radial-gradient(red, blue)"]);
 test_valid_value("background-image", "radial-gradient(at center 50px, red, blue)");
 test_valid_value("background-image", "radial-gradient(at center bottom, red, blue)", ["radial-gradient(at center bottom, red, blue)", "radial-gradient(at bottom, red, blue)"]);
-test_valid_value("background-image", "radial-gradient(at center center, red, blue)", ["radial-gradient(at center center, red, blue)", "radial-gradient(at center, red, blue)"]);
+test_valid_value("background-image", "radial-gradient(at center center, red, blue)", ["radial-gradient(at center center, red, blue)", "radial-gradient(at center, red, blue)", "radial-gradient(red, blue)"]);
 test_valid_value("background-image", "radial-gradient(at center left, red, blue)", ["radial-gradient(at left center, red, blue)", "radial-gradient(at left, red, blue)"]);
 test_valid_value("background-image", "radial-gradient(at left, red, blue)", ["radial-gradient(at left center, red, blue)", "radial-gradient(at left, red, blue)"]);
 test_valid_value("background-image", "radial-gradient(at left bottom, red, blue)");