style: Simplify text-emphasis-style.

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

components/style/properties/gecko.mako.rs

@@ -2602,12 +2602,13 @@ fn static_assert() {
         self.clear_text_emphasis_style_if_string();
         let (te, s) = match v {
             TextEmphasisStyle::None => (structs::NS_STYLE_TEXT_EMPHASIS_STYLE_NONE, ""),
-            TextEmphasisStyle::Keyword(ref keyword) => {
+            TextEmphasisStyle::Keyword { fill, shape } => {
-                let fill = match keyword.fill {
+                let gecko_fill = match fill {
                     TextEmphasisFillMode::Filled => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_FILLED,
                     TextEmphasisFillMode::Open => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_OPEN,
                 };
-                let shape = match keyword.shape {
+
+                let gecko_shape = match shape {
                     TextEmphasisShapeKeyword::Dot => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_DOT,
                     TextEmphasisShapeKeyword::Circle => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_CIRCLE,
                     TextEmphasisShapeKeyword::DoubleCircle => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_DOUBLE_CIRCLE,
@@ -2615,7 +2616,7 @@ fn static_assert() {
                     TextEmphasisShapeKeyword::Sesame => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_SESAME,
                 };
 
-                (shape | fill, keyword.shape.char(keyword.fill))
+                (gecko_shape | gecko_fill, shape.char(fill))
             },
             TextEmphasisStyle::String(ref s) => {
                 (structs::NS_STYLE_TEXT_EMPHASIS_STYLE_STRING, &**s)
@@ -2640,7 +2641,6 @@ fn static_assert() {
 
     pub fn clone_text_emphasis_style(&self) -> values::computed::TextEmphasisStyle {
         use crate::values::computed::TextEmphasisStyle;
-        use crate::values::computed::text::TextEmphasisKeywordValue;
         use crate::values::specified::text::{TextEmphasisFillMode, TextEmphasisShapeKeyword};
 
         if self.gecko.mTextEmphasisStyle == structs::NS_STYLE_TEXT_EMPHASIS_STYLE_NONE as u8 {
@@ -2648,7 +2648,7 @@ fn static_assert() {
         }
 
         if self.gecko.mTextEmphasisStyle == structs::NS_STYLE_TEXT_EMPHASIS_STYLE_STRING as u8 {
-            return TextEmphasisStyle::String(self.gecko.mTextEmphasisStyleString.to_string());
+            return TextEmphasisStyle::String(self.gecko.mTextEmphasisStyleString.to_string().into());
         }
 
         let fill =
@@ -2666,7 +2666,7 @@ fn static_assert() {
                 _ => panic!("Unexpected value in style struct for text-emphasis-style property")
             };
 
-        TextEmphasisStyle::Keyword(TextEmphasisKeywordValue { fill, shape })
+        TextEmphasisStyle::Keyword { fill, shape }
     }
 
     ${impl_non_negative_length('_webkit_text_stroke_width',

components/style/properties/longhands/inherited_text.mako.rs

@@ -249,7 +249,6 @@ ${helpers.predefined_type(
     None,
     engines="gecko",
     initial_specified_value="SpecifiedValue::None",
-    boxed=True,
     animation_value_type="discrete",
     spec="https://drafts.csswg.org/css-text-decor/#propdef-text-emphasis-style",
 )}

components/style/values/computed/text.rs

@@ -194,22 +194,18 @@ impl TextDecorationsInEffect {
     }
 }
 
-/// computed value for the text-emphasis-style property
+/// Computed value for the text-emphasis-style property
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss, ToResolvedValue)]
+#[allow(missing_docs)]
 pub enum TextEmphasisStyle {
-    /// Keyword value for the text-emphasis-style property (`filled` `open`)
+    /// [ <fill> || <shape> ]
-    Keyword(TextEmphasisKeywordValue),
+    Keyword {
+        #[css(skip_if = "TextEmphasisFillMode::is_filled")]
+        fill: TextEmphasisFillMode,
+        shape: TextEmphasisShapeKeyword,
+    },
     /// `none`
     None,
-    /// String (will be used only first grapheme cluster) for the text-emphasis-style property
+    /// `<string>` (of which only the first grapheme cluster will be used).
-    String(String),
+    String(crate::OwnedStr),
-}
-
-/// Keyword value for the text-emphasis-style property
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss, ToResolvedValue)]
-pub struct TextEmphasisKeywordValue {
-    /// fill for the text-emphasis-style property
-    pub fill: TextEmphasisFillMode,
-    /// shape for the text-emphasis-style property
-    pub shape: TextEmphasisShapeKeyword,
 }

components/style/values/specified/text.rs

@@ -7,7 +7,6 @@
 use crate::parser::{Parse, ParserContext};
 use crate::properties::longhands::writing_mode::computed_value::T as SpecifiedWritingMode;
 use crate::values::computed::text::LineHeight as ComputedLineHeight;
-use crate::values::computed::text::TextEmphasisKeywordValue as ComputedTextEmphasisKeywordValue;
 use crate::values::computed::text::TextEmphasisStyle as ComputedTextEmphasisStyle;
 use crate::values::computed::text::TextOverflow as ComputedTextOverflow;
 use crate::values::computed::{Context, ToComputedValue};
@@ -645,44 +644,29 @@ impl ToComputedValue for TextAlign {
     }
 }
 
+fn fill_mode_is_default_and_shape_exists(
+    fill: &TextEmphasisFillMode,
+    shape: &Option<TextEmphasisShapeKeyword>,
+) -> bool {
+    shape.is_some() && fill.is_filled()
+}
+
 /// Specified value of text-emphasis-style property.
+///
+/// https://drafts.csswg.org/css-text-decor/#propdef-text-emphasis-style
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
+#[allow(missing_docs)]
 pub enum TextEmphasisStyle {
-    /// <fill> <shape>
+    /// [ <fill> || <shape> ]
-    Keyword(TextEmphasisKeywordValue),
+    Keyword {
+        #[css(contextual_skip_if = "fill_mode_is_default_and_shape_exists")]
+        fill: TextEmphasisFillMode,
+        shape: Option<TextEmphasisShapeKeyword>,
+    },
     /// `none`
     None,
-    /// String (will be used only first grapheme cluster) for the text-emphasis-style property
+    /// `<string>` (of which only the first grapheme cluster will be used).
-    String(String),
+    String(crate::OwnedStr),
-}
-
-/// Keyword value for the text-emphasis-style property
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
-pub enum TextEmphasisKeywordValue {
-    /// <fill>
-    Fill(TextEmphasisFillMode),
-    /// <shape>
-    Shape(TextEmphasisShapeKeyword),
-    /// <fill> <shape>
-    FillAndShape(TextEmphasisFillMode, TextEmphasisShapeKeyword),
-}
-
-impl TextEmphasisKeywordValue {
-    fn fill(&self) -> Option<TextEmphasisFillMode> {
-        match *self {
-            TextEmphasisKeywordValue::Fill(fill) |
-            TextEmphasisKeywordValue::FillAndShape(fill, _) => Some(fill),
-            _ => None,
-        }
-    }
-
-    fn shape(&self) -> Option<TextEmphasisShapeKeyword> {
-        match *self {
-            TextEmphasisKeywordValue::Shape(shape) |
-            TextEmphasisKeywordValue::FillAndShape(_, shape) => Some(shape),
-            _ => None,
-        }
-    }
 }
 
 /// Fill mode for the text-emphasis-style property
@@ -694,6 +678,14 @@ pub enum TextEmphasisFillMode {
     Open,
 }
 
+impl TextEmphasisFillMode {
+    /// Whether the value is `filled`.
+    #[inline]
+    pub fn is_filled(&self) -> bool {
+        matches!(*self, TextEmphasisFillMode::Filled)
+    }
+}
+
 /// Shape keyword for the text-emphasis-style property
 #[derive(
     Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem,
@@ -713,7 +705,7 @@ pub enum TextEmphasisShapeKeyword {
 
 impl TextEmphasisShapeKeyword {
     /// converts fill mode to a unicode char
-    pub fn char(&self, fill: TextEmphasisFillMode) -> &str {
+    pub fn char(&self, fill: TextEmphasisFillMode) -> &'static str {
         let fill = fill == TextEmphasisFillMode::Filled;
         match *self {
             TextEmphasisShapeKeyword::Dot => {
@@ -761,27 +753,33 @@ impl ToComputedValue for TextEmphasisStyle {
     #[inline]
     fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
         match *self {
-            TextEmphasisStyle::Keyword(ref keyword) => {
+            TextEmphasisStyle::Keyword { fill, shape } => {
-                // FIXME(emilio): This should set the rule_cache_conditions
+                let shape = shape.unwrap_or_else(|| {
-                // properly.
+                    // FIXME(emilio, bug 1572958): This should set the
-                let default_shape = if context.style().get_inherited_box().clone_writing_mode() ==
+                    // rule_cache_conditions properly.
-                    SpecifiedWritingMode::HorizontalTb
+                    //
-                {
+                    // Also should probably use WritingMode::is_vertical rather
-                    TextEmphasisShapeKeyword::Circle
+                    // than the computed value of the `writing-mode` property.
-                } else {
+                    if context.style().get_inherited_box().clone_writing_mode() ==
-                    TextEmphasisShapeKeyword::Sesame
+                        SpecifiedWritingMode::HorizontalTb
-                };
+                    {
-                ComputedTextEmphasisStyle::Keyword(ComputedTextEmphasisKeywordValue {
+                        TextEmphasisShapeKeyword::Circle
-                    fill: keyword.fill().unwrap_or(TextEmphasisFillMode::Filled),
+                    } else {
-                    shape: keyword.shape().unwrap_or(default_shape),
+                        TextEmphasisShapeKeyword::Sesame
-                })
+                    }
+                });
+                ComputedTextEmphasisStyle::Keyword { fill, shape }
             },
             TextEmphasisStyle::None => ComputedTextEmphasisStyle::None,
             TextEmphasisStyle::String(ref s) => {
                 // Passing `true` to iterate over extended grapheme clusters, following
                 // recommendation at http://www.unicode.org/reports/tr29/#Grapheme_Cluster_Boundaries
-                let string = s.graphemes(true).next().unwrap_or("").to_string();
+                //
-                ComputedTextEmphasisStyle::String(string)
+                // FIXME(emilio): Doing this at computed value time seems wrong.
+                // The spec doesn't say that this should be a computed-value
+                // time operation. This is observable from getComputedStyle().
+                let s = s.graphemes(true).next().unwrap_or("").to_string();
+                ComputedTextEmphasisStyle::String(s.into())
             },
         }
     }
@@ -789,9 +787,10 @@ impl ToComputedValue for TextEmphasisStyle {
     #[inline]
     fn from_computed_value(computed: &Self::ComputedValue) -> Self {
         match *computed {
-            ComputedTextEmphasisStyle::Keyword(ref keyword) => TextEmphasisStyle::Keyword(
+            ComputedTextEmphasisStyle::Keyword { fill, shape } => TextEmphasisStyle::Keyword {
-                TextEmphasisKeywordValue::FillAndShape(keyword.fill, keyword.shape),
+                fill,
-            ),
+                shape: Some(shape),
+            },
             ComputedTextEmphasisStyle::None => TextEmphasisStyle::None,
             ComputedTextEmphasisStyle::String(ref string) => {
                 TextEmphasisStyle::String(string.clone())
@@ -814,7 +813,7 @@ impl Parse for TextEmphasisStyle {
 
         if let Ok(s) = input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned())) {
             // Handle <string>
-            return Ok(TextEmphasisStyle::String(s));
+            return Ok(TextEmphasisStyle::String(s.into()));
         }
 
         // Handle a pair of keywords
@@ -824,14 +823,17 @@ impl Parse for TextEmphasisStyle {
             shape = input.try(TextEmphasisShapeKeyword::parse).ok();
         }
 
-        // At least one of shape or fill must be handled
+        if shape.is_none() && fill.is_none() {
-        let keyword_value = match (fill, shape) {
+            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-            (Some(fill), Some(shape)) => TextEmphasisKeywordValue::FillAndShape(fill, shape),
+        }
-            (Some(fill), None) => TextEmphasisKeywordValue::Fill(fill),
+
-            (None, Some(shape)) => TextEmphasisKeywordValue::Shape(shape),
+        // If a shape keyword is specified but neither of filled nor open is
-            _ => return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)),
+        // specified, filled is assumed.
-        };
+        let fill = fill.unwrap_or(TextEmphasisFillMode::Filled);
-        Ok(TextEmphasisStyle::Keyword(keyword_value))
+
+        // We cannot do the same because the default `<shape>` depends on the
+        // computed writing-mode.
+        Ok(TextEmphasisStyle::Keyword { fill, shape })
     }
 }