Bug 1759686 - Extend part 4 to resolve keyword sizes correctly, and reuse existing code to turning lengths into absolute pixels. r=jfkthame

I think it doesn't make sense to support em/percentages without
supporting smaller/bigger/other font-relative things.

I have many questions about how other units like viewport-units etc are
supposed to behave. Blink seems to resolve viewport units to zero:

  https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/resolver/font_style_resolver.cc;l=15-27;drc=5e62802ab420dc1743a1824ec4b537a730b2b24b

Which might be reasonable, but I think keeping a simple model consistent
with what other code like DOMMatrix does seems better for now.

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

layout/style/ServoCSSParser.cpp

@@ -61,10 +61,8 @@ bool ServoCSSParser::ParseFontShorthandForMatching(
     const nsACString& aValue, URLExtraData* aUrl, StyleFontFamilyList& aList,
     StyleComputedFontStyleDescriptor& aStyle, float& aStretch, float& aWeight,
     float* aSize) {
-  float dummySize;
   return Servo_ParseFontShorthandForMatching(&aValue, aUrl, &aList, &aStyle,
-                                             &aStretch, &aWeight,
-                                             aSize ? aSize : &dummySize);
+                                             &aStretch, &aWeight, aSize);
 }
 
 /* static */

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

@@ -4,6 +4,8 @@
 
 //! Specified values for font properties
 
+use crate::context::QuirksMode;
+use crate::font_metrics::FontMetricsProvider;
 use crate::parser::{Parse, ParserContext};
 use crate::values::computed::font::{FamilyName, FontFamilyList, FontStyleAngle, SingleFontFamily};
 use crate::values::computed::FontSizeAdjust as ComputedFontSizeAdjust;
@@ -829,7 +831,30 @@ impl FontSizeKeyword {
     #[cfg(feature = "gecko")]
     #[inline]
     fn to_length(&self, cx: &Context) -> NonNegativeLength {
-        use crate::context::QuirksMode;
+        let gecko_font = cx.style().get_font().gecko();
+        let family = &gecko_font.mFont.family.families;
+        unsafe {
+            Atom::with(gecko_font.mLanguage.mRawPtr, |language| {
+                self.to_length_without_context(
+                    cx.quirks_mode,
+                    cx.font_metrics_provider,
+                    language,
+                    family,
+                )
+            })
+        }
+    }
+
+    /// Resolve a keyword length without any context, with explicit arguments.
+    #[cfg(feature = "gecko")]
+    #[inline]
+    pub fn to_length_without_context(
+        &self,
+        quirks_mode: QuirksMode,
+        font_metrics_provider: &dyn FontMetricsProvider,
+        language: &Atom,
+        family: &FontFamilyList,
+    ) -> NonNegativeLength {
 
         // The tables in this function are originally from
         // nsRuleNode::CalcFontPointSize in Gecko:
@@ -875,23 +900,12 @@ impl FontSizeKeyword {
 
         static FONT_SIZE_FACTORS: [i32; 8] = [60, 75, 89, 100, 120, 150, 200, 300];
 
-        let ref gecko_font = cx.style().get_font().gecko();
-        let generic = gecko_font
-            .mFont
-            .family
-            .families
-            .single_generic()
-            .unwrap_or(computed::GenericFontFamily::None);
-        let base_size = unsafe {
-            Atom::with(gecko_font.mLanguage.mRawPtr, |atom| {
-                cx.font_metrics_provider.get_size(atom, generic)
-            })
-        };
-
+        let generic = family.single_generic().unwrap_or(computed::GenericFontFamily::None);
+        let base_size = font_metrics_provider.get_size(language, generic);
         let base_size_px = base_size.px().round() as i32;
         let html_size = self.html_size() as usize;
         NonNegative(if base_size_px >= 9 && base_size_px <= 16 {
-            let mapping = if cx.quirks_mode == QuirksMode::Quirks {
+            let mapping = if quirks_mode == QuirksMode::Quirks {
                 QUIRKS_FONT_SIZE_MAPPING
             } else {
                 FONT_SIZE_MAPPING

servo/ports/geckolib/glue.rs

@@ -139,7 +139,6 @@ use style::values::animated::{Animate, Procedure, ToAnimatedZero};
 use style::values::computed::font::{FontFamily, FontFamilyList, GenericFontFamily};
 use style::values::computed::{self, Context, ToComputedValue};
 use style::values::distance::ComputeSquaredDistance;
-use style::values::specified::FontSizeKeyword;
 use style::values::specified::gecko::IntersectionObserverRootMargin;
 use style::values::specified::source_size_list::SourceSizeList;
 use style::values::{specified, AtomIdent, CustomIdent, KeyframesName};
@@ -6965,7 +6964,7 @@ pub unsafe extern "C" fn Servo_ParseFontShorthandForMatching(
     style: &mut ComputedFontStyleDescriptor,
     stretch: &mut f32,
     weight: &mut f32,
-    size: &mut f32,
+    size: Option<&mut f32>,
 ) -> bool {
     use style::properties::shorthands::font;
     use style::values::computed::font::FontWeight as ComputedFontWeight;
@@ -6973,7 +6972,6 @@ pub unsafe extern "C" fn Servo_ParseFontShorthandForMatching(
     use style::values::specified::font::{
         FontFamily, FontSize, FontStretch, FontStyle, FontWeight, SpecifiedFontStyle,
     };
-    use style::values::specified::LengthPercentage;
 
     let string = value.as_str_unchecked();
     let mut input = ParserInput::new(&string);
@@ -7031,44 +7029,30 @@ pub unsafe extern "C" fn Servo_ParseFontShorthandForMatching(
 
     // XXX This is unfinished; see values::specified::FontSize::ToComputedValue
     // for a more complete implementation (but we can't use it as-is).
-    *size = match font.font_size {
-        FontSize::Length(lp) => {
-            match lp {
-                LengthPercentage::Length(len) => {
-                    if let Ok(len) = len.to_computed_pixel_length_without_context() {
-                        len
-                    } else {
-                        return false;
-                    }
-                },
-                LengthPercentage::Percentage(_) => return false,
-                // XXX We should support calc() here.
-                LengthPercentage::Calc(_) => return false,
+    if let Some(size) = size {
+        *size = match font.font_size {
+            FontSize::Length(lp) => {
+                use style::values::generics::transform::ToAbsoluteLength;
+                match lp.to_pixel_length(None) {
+                    Ok(len) => len,
+                    Err(..) => return false,
+                }
+            },
+            // Map absolute-size keywords to sizes.
+            FontSize::Keyword(info) => {
+                let metrics = get_metrics_provider_for_product();
+                // TODO: Maybe get a meaningful language / quirks-mode from the
+                // caller?
+                let language = atom!("x-western");
+                let quirks_mode = QuirksMode::NoQuirks;
+                info.kw.to_length_without_context(quirks_mode, &metrics, &language, family).0.px()
             }
-        },
-        // Map absolute-size keywords to sizes.
-        // XXX This doesn't account for language- and generic-based sizing
-        // differences.
-        // XXX Chrome resolves these differently in offscreen canvas; need to
-        // check whether that's a bug or is required behavior for some reason.
-        FontSize::Keyword(info) => {
-            match info.kw {
-                FontSizeKeyword::XXSmall => 9.0,
-                FontSizeKeyword::XSmall => 10.0,
-                FontSizeKeyword::Small => 13.0,
-                FontSizeKeyword::Medium => 16.0,
-                FontSizeKeyword::Large => 18.0,
-                FontSizeKeyword::XLarge => 24.0,
-                FontSizeKeyword::XXLarge => 32.0,
-                FontSizeKeyword::XXXLarge => 48.0,
-                FontSizeKeyword::None => unreachable!(),
+            // smaller, larger not currently supported
+            FontSize::Smaller | FontSize::Larger | FontSize::System(_) => {
+                return false;
             }
-        }
-        // smaller, larger not currently supported
-        FontSize::Smaller => return false,
-        FontSize::Larger => return false,
-        FontSize::System(_) => return false,
-    };
+        };
+    }
 
     true
 }