servo: Merge #16754 - Fix parsing behavior of text-shadow property (from canaltinova:text-shadow); r=emilio
authorNazım Can Altınova <canaltinova@gmail.com>
Sat, 06 May 2017 16:10:22 -0500
changeset 405013 31258a9d9d77dc056d9493a16c93ea55245f0296
parent 405012 26a2a52b11089ba3d0c7845cfa5ff99bbde62962
child 405014 74ff52c38236a889ac008c612e4754f5dc68602a
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #16754 - Fix parsing behavior of text-shadow property (from canaltinova:text-shadow); r=emilio Blur radius should reject negative values. There was already `Shadow` struct for properties like this. `filter: drop-shadow` also has the same syntax with that property. I thought sharing the same code would be better and used Shadow struct for parsing. Converted to SpecifiedTextShadow to get rid of unneccessary fields and to be able to convert its computed value. Maybe it would be better to avoid using `Shadow` or just using `Shadow` but sharing code and avoiding unneccessary fields seems better. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors Source-Repo: https://github.com/servo/servo Source-Revision: bd2cd40c5029f3124f2ed492b4fab7dc8f9101c8
servo/components/style/properties/longhand/inherited_text.mako.rs
--- a/servo/components/style/properties/longhand/inherited_text.mako.rs
+++ b/servo/components/style/properties/longhand/inherited_text.mako.rs
@@ -715,22 +715,23 @@
 </%helpers:single_keyword_computed>
 
 <%helpers:longhand name="text-shadow"
                    animation_value_type="IntermediateTextShadowList",
                    spec="https://drafts.csswg.org/css-text-decor/#propdef-text-shadow">
     use cssparser;
     use std::fmt;
     use style_traits::ToCss;
+    use values::specified::Shadow;
     use values::HasViewportPercentage;
 
     impl HasViewportPercentage for SpecifiedValue {
         fn has_viewport_percentage(&self) -> bool {
             let &SpecifiedValue(ref vec) = self;
-            vec.iter().any(|ref x| x .has_viewport_percentage())
+            vec.iter().any(|ref x| x.has_viewport_percentage())
         }
     }
 
     #[derive(Clone, PartialEq, Debug)]
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     pub struct SpecifiedValue(Vec<SpecifiedTextShadow>);
 
     impl HasViewportPercentage for SpecifiedTextShadow {
@@ -745,16 +746,27 @@
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     pub struct SpecifiedTextShadow {
         pub offset_x: specified::Length,
         pub offset_y: specified::Length,
         pub blur_radius: specified::Length,
         pub color: Option<specified::CSSColor>,
     }
 
+    impl From<Shadow> for SpecifiedTextShadow {
+        fn from(shadow: Shadow) -> SpecifiedTextShadow {
+            SpecifiedTextShadow {
+                offset_x: shadow.offset_x,
+                offset_y: shadow.offset_y,
+                blur_radius: shadow.blur_radius,
+                color: shadow.color,
+            }
+        }
+    }
+
     pub mod computed_value {
         use app_units::Au;
         use cssparser::Color;
         use smallvec::SmallVec;
 
         #[derive(Clone, PartialEq, Debug)]
         #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
         pub struct T(pub SmallVec<[TextShadow; 1]>);
@@ -767,141 +779,87 @@
             pub blur_radius: Au,
             pub color: Color,
         }
     }
 
     impl ToCss for computed_value::T {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             let mut iter = self.0.iter();
-            if let Some(shadow) = iter.next() {
-                try!(shadow.to_css(dest));
-            } else {
-                try!(dest.write_str("none"));
-                return Ok(())
+            match iter.next() {
+                Some(shadow) => shadow.to_css(dest)?,
+                None => return dest.write_str("none"),
             }
             for shadow in iter {
-                try!(dest.write_str(", "));
-                try!(shadow.to_css(dest));
+                dest.write_str(", ")?;
+                shadow.to_css(dest)?;
             }
             Ok(())
         }
     }
 
     impl ToCss for computed_value::TextShadow {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            try!(self.offset_x.to_css(dest));
-            try!(dest.write_str(" "));
-            try!(self.offset_y.to_css(dest));
-            try!(dest.write_str(" "));
-            try!(self.blur_radius.to_css(dest));
-            try!(dest.write_str(" "));
-            try!(self.color.to_css(dest));
-            Ok(())
+            self.offset_x.to_css(dest)?;
+            dest.write_str(" ")?;
+            self.offset_y.to_css(dest)?;
+            dest.write_str(" ")?;
+            self.blur_radius.to_css(dest)?;
+            dest.write_str(" ")?;
+            self.color.to_css(dest)
         }
     }
 
     impl ToCss for SpecifiedValue {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             let mut iter = self.0.iter();
-            if let Some(shadow) = iter.next() {
-                try!(shadow.to_css(dest));
-            } else {
-                try!(dest.write_str("none"));
-                return Ok(())
+            match iter.next() {
+                Some(shadow) => shadow.to_css(dest)?,
+                None => return dest.write_str("none"),
             }
             for shadow in iter {
-                try!(dest.write_str(", "));
-                try!(shadow.to_css(dest));
+                dest.write_str(", ")?;
+                shadow.to_css(dest)?;
             }
             Ok(())
         }
     }
 
     impl ToCss for SpecifiedTextShadow {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            try!(self.offset_x.to_css(dest));
-            try!(dest.write_str(" "));
-            try!(self.offset_y.to_css(dest));
-            try!(dest.write_str(" "));
-            try!(self.blur_radius.to_css(dest));
+            self.offset_x.to_css(dest)?;
+            dest.write_str(" ")?;
+            self.offset_y.to_css(dest)?;
+            dest.write_str(" ")?;
+            self.blur_radius.to_css(dest)?;
 
             if let Some(ref color) = self.color {
-                try!(dest.write_str(" "));
-                try!(color.to_css(dest));
+                dest.write_str(" ")?;
+                color.to_css(dest)?;
             }
             Ok(())
         }
     }
 
     #[inline]
     pub fn get_initial_value() -> computed_value::T {
         use smallvec::SmallVec;
         computed_value::T(SmallVec::new())
     }
 
     pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue,()> {
         if input.try(|input| input.expect_ident_matching("none")).is_ok() {
             Ok(SpecifiedValue(Vec::new()))
         } else {
-            input.parse_comma_separated(|i| parse_one_text_shadow(context, i)).map(SpecifiedValue)
+            input.parse_comma_separated(|i| {
+                Ok(SpecifiedTextShadow::from(Shadow::parse(context, i, true)?))
+            }).map(SpecifiedValue)
         }
     }
 
-    fn parse_one_text_shadow(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedTextShadow,()> {
-        use app_units::Au;
-        let mut lengths = [specified::Length::zero(), specified::Length::zero(), specified::Length::zero()];
-        let mut lengths_parsed = false;
-        let mut color = None;
-
-        loop {
-            if !lengths_parsed {
-                if let Ok(value) = input.try(|i| specified::Length::parse(context, i)) {
-                    lengths[0] = value;
-                    let mut length_parsed_count = 1;
-                    while length_parsed_count < 3 {
-                        if let Ok(value) = input.try(|i| specified::Length::parse(context, i)) {
-                            lengths[length_parsed_count] = value
-                        } else {
-                            break
-                        }
-                        length_parsed_count += 1;
-                    }
-
-                    // The first two lengths must be specified.
-                    if length_parsed_count < 2 {
-                        return Err(())
-                    }
-
-                    lengths_parsed = true;
-                    continue
-                }
-            }
-            if color.is_none() {
-                if let Ok(value) = input.try(|i| specified::CSSColor::parse(context, i)) {
-                    color = Some(value);
-                    continue
-                }
-            }
-            break
-        }
-
-        // Lengths must be specified.
-        if !lengths_parsed {
-            return Err(())
-        }
-
-        Ok(SpecifiedTextShadow {
-            offset_x: lengths[0].take(),
-            offset_y: lengths[1].take(),
-            blur_radius: lengths[2].take(),
-            color: color,
-        })
-    }
-
     impl ToComputedValue for SpecifiedValue {
         type ComputedValue = computed_value::T;
 
         fn to_computed_value(&self, context: &Context) -> computed_value::T {
             computed_value::T(self.0.iter().map(|value| {
                 computed_value::TextShadow {
                     offset_x: value.offset_x.to_computed_value(context),
                     offset_y: value.offset_y.to_computed_value(context),