Bug 1784022 - [refactor] Convert text-emphasis-position #defines to enum classes r=emilio

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

.gitignore

@@ -131,6 +131,9 @@ mobile/android/gradle/.gradle
 
 # Rust/Cargo output from running `cargo` directly
 /target/
+/servo/ports/geckolib/target/
+/dom/base/rust/target/
+/servo/components/style/target/
 
 # Ignore mozharness execution files
 testing/mozharness/.tox/

.hgignore

@@ -147,6 +147,9 @@ _OPT\.OBJ/
 
 # Rust/Cargo output from running `cargo` directly
 ^target/
+^servo/ports/geckolib/target/
+^dom/base/rust/target/
+^servo/components/style/target/
 
 # Ignore mozharness execution files
 ^testing/mozharness/.tox/

dom/html/nsGenericHTMLElement.cpp

@@ -1188,13 +1188,13 @@ static inline void MapLangAttributeInto(const nsMappedAttributes* aAttributes,
     const nsAtom* lang = langValue->GetAtomValue();
     if (nsStyleUtil::MatchesLanguagePrefix(lang, u"zh")) {
       aDecls.SetKeywordValue(eCSSProperty_text_emphasis_position,
-                             NS_STYLE_TEXT_EMPHASIS_POSITION_DEFAULT_ZH);
+                             StyleTextEmphasisPosition::DEFAULT_ZH.bits);
     } else if (nsStyleUtil::MatchesLanguagePrefix(lang, u"ja") ||
                nsStyleUtil::MatchesLanguagePrefix(lang, u"mn")) {
       // This branch is currently no part of the spec.
       // See bug 1040668 comment 69 and comment 75.
       aDecls.SetKeywordValue(eCSSProperty_text_emphasis_position,
-                             NS_STYLE_TEXT_EMPHASIS_POSITION_DEFAULT);
+                             StyleTextEmphasisPosition::DEFAULT.bits);
     }
   }
 }

layout/style/ServoBindings.toml

@@ -566,6 +566,7 @@ cbindgen-types = [
     { gecko = "StyleGenericTrackList", servo = "crate::values::generics::grid::TrackList" },
     { gecko = "StyleGenericGridTemplateComponent", servo = "crate::values::generics::grid::GridTemplateComponent" },
     { gecko = "StyleTextEmphasisStyle", servo = "crate::values::computed::text::TextEmphasisStyle" },
+    { gecko = "StyleTextEmphasisPosition", servo = "crate::values::computed::TextEmphasisPosition" },
     { gecko = "StyleVariantAlternatesList", servo = "crate::values::specified::font::VariantAlternatesList" },
     { gecko = "StyleSVGPaintOrder", servo = "crate::values::specified::svg::SVGPaintOrder" },
     { gecko = "StyleClipRectOrAuto", servo = "crate::values::computed::ClipRectOrAuto" },

layout/style/nsStyleConsts.h

@@ -682,17 +682,6 @@ enum class StyleTextAnchor : uint8_t {
   End,
 };
 
-// text-emphasis-position
-#define NS_STYLE_TEXT_EMPHASIS_POSITION_OVER (1 << 0)
-#define NS_STYLE_TEXT_EMPHASIS_POSITION_UNDER (1 << 1)
-#define NS_STYLE_TEXT_EMPHASIS_POSITION_LEFT (1 << 2)
-#define NS_STYLE_TEXT_EMPHASIS_POSITION_RIGHT (1 << 3)
-#define NS_STYLE_TEXT_EMPHASIS_POSITION_DEFAULT \
-  (NS_STYLE_TEXT_EMPHASIS_POSITION_OVER | NS_STYLE_TEXT_EMPHASIS_POSITION_RIGHT)
-#define NS_STYLE_TEXT_EMPHASIS_POSITION_DEFAULT_ZH \
-  (NS_STYLE_TEXT_EMPHASIS_POSITION_UNDER |         \
-   NS_STYLE_TEXT_EMPHASIS_POSITION_RIGHT)
-
 // text-rendering
 enum class StyleTextRendering : uint8_t {
   Auto,

layout/style/nsStyleStruct.cpp

@@ -2965,8 +2965,8 @@ nsStyleText::nsStyleText(const Document& aDocument)
   RefPtr<nsAtom> language = aDocument.GetContentLanguageAsAtomForStyle();
   mTextEmphasisPosition =
       language && nsStyleUtil::MatchesLanguagePrefix(language, u"zh")
-          ? NS_STYLE_TEXT_EMPHASIS_POSITION_DEFAULT_ZH
+          ? StyleTextEmphasisPosition::DEFAULT_ZH
-          : NS_STYLE_TEXT_EMPHASIS_POSITION_DEFAULT;
+          : StyleTextEmphasisPosition::DEFAULT;
 }
 
 nsStyleText::nsStyleText(const nsStyleText& aSource)
@@ -3092,17 +3092,16 @@ nsChangeHint nsStyleText::CalcDifference(const nsStyleText& aNewData) const {
 }
 
 LogicalSide nsStyleText::TextEmphasisSide(WritingMode aWM) const {
-  MOZ_ASSERT(
+  MOZ_ASSERT((!(mTextEmphasisPosition & StyleTextEmphasisPosition::LEFT) !=
-      (!(mTextEmphasisPosition & NS_STYLE_TEXT_EMPHASIS_POSITION_LEFT) !=
+              !(mTextEmphasisPosition & StyleTextEmphasisPosition::RIGHT)) &&
-       !(mTextEmphasisPosition & NS_STYLE_TEXT_EMPHASIS_POSITION_RIGHT)) &&
+             (!(mTextEmphasisPosition & StyleTextEmphasisPosition::OVER) !=
-      (!(mTextEmphasisPosition & NS_STYLE_TEXT_EMPHASIS_POSITION_OVER) !=
+              !(mTextEmphasisPosition & StyleTextEmphasisPosition::UNDER)));
-       !(mTextEmphasisPosition & NS_STYLE_TEXT_EMPHASIS_POSITION_UNDER)));
   mozilla::Side side =
       aWM.IsVertical()
-          ? (mTextEmphasisPosition & NS_STYLE_TEXT_EMPHASIS_POSITION_LEFT
+          ? (mTextEmphasisPosition & StyleTextEmphasisPosition::LEFT
                  ? eSideLeft
                  : eSideRight)
-          : (mTextEmphasisPosition & NS_STYLE_TEXT_EMPHASIS_POSITION_OVER
+          : (mTextEmphasisPosition & StyleTextEmphasisPosition::OVER
                  ? eSideTop
                  : eSideBottom);
   LogicalSide result = aWM.LogicalSideForPhysicalSide(side);

layout/style/nsStyleStruct.h

@@ -949,7 +949,7 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleText {
   mozilla::StyleTextSizeAdjust mTextSizeAdjust;
   uint8_t mTextCombineUpright;  // NS_STYLE_TEXT_COMBINE_UPRIGHT_*
   mozilla::StyleMozControlCharacterVisibility mMozControlCharacterVisibility;
-  uint8_t mTextEmphasisPosition;  // NS_STYLE_TEXT_EMPHASIS_POSITION_*
+  mozilla::StyleTextEmphasisPosition mTextEmphasisPosition;
   mozilla::StyleTextRendering mTextRendering;
   mozilla::StyleColor mTextEmphasisColor;
   mozilla::StyleColor mWebkitTextFillColor;

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

@@ -220,9 +220,9 @@ ${helpers.predefined_type(
 ${helpers.predefined_type(
     "text-emphasis-position",
     "TextEmphasisPosition",
-    "computed::TextEmphasisPosition::over_right()",
+    "computed::TextEmphasisPosition::DEFAULT",
     engines="gecko",
-    initial_specified_value="specified::TextEmphasisPosition::over_right()",
+    initial_specified_value="specified::TextEmphasisPosition::DEFAULT",
     animation_value_type="discrete",
     spec="https://drafts.csswg.org/css-text-decor/#propdef-text-emphasis-position",
 )}

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

@@ -772,146 +772,32 @@ impl Parse for TextEmphasisStyle {
     }
 }
 
-/// The allowed horizontal values for the `text-emphasis-position` property.
+bitflags! {
-#[derive(
+    #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem, Parse, ToCss)]
-    Clone,
+    #[repr(C)]
-    Copy,
+    #[css(bitflags(mixed="over,under,left,right", validate_mixed="Self::is_valid"))]
-    Debug,
+    /// Values for text-emphasis-position:
-    Eq,
+    /// <https://drafts.csswg.org/css-text-decor/#text-emphasis-position-property>
-    MallocSizeOf,
+    pub struct TextEmphasisPosition: u8 {
-    Parse,
+        /// Draws marks to the right of the text in vertical writing mode.
-    PartialEq,
+        const OVER = 1 << 0;
-    SpecifiedValueInfo,
+        /// Draw marks under the text in horizontal writing mode.
-    ToComputedValue,
+        const UNDER = 1 << 1;
-    ToCss,
+        /// Draw marks to the left of the text in vertical writing mode.
-    ToResolvedValue,
+        const LEFT = 1 << 2;
-    ToShmem,
+        /// Draws marks to the right of the text in vertical writing mode.
-)]
+        const RIGHT = 1 << 3;
-pub enum TextEmphasisHorizontalWritingModeValue {
+        /// Returns the initial value of `text-emphasis-position`
-    /// Draw marks over the text in horizontal writing mode.
+        const DEFAULT = Self::OVER.bits | Self::RIGHT.bits;
-    Over,
+        /// Non-standard behavior: Intelligent default for zh locale
-    /// Draw marks under the text in horizontal writing mode.
+        const DEFAULT_ZH = Self::UNDER.bits | Self::RIGHT.bits;
-    Under,
+    }
 }
 
-/// The allowed vertical values for the `text-emphasis-position` property.
-#[derive(
-    Clone,
-    Copy,
-    Debug,
-    Eq,
-    MallocSizeOf,
-    Parse,
-    PartialEq,
-    SpecifiedValueInfo,
-    ToComputedValue,
-    ToCss,
-    ToResolvedValue,
-    ToShmem,
-)]
-pub enum TextEmphasisVerticalWritingModeValue {
-    /// Draws marks to the right of the text in vertical writing mode.
-    Right,
-    /// Draw marks to the left of the text in vertical writing mode.
-    Left,
-}
-
-/// Specified value of `text-emphasis-position` property.
-#[derive(
-    Clone,
-    Copy,
-    Debug,
-    MallocSizeOf,
-    PartialEq,
-    SpecifiedValueInfo,
-    ToComputedValue,
-    ToCss,
-    ToResolvedValue,
-    ToShmem,
-)]
-pub struct TextEmphasisPosition(
-    pub TextEmphasisHorizontalWritingModeValue,
-    pub TextEmphasisVerticalWritingModeValue,
-);
-
 impl TextEmphasisPosition {
-    #[inline]
+    fn is_valid(self) -> bool {
-    /// Returns the initial value of `text-emphasis-position`
+        return self.intersects(Self::LEFT) != self.intersects(Self::RIGHT) &&
-    pub fn over_right() -> Self {
+            self.intersects(Self::OVER) != self.intersects(Self::UNDER);
-        TextEmphasisPosition(
-            TextEmphasisHorizontalWritingModeValue::Over,
-            TextEmphasisVerticalWritingModeValue::Right,
-        )
-    }
-
-    #[cfg(feature = "gecko")]
-    /// Converts an enumerated value coming from Gecko to a `TextEmphasisPosition`.
-    pub fn from_gecko_keyword(kw: u32) -> Self {
-        use crate::gecko_bindings::structs;
-
-        let vert = if kw & structs::NS_STYLE_TEXT_EMPHASIS_POSITION_RIGHT != 0 {
-            TextEmphasisVerticalWritingModeValue::Right
-        } else {
-            debug_assert!(kw & structs::NS_STYLE_TEXT_EMPHASIS_POSITION_LEFT != 0);
-            TextEmphasisVerticalWritingModeValue::Left
-        };
-        let horiz = if kw & structs::NS_STYLE_TEXT_EMPHASIS_POSITION_OVER != 0 {
-            TextEmphasisHorizontalWritingModeValue::Over
-        } else {
-            debug_assert!(kw & structs::NS_STYLE_TEXT_EMPHASIS_POSITION_UNDER != 0);
-            TextEmphasisHorizontalWritingModeValue::Under
-        };
-        TextEmphasisPosition(horiz, vert)
-    }
-}
-
-impl Parse for TextEmphasisPosition {
-    fn parse<'i, 't>(
-        _context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        if let Ok(horizontal) =
-            input.try_parse(|input| TextEmphasisHorizontalWritingModeValue::parse(input))
-        {
-            let vertical = TextEmphasisVerticalWritingModeValue::parse(input)?;
-            Ok(TextEmphasisPosition(horizontal, vertical))
-        } else {
-            let vertical = TextEmphasisVerticalWritingModeValue::parse(input)?;
-            let horizontal = TextEmphasisHorizontalWritingModeValue::parse(input)?;
-            Ok(TextEmphasisPosition(horizontal, vertical))
-        }
-    }
-}
-
-#[cfg(feature = "gecko")]
-impl From<u8> for TextEmphasisPosition {
-    fn from(bits: u8) -> Self {
-        TextEmphasisPosition::from_gecko_keyword(bits as u32)
-    }
-}
-
-#[cfg(feature = "gecko")]
-impl From<TextEmphasisPosition> for u8 {
-    fn from(v: TextEmphasisPosition) -> u8 {
-        use crate::gecko_bindings::structs;
-
-        let mut result = match v.0 {
-            TextEmphasisHorizontalWritingModeValue::Over => {
-                structs::NS_STYLE_TEXT_EMPHASIS_POSITION_OVER
-            },
-            TextEmphasisHorizontalWritingModeValue::Under => {
-                structs::NS_STYLE_TEXT_EMPHASIS_POSITION_UNDER
-            },
-        };
-        match v.1 {
-            TextEmphasisVerticalWritingModeValue::Right => {
-                result |= structs::NS_STYLE_TEXT_EMPHASIS_POSITION_RIGHT;
-            },
-            TextEmphasisVerticalWritingModeValue::Left => {
-                result |= structs::NS_STYLE_TEXT_EMPHASIS_POSITION_LEFT;
-            },
-        };
-        result as u8
     }
 }
 

servo/components/style_derive/to_css.rs

@@ -322,24 +322,29 @@ fn derive_single_field_expr(
 }
 
 #[derive(Default, FromMeta)]
+#[darling(default)]
 pub struct CssBitflagAttrs {
     /// Flags that can only go on their own, comma-separated.
-    pub single: String,
+    pub single: Option<String>,
     /// Flags that can go mixed with each other, comma-separated.
-    pub mixed: String,
+    pub mixed: Option<String>,
     /// Extra validation of the resulting mixed flags.
-    #[darling(default)]
     pub validate_mixed: Option<Path>,
     /// Whether there are overlapping bits we need to take care of when
     /// serializing.
-    #[darling(default)]
     pub overlapping_bits: bool,
 }
 
 impl CssBitflagAttrs {
     /// Returns a vector of (rust_name, css_name) of a given flag list.
-    fn names(s: &str) -> Vec<(String, String)> {
+    fn names(s: &Option<String>) -> Vec<(String, String)> {
-        s.split(',').map(|css_name| (cg::to_scream_case(css_name), css_name.to_owned())).collect()
+        let s = match s {
+            Some(s) => s,
+            None => return vec![],
+        };
+        s.split(',')
+            .map(|css_name| (cg::to_scream_case(css_name), css_name.to_owned()))
+            .collect()
     }
 
     pub fn single_flags(&self) -> Vec<(String, String)> {

servo/ports/geckolib/cbindgen.toml

@@ -207,6 +207,7 @@ include = [
   "SVGPaintKind",
   "GridTemplateComponent",
   "TextEmphasisStyle",
+  "TextEmphasisPosition",
   "VariantAlternatesList",
   "PaintOrder",
   "SVGPaintOrder",

servo/ports/geckolib/glue.rs

@@ -5089,7 +5089,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetKeywordValue(
     use style::values::generics::box_::{VerticalAlign, VerticalAlignKeyword};
     use style::values::generics::font::FontStyle;
     use style::values::specified::{
-        table::CaptionSide, BorderStyle, Clear, Display, Float, TextAlign,
+        table::CaptionSide, BorderStyle, Clear, Display, Float, TextAlign, TextEmphasisPosition
     };
 
     fn get_from_computed<T>(value: u32) -> T
@@ -5111,7 +5111,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetKeywordValue(
         Clear => get_from_computed::<Clear>(value),
         VerticalAlign => VerticalAlign::Keyword(VerticalAlignKeyword::from_u32(value).unwrap()),
         TextAlign => get_from_computed::<TextAlign>(value),
-        TextEmphasisPosition => longhands::text_emphasis_position::SpecifiedValue::from_gecko_keyword(value),
+        TextEmphasisPosition => TextEmphasisPosition::from_bits_truncate(value as u8),
         FontSize => {
             // We rely on Gecko passing in font-size values (0...7) here.
             longhands::font_size::SpecifiedValue::from_html_size(value as u8)