From d7e00ba03d0ed8d7382ff7b04103bec8c04c2e5a Mon Sep 17 00:00:00 2001
From: Mats Palmgren <mats@mozilla.com>
Date: Sun, 21 May 2023 23:39:34 +0200
Subject: [PATCH] style: Add some style quirks for legacy ::markers created
 from list-style-type/list-style-image

Specifically:
For "bullets", i.e. 'list-style-type:disc|circle|square|
disclosure-closed|disclosure-open', we use a built-in font
(-moz-bullet-font, which has glyphs for those symbols + space) to
retain mostly backwards compatible rendering for those.  Authors may
override that with an explicit 'font-family' ::marker style though.
We also use this font for 'list-style-image' in case it would
fallback to one of the above when the image fails to load (so that
we get the same width space).

When the -moz-bullet-font is used we also set 'font-synthesis' to
avoid synthesizing italic/bold for this font. Authors may override
this with an explicit ::marker declaration.

We also set 'letter-spacing' and 'word-spacing' to the initial value
for bullets for web-compat reasons.  Again, authors may override
this with an explicit ::marker declaration. (This breaks backwards-
compat slightly but makes us compatible with Chrome.  We used to
ignore these for list-style-type:<string> too.)

Differential Revision: https://phabricator.services.mozilla.com/D111693
---
 components/style/properties/cascade.rs        | 28 +++++++++++
 .../style/properties/computed_value_flags.rs  | 14 +++++-
 components/style/style_adjuster.rs            | 47 +++++++++++++++++++
 components/style/values/computed/text.rs      |  8 ++++
 components/style/values/generics/mod.rs       | 15 ++++++
 components/style/values/specified/font.rs     |  8 ++++
 components/style/values/specified/list.rs     |  9 ++++
 7 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs
index 9c3e6ebd85e..1368cf9c8e7 100644
--- a/components/style/properties/cascade.rs
+++ b/components/style/properties/cascade.rs
@@ -786,6 +786,34 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
             builder.add_flags(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_PADDING);
         }
 
+        if self
+            .author_specified
+            .contains(LonghandId::FontFamily)
+        {
+            builder.add_flags(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_FONT_FAMILY);
+        }
+
+        if self
+            .author_specified
+            .contains(LonghandId::LetterSpacing)
+        {
+            builder.add_flags(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_LETTER_SPACING);
+        }
+
+        if self
+            .author_specified
+            .contains(LonghandId::WordSpacing)
+        {
+            builder.add_flags(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_WORD_SPACING);
+        }
+
+        if self
+            .author_specified
+            .contains(LonghandId::FontSynthesis)
+        {
+            builder.add_flags(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_FONT_SYNTHESIS);
+        }
+
         #[cfg(feature = "servo")]
         {
             if let Some(font) = builder.get_font_if_mutated() {
diff --git a/components/style/properties/computed_value_flags.rs b/components/style/properties/computed_value_flags.rs
index cc7d9c94fb9..b3e5fe4f7f9 100644
--- a/components/style/properties/computed_value_flags.rs
+++ b/components/style/properties/computed_value_flags.rs
@@ -12,7 +12,7 @@ bitflags! {
     /// If we ever want to add some flags that shouldn't inherit for them,
     /// we might want to add a function to handle this.
     #[repr(C)]
-    pub struct ComputedValueFlags: u16 {
+    pub struct ComputedValueFlags: u32 {
         /// Whether the style or any of the ancestors has a text-decoration-line
         /// property that should get propagated to descendants.
         ///
@@ -89,6 +89,18 @@ bitflags! {
         /// FIXME(emilio): Try to merge this with BORDER_BACKGROUND, see
         /// https://github.com/w3c/csswg-drafts/issues/4777
         const HAS_AUTHOR_SPECIFIED_PADDING = 1 << 15;
+
+        /// Whether there are author-specified rules for `font-family`.
+        const HAS_AUTHOR_SPECIFIED_FONT_FAMILY = 1 << 16;
+
+        /// Whether there are author-specified rules for `font-synthesis`.
+        const HAS_AUTHOR_SPECIFIED_FONT_SYNTHESIS = 1 << 17;
+
+        /// Whether there are author-specified rules for `letter-spacing`.
+        const HAS_AUTHOR_SPECIFIED_LETTER_SPACING = 1 << 18;
+
+        /// Whether there are author-specified rules for `word-spacing`.
+        const HAS_AUTHOR_SPECIFIED_WORD_SPACING = 1 << 19;
     }
 }
 
diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs
index 158be6d5dfb..bfd43da6088 100644
--- a/components/style/style_adjuster.rs
+++ b/components/style/style_adjuster.rs
@@ -818,6 +818,52 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         }
     }
 
+    /// A legacy ::marker (i.e. no 'content') without an author-specified 'font-family'
+    /// and 'list-style-type:disc|circle|square|disclosure-closed|disclosure-open'
+    /// is assigned 'font-family:-moz-bullet-font'. (This is for <ul><li> etc.)
+    /// We don't want synthesized italic/bold for this font, so turn that off too.
+    /// Likewise for 'letter/word-spacing' -- unless the author specified it then reset
+    /// them to their initial value because traditionally we never added such spacing
+    /// between a legacy bullet and the list item's content, so we keep that behavior
+    /// for web-compat reasons.
+    /// We intentionally don't check 'list-style-image' below since we want it to use
+    /// the same font as its fallback ('list-style-type') in case it fails to load.
+    #[cfg(feature = "gecko")]
+    fn adjust_for_marker_pseudo(&mut self) {
+        use crate::values::computed::font::{FamilyName, FontFamily, FontFamilyList, FontFamilyNameSyntax, FontSynthesis, SingleFontFamily};
+        use crate::values::computed::text::{LetterSpacing, WordSpacing};
+
+        let is_legacy_marker = self.style.pseudo.map_or(false, |p| p.is_marker()) &&
+            self.style.get_counters().ineffective_content_property() &&
+            self.style.get_list().clone_list_style_type().is_bullet();
+        if !is_legacy_marker {
+            return;
+        }
+        if !self.style.flags.get().contains(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_FONT_FAMILY) {
+            let moz_bullet_font_family = FontFamily {
+                families: FontFamilyList::new(Box::new([SingleFontFamily::FamilyName(FamilyName {
+                    name: atom!("-moz-bullet-font"),
+                    syntax: FontFamilyNameSyntax::Identifiers,
+                })])),
+                is_system_font: false,
+            };
+            self.style.mutate_font().set_font_family(moz_bullet_font_family);
+
+            // FIXME(mats): We can remove this if support for font-synthesis is added to @font-face rules.
+            // Then we can add it to the @font-face rule in html.css instead.
+            // https://github.com/w3c/csswg-drafts/issues/6081
+            if !self.style.flags.get().contains(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_FONT_SYNTHESIS) {
+                self.style.mutate_font().set_font_synthesis(FontSynthesis::none());
+            }
+        }
+        if !self.style.flags.get().contains(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_LETTER_SPACING) {
+            self.style.mutate_inherited_text().set_letter_spacing(LetterSpacing::normal());
+        }
+        if !self.style.flags.get().contains(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_WORD_SPACING) {
+            self.style.mutate_inherited_text().set_word_spacing(WordSpacing::normal());
+        }
+    }
+
     /// Adjusts the style to account for various fixups that don't fit naturally
     /// into the cascade.
     ///
@@ -883,6 +929,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         {
             self.adjust_for_appearance(element);
             self.adjust_for_inert();
+            self.adjust_for_marker_pseudo();
         }
         self.set_bits();
     }
diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs
index b42dfb488a3..629626b06b3 100644
--- a/components/style/values/computed/text.rs
+++ b/components/style/values/computed/text.rs
@@ -110,6 +110,14 @@ impl ToComputedValue for specified::WordSpacing {
 /// A computed value for the `line-height` property.
 pub type LineHeight = GenericLineHeight<NonNegativeNumber, NonNegativeLength>;
 
+impl WordSpacing {
+    /// Return the `normal` computed value, which is just zero.
+    #[inline]
+    pub fn normal() -> Self {
+        LengthPercentage::zero()
+    }
+}
+
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToResolvedValue)]
 #[repr(C)]
 /// text-overflow.
diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs
index 39c898efed7..c0cdf04b71d 100644
--- a/components/style/values/generics/mod.rs
+++ b/components/style/values/generics/mod.rs
@@ -95,6 +95,21 @@ impl CounterStyle {
     pub fn decimal() -> Self {
         CounterStyle::Name(CustomIdent(atom!("decimal")))
     }
+
+    /// Is this a bullet? (i.e. `list-style-type: disc|circle|square|disclosure-closed|disclosure-open`)
+    #[inline]
+    pub fn is_bullet(&self) -> bool {
+        match self {
+            CounterStyle::Name(CustomIdent(ref name)) => {
+                name == &atom!("disc") ||
+                    name == &atom!("circle") ||
+                    name == &atom!("square") ||
+                    name == &atom!("disclosure-closed") ||
+                    name == &atom!("disclosure-open")
+            }
+            _ => false,
+        }
+    }
 }
 
 impl Parse for CounterStyle {
diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs
index 329dedd31ca..b007bc3f5a4 100644
--- a/components/style/values/specified/font.rs
+++ b/components/style/values/specified/font.rs
@@ -2005,6 +2005,14 @@ impl FontSynthesis {
             style: true,
         }
     }
+    #[inline]
+    /// Get the 'none' value of font-synthesis
+    pub fn none() -> Self {
+        FontSynthesis {
+            weight: false,
+            style: false,
+        }
+    }
 }
 
 impl Parse for FontSynthesis {
diff --git a/components/style/values/specified/list.rs b/components/style/values/specified/list.rs
index 5d31c0b34d2..14004e65cc8 100644
--- a/components/style/values/specified/list.rs
+++ b/components/style/values/specified/list.rs
@@ -67,6 +67,15 @@ impl ListStyleType {
             _ => unreachable!("Unknown counter style keyword value"),
         })))
     }
+
+    /// Is this a bullet? (i.e. `list-style-type: disc|circle|square|disclosure-closed|disclosure-open`)
+    #[inline]
+    pub fn is_bullet(&self) -> bool {
+        match self {
+            ListStyleType::CounterStyle(ref style) => style.is_bullet(),
+            _ => false,
+        }
+    }
 }
 
 #[cfg(feature = "gecko")]