Bug 650372 - Accept keywords in addition to strings in the @font-face format() hint function. r=emilio

The CSS Fonts 4 spec requires this, and Safari (at least) has long supported it.

Depends on D154277

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

layout/style/FontFaceSetImpl.cpp

@@ -530,50 +530,85 @@ FontFaceSetImpl::FindOrCreateUserFontEntryFromFontFace(
           face->mFormatHint = FormatHint::NONE;
 
           if (i + 1 < len) {
-            const auto& maybeFontFormat = sourceListComponents[i + 1];
-            if (maybeFontFormat.tag ==
-                StyleFontFaceSourceListComponent::Tag::FormatHint) {
-              nsDependentCSubstring valueString(
-                  reinterpret_cast<const char*>(
-                      maybeFontFormat.format_hint.utf8_bytes),
-                  maybeFontFormat.format_hint.length);
-
-              if (valueString.LowerCaseEqualsASCII("woff")) {
-                face->mFormatHint = gfxUserFontSet::FormatHint::WOFF;
-              } else if (valueString.LowerCaseEqualsASCII("woff2")) {
-                face->mFormatHint = gfxUserFontSet::FormatHint::WOFF2;
-              } else if (valueString.LowerCaseEqualsASCII("opentype")) {
-                face->mFormatHint = gfxUserFontSet::FormatHint::OPENTYPE;
-              } else if (valueString.LowerCaseEqualsASCII("truetype")) {
-                face->mFormatHint = gfxUserFontSet::FormatHint::TRUETYPE;
-              } else if (valueString.LowerCaseEqualsASCII("truetype-aat")) {
-                face->mFormatHint = gfxUserFontSet::FormatHint::TRUETYPE;
-              } else if (valueString.LowerCaseEqualsASCII(
-                             "embedded-opentype")) {
-                face->mFormatHint = gfxUserFontSet::FormatHint::EOT;
-              } else if (valueString.LowerCaseEqualsASCII("svg")) {
-                face->mFormatHint = gfxUserFontSet::FormatHint::SVG;
-              } else if (StaticPrefs::layout_css_font_variations_enabled()) {
-                // Non-standard values that Firefox accepted, for back-compat;
-                // these are superseded by the tech() function.
-                if (valueString.LowerCaseEqualsASCII("woff-variations")) {
-                  face->mFormatHint = gfxUserFontSet::FormatHint::WOFF;
-                } else if (valueString.LowerCaseEqualsASCII(
-                               "woff2-variations")) {
-                  face->mFormatHint = gfxUserFontSet::FormatHint::WOFF2;
-                } else if (valueString.LowerCaseEqualsASCII(
-                               "opentype-variations")) {
-                  face->mFormatHint = gfxUserFontSet::FormatHint::OPENTYPE;
-                } else if (valueString.LowerCaseEqualsASCII(
-                               "truetype-variations")) {
-                  face->mFormatHint = gfxUserFontSet::FormatHint::TRUETYPE;
+            // Check for a format hint.
+            const auto& next = sourceListComponents[i + 1];
+            switch (next.tag) {
+              case StyleFontFaceSourceListComponent::Tag::FormatHintKeyword:
+                switch (next.format_hint_keyword._0) {
+                  case StyleFontFaceSourceFormatKeyword::Collection:
+                    face->mFormatHint = gfxUserFontSet::FormatHint::COLLECTION;
+                    break;
+                  case StyleFontFaceSourceFormatKeyword::EmbeddedOpentype:
+                    face->mFormatHint = gfxUserFontSet::FormatHint::EOT;
+                    break;
+                  case StyleFontFaceSourceFormatKeyword::Opentype:
+                    face->mFormatHint = gfxUserFontSet::FormatHint::OPENTYPE;
+                    break;
+                  case StyleFontFaceSourceFormatKeyword::Svg:
+                    face->mFormatHint = gfxUserFontSet::FormatHint::SVG;
+                    break;
+                  case StyleFontFaceSourceFormatKeyword::Truetype:
+                    face->mFormatHint = gfxUserFontSet::FormatHint::TRUETYPE;
+                    break;
+                  case StyleFontFaceSourceFormatKeyword::Woff:
+                    face->mFormatHint = gfxUserFontSet::FormatHint::WOFF;
+                    break;
+                  case StyleFontFaceSourceFormatKeyword::Woff2:
+                    face->mFormatHint = gfxUserFontSet::FormatHint::WOFF2;
+                    break;
+                  default:
+                    MOZ_ASSERT_UNREACHABLE("bad hint!");
+                    break;
                 }
-              } else {
-                // unknown format specified, mark to distinguish from the
-                // case where no format hints are specified
-                face->mFormatHint = gfxUserFontSet::FormatHint::UNKNOWN;
+                i++;
+                break;
+              case StyleFontFaceSourceListComponent::Tag::FormatHintString: {
+                nsDependentCSubstring valueString(
+                    reinterpret_cast<const char*>(
+                        next.format_hint_string.utf8_bytes),
+                    next.format_hint_string.length);
+
+                if (valueString.LowerCaseEqualsASCII("woff")) {
+                  face->mFormatHint = gfxUserFontSet::FormatHint::WOFF;
+                } else if (valueString.LowerCaseEqualsASCII("woff2")) {
+                  face->mFormatHint = gfxUserFontSet::FormatHint::WOFF2;
+                } else if (valueString.LowerCaseEqualsASCII("opentype")) {
+                  face->mFormatHint = gfxUserFontSet::FormatHint::OPENTYPE;
+                } else if (valueString.LowerCaseEqualsASCII("truetype")) {
+                  face->mFormatHint = gfxUserFontSet::FormatHint::TRUETYPE;
+                } else if (valueString.LowerCaseEqualsASCII("truetype-aat")) {
+                  face->mFormatHint = gfxUserFontSet::FormatHint::TRUETYPE;
+                } else if (valueString.LowerCaseEqualsASCII(
+                               "embedded-opentype")) {
+                  face->mFormatHint = gfxUserFontSet::FormatHint::EOT;
+                } else if (valueString.LowerCaseEqualsASCII("svg")) {
+                  face->mFormatHint = gfxUserFontSet::FormatHint::SVG;
+                } else if (StaticPrefs::layout_css_font_variations_enabled()) {
+                  // Non-standard values that Firefox accepted, for back-compat;
+                  // these are superseded by the tech() function.
+                  if (valueString.LowerCaseEqualsASCII("woff-variations")) {
+                    face->mFormatHint = gfxUserFontSet::FormatHint::WOFF;
+                  } else if (valueString.LowerCaseEqualsASCII(
+                                 "woff2-variations")) {
+                    face->mFormatHint = gfxUserFontSet::FormatHint::WOFF2;
+                  } else if (valueString.LowerCaseEqualsASCII(
+                                 "opentype-variations")) {
+                    face->mFormatHint = gfxUserFontSet::FormatHint::OPENTYPE;
+                  } else if (valueString.LowerCaseEqualsASCII(
+                                 "truetype-variations")) {
+                    face->mFormatHint = gfxUserFontSet::FormatHint::TRUETYPE;
+                  }
+                } else {
+                  // unknown format specified, mark to distinguish from the
+                  // case where no format hints are specified
+                  face->mFormatHint = gfxUserFontSet::FormatHint::UNKNOWN;
+                }
+                i++;
+                break;
               }
-              i++;
+              case StyleFontFaceSourceListComponent::Tag::Local:
+              case StyleFontFaceSourceListComponent::Tag::Url:
+                break;
             }
           }
 
@@ -586,7 +621,8 @@ FontFaceSetImpl::FindOrCreateUserFontEntryFromFontFace(
           break;
         }
 
-        case StyleFontFaceSourceListComponent::Tag::FormatHint:
+        case StyleFontFaceSourceListComponent::Tag::FormatHintKeyword:
+        case StyleFontFaceSourceListComponent::Tag::FormatHintString:
           MOZ_ASSERT_UNREACHABLE(
               "Should always come after a URL source, and be consumed already");
           break;

servo/components/style/font_face.rs

@@ -46,22 +46,46 @@ impl OneOrMoreSeparated for Source {
     type S = Comma;
 }
 
+/// Keywords for the font-face src descriptor's format() function.
+#[derive(Clone, Copy, Debug, Eq, Parse, PartialEq, ToCss, ToShmem)]
+#[repr(u8)]
+#[allow(missing_docs)]
+pub enum FontFaceSourceFormatKeyword {
+    Collection,
+    EmbeddedOpentype,
+    Opentype,
+    Svg,
+    Truetype,
+    Woff,
+    Woff2,
+}
+
 /// A POD representation for Gecko. All pointers here are non-owned and as such
 /// can't outlive the rule they came from, but we can't enforce that via C++.
 ///
 /// All the strings are of course utf8.
 #[cfg(feature = "gecko")]
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
 #[repr(u8)]
 #[allow(missing_docs)]
 pub enum FontFaceSourceListComponent {
     Url(*const crate::gecko::url::CssUrl),
     Local(*mut crate::gecko_bindings::structs::nsAtom),
-    FormatHint {
+    FormatHintKeyword(FontFaceSourceFormatKeyword),
+    FormatHintString {
         length: usize,
         utf8_bytes: *const u8,
     },
 }
 
+#[derive(Clone, Debug, Eq, PartialEq, ToCss, ToShmem)]
+#[repr(u8)]
+#[allow(missing_docs)]
+pub enum FontFaceSourceFormat {
+    Keyword(FontFaceSourceFormatKeyword),
+    String(String),
+}
+
 /// A `UrlSource` represents a font-face source that has been specified with a
 /// `url()` function.
 ///
@@ -72,7 +96,7 @@ pub struct UrlSource {
     /// The specified url.
     pub url: SpecifiedUrl,
     /// The format hint specified with the `format()` function, if present.
-    pub format_hint: Option<String>,
+    pub format_hint: Option<FontFaceSourceFormat>,
 }
 
 impl ToCss for UrlSource {
@@ -83,7 +107,7 @@ impl ToCss for UrlSource {
         self.url.to_css(dest)?;
         if let Some(hint) = &self.format_hint {
             dest.write_str(" format(")?;
-            dest.write_str(&hint)?;
+            hint.to_css(dest)?;
             dest.write_char(')')?;
         }
         Ok(())
@@ -389,7 +413,12 @@ impl Parse for Source {
             .is_ok()
         {
             input.parse_nested_block(|input| {
-                Ok(Some(input.expect_string()?.as_ref().to_owned()))
+                if let Ok(kw) = input.try_parse(FontFaceSourceFormatKeyword::parse) {
+                    Ok(Some(FontFaceSourceFormat::Keyword(kw)))
+                } else {
+                    let s = input.expect_string()?.as_ref().to_owned();
+                    Ok(Some(FontFaceSourceFormat::String(s)))
+                }
             })?
         } else {
             None

servo/ports/geckolib/glue.rs

@@ -28,7 +28,7 @@ use style::data::{self, ElementStyles};
 use style::dom::{ShowSubtreeData, TDocument, TElement, TNode};
 use style::driver;
 use style::error_reporting::{ContextualParseError, ParseErrorReporter};
-use style::font_face::{self, FontFaceSourceListComponent, Source};
+use style::font_face::{self, FontFaceSourceListComponent, FontFaceSourceFormat, Source};
 use style::font_metrics::{get_metrics_provider_for_product, FontMetricsProvider};
 use style::gecko::data::{GeckoStyleSheet, PerDocumentStyleData, PerDocumentStyleDataImpl};
 use style::gecko::restyle_damage::GeckoRestyleDamage;
@@ -3272,10 +3272,15 @@ pub unsafe extern "C" fn Servo_FontFaceRule_GetSources(
                     Source::Url(ref url) => {
                         set_next(FontFaceSourceListComponent::Url(&url.url));
                         if let Some(hint) = &url.format_hint {
-                            set_next(FontFaceSourceListComponent::FormatHint {
-                                length: hint.len(),
-                                utf8_bytes: hint.as_ptr(),
-                            });
+                            match hint {
+                                FontFaceSourceFormat::Keyword(kw) =>
+                                    set_next(FontFaceSourceListComponent::FormatHintKeyword(*kw)),
+                                FontFaceSourceFormat::String(s) =>
+                                    set_next(FontFaceSourceListComponent::FormatHintString {
+                                        length: s.len(),
+                                        utf8_bytes: s.as_ptr(),
+                                }),
+                            }
                         }
                     },
                     Source::Local(ref name) => {

testing/web-platform/meta/css/css-fonts/parsing/font-face-src-format.html.ini

@@ -1,21 +0,0 @@
-[font-face-src-format.html]
-  [Check that src: url("foo.ttf") format(collection) is valid]
-    expected: FAIL
-
-  [Check that src: url("foo.ttf") format(embedded-opentype) is valid]
-    expected: FAIL
-
-  [Check that src: url("foo.ttf") format(opentype) is valid]
-    expected: FAIL
-
-  [Check that src: url("foo.ttf") format(svg) is valid]
-    expected: FAIL
-
-  [Check that src: url("foo.ttf") format(truetype) is valid]
-    expected: FAIL
-
-  [Check that src: url("foo.ttf") format(woff) is valid]
-    expected: FAIL
-
-  [Check that src: url("foo.ttf") format(woff2) is valid]
-    expected: FAIL