servo: Merge #15357 - Refactor outline-style to accept "auto" value in addition to border-style values (from dashed:gh-15207); r=Wafflespeanut,emilio

Fixes https://github.com/servo/servo/issues/15207

Refactored as per https://github.com/servo/servo/issues/15207#issuecomment-275171590 .

<!-- Please describe your changes on the following line: -->

- [x] Correct refactor? I'd appreciate any feedback on this.
- [x] ~~~proper borderstyle value for `outline-style: auto;`?~~~ ~~~(EDIT: deferred to a `FIXME`)~~~ (EDIT2: it is now solid for behaviour parity with firefox)
- [x] squash pending PR review
- [x] mako code review

---
<!-- 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
- [x] These changes fix #15207 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 368af6f8619804244a7d8ba4614f0b4950cfd9f1

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : e2c20e25b0f8fea6d8da236085f918c1efd8b8ad

servo/components/layout/display_list_builder.rs

@@ -1072,15 +1072,18 @@ impl FragmentDisplayListBuilding for Fragment {
                                                     style: &ServoComputedValues,
                                                     bounds: &Rect<Au>,
                                                     clip: &ClippingRegion) {
+        use style::values::Either;
+
         let width = style.get_outline().outline_width;
         if width == Au(0) {
             return
         }
 
-        let outline_style = style.get_outline().outline_style;
-        if outline_style == border_style::T::none {
-            return
-        }
+        let outline_style = match style.get_outline().outline_style {
+            Either::First(_auto) => border_style::T::solid,
+            Either::Second(border_style::T::none) => return,
+            Either::Second(border_style) => border_style
+        };
 
         // Outlines are not accounted for in the dimensions of the border box, so adjust the
         // absolute bounds.

servo/components/style/properties/gecko.mako.rs

@@ -59,6 +59,8 @@ use std::ptr;
 use std::sync::Arc;
 use std::cmp;
 use values::computed::ToComputedValue;
+use values::{Either, Auto};
+use computed_values::border_style;
 
 pub mod style_structs {
     % for style_struct in data.style_structs:
@@ -913,7 +915,38 @@ fn static_assert() {
                   skip_longhands="${skip_outline_longhands}"
                   skip_additionals="*">
 
-    <% impl_keyword("outline_style", "mOutlineStyle", border_style_keyword, need_clone=True) %>
+    #[allow(non_snake_case)]
+    pub fn set_outline_style(&mut self, v: longhands::outline_style::computed_value::T) {
+        // FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts
+        let result = match v {
+            % for value in border_style_keyword.values_for('gecko'):
+                Either::Second(border_style::T::${to_rust_ident(value)}) =>
+                    structs::${border_style_keyword.gecko_constant(value)} ${border_style_keyword.maybe_cast("u8")},
+            % endfor
+                Either::First(Auto) =>
+                    structs::${border_style_keyword.gecko_constant('auto')} ${border_style_keyword.maybe_cast("u8")},
+        };
+        ${set_gecko_property("mOutlineStyle", "result")}
+    }
+
+    #[allow(non_snake_case)]
+    pub fn copy_outline_style_from(&mut self, other: &Self) {
+        self.gecko.mOutlineStyle = other.gecko.mOutlineStyle;
+    }
+
+    #[allow(non_snake_case)]
+    pub fn clone_outline_style(&self) -> longhands::outline_style::computed_value::T {
+        // FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts
+        match ${get_gecko_property("mOutlineStyle")} ${border_style_keyword.maybe_cast("u32")} {
+            % for value in border_style_keyword.values_for('gecko'):
+            structs::${border_style_keyword.gecko_constant(value)} => Either::Second(border_style::T::${value}),
+            % endfor
+            structs::${border_style_keyword.gecko_constant('auto')} => Either::First(Auto),
+            % if border_style_keyword.gecko_inexhaustive:
+            x => panic!("Found unexpected value in style struct for outline_style property: {:?}", x),
+            % endif
+        }
+    }
 
     <% impl_app_units("outline_width", "mActualOutlineWidth", need_clone=True,
                       round_to_pixels=True) %>
@@ -2809,4 +2842,3 @@ pub unsafe extern "C" fn Servo_GetStyleVariables(_cv: ServoComputedValuesBorrowe
                                                  -> *const nsStyleVariables {
     &*EMPTY_VARIABLES_STRUCT
 }
-

servo/components/style/properties/longhand/border.mako.rs

@@ -26,7 +26,7 @@
 % for side in ALL_SIDES:
     ${helpers.predefined_type("border-%s-style" % side[0], "BorderStyle",
                               "specified::BorderStyle::none",
-                              needs_context=False, need_clone=True,
+                              need_clone=True,
                               alias=maybe_moz_logical_alias(product, side, "-moz-border-%s-style"),
                               spec=maybe_logical_spec(side, "style"),
                               animatable=False, logical = side[1])}

servo/components/style/properties/longhand/outline.mako.rs

@@ -16,17 +16,46 @@ ${helpers.predefined_type("outline-color", "CSSColor", "::cssparser::Color::Curr
 
 <%helpers:longhand name="outline-style" need_clone="True" animatable="False"
                    spec="https://drafts.csswg.org/css-ui/#propdef-outline-style">
-    pub use values::specified::BorderStyle as SpecifiedValue;
-    pub fn get_initial_value() -> SpecifiedValue { SpecifiedValue::none }
-    pub mod computed_value {
-        pub use values::specified::BorderStyle as T;
-    }
-    pub fn parse(_: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
-        match SpecifiedValue::parse(input) {
-            Ok(SpecifiedValue::hidden) => Err(()),
-            result => result
+
+    use std::fmt;
+    use style_traits::ToCss;
+    use values::specified::BorderStyle;
+    use values::NoViewportPercentage;
+    use values::computed::ComputedValueAsSpecified;
+
+    pub type SpecifiedValue = Either<Auto, BorderStyle>;
+
+    impl SpecifiedValue {
+        #[inline]
+        pub fn none_or_hidden(&self) -> bool {
+            match *self {
+                Either::First(ref _auto) => false,
+                Either::Second(ref border_style) => border_style.none_or_hidden()
+            }
         }
     }
+
+    #[inline]
+    pub fn get_initial_value() -> computed_value::T {
+        Either::Second(BorderStyle::none)
+    }
+
+    pub mod computed_value {
+        pub type T = super::SpecifiedValue;
+    }
+
+    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
+        SpecifiedValue::parse(context, input)
+            .and_then(|result| {
+                if let Either::Second(BorderStyle::hidden) = result {
+                    // The outline-style property accepts the same values as border-style,
+                    // except that 'hidden' is not a legal outline style.
+                    Err(())
+                } else {
+                    Ok(result)
+                }
+            })
+    }
 </%helpers:longhand>
 
 <%helpers:longhand name="outline-width" animatable="True"

servo/components/style/properties/shorthand/border.mako.rs

@@ -10,7 +10,6 @@ ${helpers.four_sides_shorthand("border-color", "border-%s-color", "specified::CS
 
 ${helpers.four_sides_shorthand("border-style", "border-%s-style",
                                "specified::BorderStyle::parse",
-                               needs_context=False,
                                spec="https://drafts.csswg.org/css-backgrounds/#border-style")}
 
 <%helpers:shorthand name="border-width" sub_properties="${
@@ -61,7 +60,7 @@ pub fn parse_border(context: &ParserContext, input: &mut Parser)
             }
         }
         if style.is_none() {
-            if let Ok(value) = input.try(specified::BorderStyle::parse) {
+            if let Ok(value) = input.try(|i| specified::BorderStyle::parse(context, i)) {
                 style = Some(value);
                 any = true;
                 continue

servo/components/style/properties/shorthand/outline.mako.rs

@@ -6,7 +6,7 @@
 
 <%helpers:shorthand name="outline" sub_properties="outline-color outline-style outline-width"
                     spec="https://drafts.csswg.org/css-ui/#propdef-outline">
-    use properties::longhands::outline_width;
+    use properties::longhands::{outline_width, outline_style};
     use values::specified;
     use parser::Parse;
 
@@ -25,7 +25,7 @@
                 }
             }
             if style.is_none() {
-                if let Ok(value) = input.try(specified::BorderStyle::parse) {
+                if let Ok(value) = input.try(|input| outline_style::parse(context, input)) {
                     style = Some(value);
                     any = true;
                     continue

servo/components/style/values/mod.rs

@@ -25,9 +25,9 @@ macro_rules! define_numbered_css_keyword_enum {
             $( $variant = $value ),+
         }
 
-        impl $name {
+        impl Parse for $name {
             #[allow(missing_docs)]
-            pub fn parse(input: &mut ::cssparser::Parser) -> Result<$name, ()> {
+            fn parse(_context: &ParserContext, input: &mut ::cssparser::Parser) -> Result<$name, ()> {
                 match_ignore_ascii_case! { try!(input.expect_ident()),
                     $( $css => Ok($name::$variant), )+
                     _ => Err(())

servo/tests/unit/style/parsing/border.rs

@@ -5,11 +5,12 @@
 use cssparser::Parser;
 use media_queries::CSSErrorReporterTest;
 use servo_url::ServoUrl;
-use style::parser::ParserContext;
+use style::parser::{ParserContext, Parse};
 use style::properties::longhands::{border_image_outset, border_image_repeat, border_image_slice};
 use style::properties::longhands::{border_image_source, border_image_width};
 use style::properties::shorthands::border_image;
 use style::stylesheets::Origin;
+use style_traits::ToCss;
 
 #[test]
 fn border_image_shorthand_should_parse_when_all_properties_specified() {
@@ -122,3 +123,19 @@ fn border_image_outset_should_return_length_on_length_zero() {
     let result = border_image_outset::parse(&context, &mut parser);
     assert_eq!(result.unwrap(), parse_longhand!(border_image_outset, "0em"));
 }
+
+#[test]
+fn test_border_style() {
+    use style::values::specified::BorderStyle;
+
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"none"#);
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"hidden"#);
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"solid"#);
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"double"#);
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"dotted"#);
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"dashed"#);
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"groove"#);
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"ridge"#);
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"inset"#);
+    assert_roundtrip_with_context!(BorderStyle::parse, r#"outset"#);
+}

servo/tests/unit/style/parsing/mod.rs

@@ -75,6 +75,7 @@ mod image;
 mod inherited_box;
 mod inherited_text;
 mod mask;
+mod outline;
 mod position;
 mod selectors;
 mod supports;

servo/tests/unit/style/parsing/outline.rs

@@ -0,0 +1,37 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use cssparser::Parser;
+use media_queries::CSSErrorReporterTest;
+use style::parser::ParserContext;
+use style::stylesheets::Origin;
+use style_traits::ToCss;
+
+#[test]
+fn test_outline_style() {
+    use style::properties::longhands::outline_style;
+
+    assert_roundtrip_with_context!(outline_style::parse, r#"auto"#);
+    assert_roundtrip_with_context!(outline_style::parse, r#"none"#);
+    assert_roundtrip_with_context!(outline_style::parse, r#"solid"#);
+    assert_roundtrip_with_context!(outline_style::parse, r#"double"#);
+    assert_roundtrip_with_context!(outline_style::parse, r#"dotted"#);
+    assert_roundtrip_with_context!(outline_style::parse, r#"dashed"#);
+    assert_roundtrip_with_context!(outline_style::parse, r#"groove"#);
+    assert_roundtrip_with_context!(outline_style::parse, r#"ridge"#);
+    assert_roundtrip_with_context!(outline_style::parse, r#"inset"#);
+    assert_roundtrip_with_context!(outline_style::parse, r#"outset"#);
+
+    {
+        // The outline-style property accepts the same values as border-style,
+        // except that 'hidden' is not a legal outline style.
+
+        let url = ::servo_url::ServoUrl::parse("http://localhost").unwrap();
+        let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
+        let mut parser = Parser::new(r#"hidden"#);
+        let parsed = outline_style::parse(&context, &mut parser);
+        assert!(parsed.is_err());
+    };
+
+}

servo/tests/unit/style/properties/serialization.rs

@@ -8,7 +8,8 @@ pub use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarat
 pub use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length, NoCalcLength};
 pub use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent};
 pub use style::properties::longhands::outline_color::computed_value::T as ComputedColor;
-pub use style::values::RGBA;
+pub use style::properties::longhands::outline_style::SpecifiedValue as OutlineStyle;
+pub use style::values::{RGBA, Auto};
 pub use style::values::specified::url::{UrlExtraData, SpecifiedUrl};
 pub use style_traits::ToCss;
 
@@ -459,6 +460,7 @@ mod shorthand_serialization {
 
     mod outline {
         use style::properties::longhands::outline_width::SpecifiedValue as WidthContainer;
+        use style::values::Either;
         use super::*;
 
         #[test]
@@ -466,7 +468,7 @@ mod shorthand_serialization {
             let mut properties = Vec::new();
 
             let width = DeclaredValue::Value(WidthContainer(Length::from_px(4f32)));
-            let style = DeclaredValue::Value(BorderStyle::solid);
+            let style = DeclaredValue::Value(Either::Second(BorderStyle::solid));
             let color = DeclaredValue::Value(CSSColor {
                 parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }),
                 authored: None
@@ -485,7 +487,7 @@ mod shorthand_serialization {
             let mut properties = Vec::new();
 
             let width = DeclaredValue::Value(WidthContainer(Length::from_px(4f32)));
-            let style = DeclaredValue::Value(BorderStyle::solid);
+            let style = DeclaredValue::Value(Either::Second(BorderStyle::solid));
             let color = DeclaredValue::Initial;
 
             properties.push(PropertyDeclaration::OutlineWidth(width));
@@ -513,6 +515,24 @@ mod shorthand_serialization {
             let serialization = shorthand_properties_to_string(properties);
             assert_eq!(serialization, "outline: 4px none rgb(255, 0, 0);");
         }
+
+        #[test]
+        fn outline_should_serialize_correctly_when_style_is_auto() {
+            let mut properties = Vec::new();
+
+            let width = DeclaredValue::Value(WidthContainer(Length::from_px(4f32)));
+            let style = DeclaredValue::Value(Either::First(Auto));
+            let color = DeclaredValue::Value(CSSColor {
+                parsed: ComputedColor::RGBA(RGBA { red: 1f32, green: 0f32, blue: 0f32, alpha: 1f32 }),
+                authored: None
+            });
+            properties.push(PropertyDeclaration::OutlineWidth(width));
+            properties.push(PropertyDeclaration::OutlineStyle(style));
+            properties.push(PropertyDeclaration::OutlineColor(color));
+
+            let serialization = shorthand_properties_to_string(properties);
+            assert_eq!(serialization, "outline: 4px auto rgb(255, 0, 0);");
+        }
     }
 
     #[test]