Bug 1792608 - Make vertical writing mode (left/right) in text-emphasis-position optional r=emilio

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

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,
-                             StyleTextEmphasisPosition::DEFAULT_ZH.bits);
+                             StyleTextEmphasisPosition::UNDER.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,
-                             StyleTextEmphasisPosition::DEFAULT.bits);
+                             StyleTextEmphasisPosition::OVER.bits);
     }
   }
 }

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")
-          ? StyleTextEmphasisPosition::DEFAULT_ZH
-          : StyleTextEmphasisPosition::DEFAULT;
+          ? StyleTextEmphasisPosition::UNDER
+          : StyleTextEmphasisPosition::OVER;
 }
 
 nsStyleText::nsStyleText(const nsStyleText& aSource)
@@ -3092,18 +3092,17 @@ nsChangeHint nsStyleText::CalcDifference(const nsStyleText& aNewData) const {
 }
 
 LogicalSide nsStyleText::TextEmphasisSide(WritingMode aWM) const {
-  MOZ_ASSERT((!(mTextEmphasisPosition & StyleTextEmphasisPosition::LEFT) !=
-              !(mTextEmphasisPosition & StyleTextEmphasisPosition::RIGHT)) &&
-             (!(mTextEmphasisPosition & StyleTextEmphasisPosition::OVER) !=
-              !(mTextEmphasisPosition & StyleTextEmphasisPosition::UNDER)));
-  mozilla::Side side =
-      aWM.IsVertical()
-          ? (mTextEmphasisPosition & StyleTextEmphasisPosition::LEFT
-                 ? eSideLeft
-                 : eSideRight)
-          : (mTextEmphasisPosition & StyleTextEmphasisPosition::OVER
-                 ? eSideTop
-                 : eSideBottom);
+  bool noLeftBit = !(mTextEmphasisPosition & StyleTextEmphasisPosition::LEFT);
+  DebugOnly<bool> noRightBit =
+      !(mTextEmphasisPosition & StyleTextEmphasisPosition::RIGHT);
+  bool noOverBit = !(mTextEmphasisPosition & StyleTextEmphasisPosition::OVER);
+  DebugOnly<bool> noUnderBit =
+      !(mTextEmphasisPosition & StyleTextEmphasisPosition::UNDER);
+
+  MOZ_ASSERT((noOverBit != noUnderBit) &&
+             ((noLeftBit != noRightBit) || noRightBit));
+  mozilla::Side side = aWM.IsVertical() ? (noLeftBit ? eSideRight : eSideLeft)
+                                        : (noOverBit ? eSideBottom : eSideTop);
   LogicalSide result = aWM.LogicalSideForPhysicalSide(side);
   MOZ_ASSERT(IsBlock(result));
   return result;

layout/style/test/property_database.js

@@ -7900,7 +7900,7 @@ var gCSSProperties = {
     domProp: "textEmphasisPosition",
     inherited: true,
     type: CSS_TYPE_LONGHAND,
-    initial_values: ["over right", "right over"],
+    initial_values: ["over right", "right over", "over"],
     other_values: [
       "over left",
       "left over",
@@ -7908,6 +7908,7 @@ var gCSSProperties = {
       "left under",
       "under right",
       "right under",
+      "under",
     ],
     invalid_values: [
       "over over",

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::DEFAULT",
+    "computed::TextEmphasisPosition::OVER",
     engines="gecko",
-    initial_specified_value="specified::TextEmphasisPosition::DEFAULT",
+    initial_specified_value="specified::TextEmphasisPosition::OVER",
     animation_value_type="discrete",
     spec="https://drafts.csswg.org/css-text-decor/#propdef-text-emphasis-position",
 )}

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

@@ -2264,7 +2264,7 @@ bitflags! {
 
 impl ScrollbarGutter {
     #[inline]
-    fn has_stable(self) -> bool {
+    fn has_stable(&self) -> bool {
         self.intersects(Self::STABLE)
     }
 }

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

@@ -775,7 +775,7 @@ impl Parse for TextEmphasisStyle {
 bitflags! {
     #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem, Parse, ToCss)]
     #[repr(C)]
-    #[css(bitflags(mixed="over,under,left,right", validate_mixed="Self::is_valid"))]
+    #[css(bitflags(mixed="over,under,left,right", validate_mixed="Self::validate_and_simplify"))]
     /// Values for text-emphasis-position:
     /// <https://drafts.csswg.org/css-text-decor/#text-emphasis-position-property>
     pub struct TextEmphasisPosition: u8 {
@@ -787,17 +787,21 @@ bitflags! {
         const LEFT = 1 << 2;
         /// Draws marks to the right of the text in vertical writing mode.
         const RIGHT = 1 << 3;
-        /// Returns the initial value of `text-emphasis-position`
-        const DEFAULT = Self::OVER.bits | Self::RIGHT.bits;
-        /// Non-standard behavior: Intelligent default for zh locale
-        const DEFAULT_ZH = Self::UNDER.bits | Self::RIGHT.bits;
     }
 }
 
 impl TextEmphasisPosition {
-    fn is_valid(self) -> bool {
-        return self.intersects(Self::LEFT) != self.intersects(Self::RIGHT) &&
-            self.intersects(Self::OVER) != self.intersects(Self::UNDER);
+    fn validate_and_simplify(&mut self) -> bool {
+        if self.intersects(Self::OVER) == self.intersects(Self::UNDER) {
+            return false;
+        }
+
+        if self.intersects(Self::LEFT) {
+            return !self.intersects(Self::RIGHT);
+        }
+
+        self.remove(Self::RIGHT); // Right is the default
+        true
     }
 }
 

servo/components/style_derive/parse.rs

@@ -44,7 +44,7 @@ fn parse_bitflags(bitflags: &CssBitflagAttrs) -> TokenStream {
     let mut validate_condition = quote! { !result.is_empty() };
     if let Some(ref function) = bitflags.validate_mixed {
         validate_condition.append_all(quote! {
-            && #function(result)
+            && #function(&mut result)
         });
     }
 

testing/web-platform/meta/css/css-text-decor/inheritance.html.ini

@@ -1,3 +1 @@
 [inheritance.html]
-  [Property text-emphasis-position has initial value over]
-    expected: FAIL

testing/web-platform/meta/css/css-text-decor/parsing/text-emphasis-position-computed.html.ini

@@ -1,14 +1,4 @@
 [text-emphasis-position-computed.html]
   expected:
     if (os == "android") and fission: [TIMEOUT, OK]
-  [Property text-emphasis-position value 'over']
-    expected: FAIL
 
-  [Property text-emphasis-position value 'under']
-    expected: FAIL
-
-  [Property text-emphasis-position value 'over right']
-    expected: FAIL
-
-  [Property text-emphasis-position value 'under right']
-    expected: FAIL

testing/web-platform/meta/web-animations/animation-model/animation-types/accumulation-per-property-002.html.ini

@@ -1,5 +1,3 @@
 [accumulation-per-property-002.html]
   expected:
     if (os == "win") and not debug and not fission and (processor == "x86_64"): [OK, TIMEOUT]
-  [text-emphasis-position: "over" onto "under left"]
-    expected: FAIL

testing/web-platform/meta/web-animations/animation-model/animation-types/addition-per-property-002.html.ini

@@ -1,3 +1,2 @@
 [addition-per-property-002.html]
-  [text-emphasis-position: "over" onto "under left"]
-    expected: FAIL
+