Bug 1434130 part 1 - Skip system font variant for ToCss in font subproperties. r?emilio
System font keywords are not a valid value for those properties.
The newly-added #[css(skip)] would be reused by deriving algorithm of
SpecifiedValueInfo to skip them as well.
MozReview-Commit-ID: EmnhkaA9RR5
--- a/layout/style/test/test_value_storage.html
+++ b/layout/style/test/test_value_storage.html
@@ -149,16 +149,17 @@ function test_property(property)
}
is(gDeclaration.getPropertyValue(sh), "",
"setting '" + value + "' on '" + property + "' (for shorthand '" + sh + "')");
}
}
function test_value(value, resolved_value) {
var value_has_variable_reference = resolved_value != null;
+ var is_system_font = property == "font" && value in gSystemFont;
var colon = value == "var(--a)" ? ":" : ": ";
gDeclaration.setProperty(property, value, "");
var idx;
var step1val = gDeclaration.getPropertyValue(property);
var step1vals = [];
@@ -170,17 +171,19 @@ function test_property(property)
var step1comps = [];
if (info.type != CSS_TYPE_TRUE_SHORTHAND)
step1comp = gComputedStyle.getPropertyValue(property);
if ("subproperties" in info)
for (idx in info.subproperties)
step1comps.push(gComputedStyle.getPropertyValue(info.subproperties[idx]));
SimpleTest.isnot(step1val, "", "setting '" + value + "' on '" + property + "'");
- if ("subproperties" in info)
+ if ("subproperties" in info &&
+ // System font doesn't produce meaningful value for subproperties.
+ !is_system_font)
for (idx in info.subproperties) {
var subprop = info.subproperties[idx];
if (value_has_variable_reference &&
(!info.alias_for || info.type == CSS_TYPE_TRUE_SHORTHAND)) {
is(gDeclaration.getPropertyValue(subprop), "",
"setting '" + value + "' on '" + property + "' (for '" + subprop + "')");
test_other_shorthands_empty(value, subprop);
} else {
@@ -212,17 +215,17 @@ function test_property(property)
is(gComputedStyle.getPropertyValue(property), step1comp,
"serialize+parse should be identity transform for '" +
property + ": " + value + "'");
}
if ("subproperties" in info &&
// Using setProperty over subproperties is not sufficient for
// system fonts, since the shorthand does more than its parts.
- (property != "font" || !(value in gSystemFont)) &&
+ !is_system_font &&
// Likewise for special compatibility values of transform
(property != "-moz-transform" || !value.match(/^matrix.*(px|em|%)/)) &&
!value_has_variable_reference) {
gDeclaration.removeProperty(property);
for (idx in info.subproperties) {
var subprop = info.subproperties[idx];
gDeclaration.setProperty(subprop, step1vals[idx], "");
}
@@ -245,17 +248,17 @@ function test_property(property)
property != "mask") {
gDeclaration.removeProperty(property);
gDeclaration.setProperty(property, step1comp, "");
var func = xfail_compute(property, value) ? todo_is : is;
func(gComputedStyle.getPropertyValue(property), step1comp,
"parse+compute+serialize should be idempotent for '" +
property + ": " + value + "'");
}
- if ("subproperties" in info) {
+ if ("subproperties" in info && !is_system_font) {
gDeclaration.removeProperty(property);
for (idx in info.subproperties) {
var subprop = info.subproperties[idx];
gDeclaration.setProperty(subprop, step1comps[idx], "");
}
// Now that all the subprops are set, check their values. Note that we
// need this in a separate loop, in case parts of the shorthand affect
--- a/servo/components/style/properties/helpers.mako.rs
+++ b/servo/components/style/properties/helpers.mako.rs
@@ -406,16 +406,17 @@
${gecko_keyword_conversion(keyword, keyword.values_for(product), type="T", cast_to="i32")}
}
#[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
#[derive(Clone, Copy, Debug, Eq, PartialEq, SpecifiedValueInfo, ToCss)]
pub enum SpecifiedValue {
Keyword(computed_value::T),
+ #[css(skip)]
System(SystemFont),
}
pub fn parse<'i, 't>(_: &ParserContext, input: &mut Parser<'i, 't>) -> Result<SpecifiedValue, ParseError<'i>> {
Ok(SpecifiedValue::Keyword(computed_value::T::parse(input)?))
}
impl ToComputedValue for SpecifiedValue {
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -269,16 +269,21 @@ https://drafts.csswg.org/css-fonts-4/#lo
//! While Gecko handles these as a separate property and keyword
//! values on each property indicating that the font should be picked
//! from the -x-system-font property, we avoid this. Instead,
//! each font longhand has a special SystemFont variant which contains
//! the specified system font. When the cascade function (in helpers)
//! detects that a value has a system font, it will resolve it, and
//! cache it on the ComputedValues. After this, it can be just fetched
//! whenever a font longhand on the same element needs the system font.
+ //!
+ //! When a longhand property is holding a SystemFont, it's serialized
+ //! to an empty string as if its value comes from a shorthand with
+ //! variable reference. We may want to improve this behavior at some
+ //! point. See also https://github.com/w3c/csswg-drafts/issues/1586.
use app_units::Au;
use cssparser::{Parser, ToCss};
use gecko_bindings::structs::FontFamilyType;
use properties::longhands;
use std::fmt;
use std::hash::{Hash, Hasher};
use style_traits::ParseError;
--- a/servo/components/style/values/specified/font.rs
+++ b/servo/components/style/values/specified/font.rs
@@ -81,16 +81,17 @@ pub const MAX_FONT_WEIGHT: f32 = 1000.;
pub enum FontWeight {
/// `<font-weight-absolute>`
Absolute(AbsoluteFontWeight),
/// Bolder variant
Bolder,
/// Lighter variant
Lighter,
/// System font variant.
+ #[css(skip)]
System(SystemFont),
}
impl FontWeight {
system_font_methods!(FontWeight, font_weight);
/// `normal`
#[inline]
@@ -333,16 +334,17 @@ impl SpecifiedFontStyle {
}
/// The specified value of the `font-style` property.
#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo,
ToCss)]
#[allow(missing_docs)]
pub enum FontStyle {
Specified(SpecifiedFontStyle),
+ #[css(skip)]
System(SystemFont),
}
impl FontStyle {
/// Return the `normal` value.
#[inline]
pub fn normal() -> Self {
FontStyle::Specified(generics::FontStyle::Normal)
@@ -379,16 +381,17 @@ impl Parse for FontStyle {
///
/// https://drafts.csswg.org/css-fonts-4/#font-stretch-prop
#[allow(missing_docs)]
#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo,
ToCss)]
pub enum FontStretch {
Stretch(Percentage),
Keyword(FontStretchKeyword),
+ #[css(skip)]
System(SystemFont),
}
/// A keyword value for `font-stretch`.
#[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo,
ToCss)]
#[allow(missing_docs)]
pub enum FontStretchKeyword {
@@ -520,32 +523,34 @@ pub enum FontSize {
/// will go into the offset.
/// See bug 1355707.
Keyword(KeywordInfo),
/// font-size: smaller
Smaller,
/// font-size: larger
Larger,
/// Derived from a specified system font.
+ #[css(skip)]
System(SystemFont),
}
impl From<LengthOrPercentage> for FontSize {
fn from(other: LengthOrPercentage) -> Self {
FontSize::Length(other)
}
}
/// Specifies a prioritized list of font family names or generic family names.
#[derive(Clone, Debug, Eq, Hash, PartialEq, ToCss)]
pub enum FontFamily {
/// List of `font-family`
#[css(comma)]
Values(#[css(iterable)] FontFamilyList),
/// System font
+ #[css(skip)]
System(SystemFont),
}
impl FontFamily {
system_font_methods!(FontFamily, font_family);
/// Parse a specified font-family value
pub fn parse_specified<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
@@ -627,16 +632,17 @@ impl Parse for FamilyName {
#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
/// Preserve the readability of text when font fallback occurs
pub enum FontSizeAdjust {
/// None variant
None,
/// Number variant
Number(Number),
/// system font
+ #[css(skip)]
System(SystemFont),
}
impl FontSizeAdjust {
#[inline]
/// Default value of font-size-adjust
pub fn none() -> Self {
FontSizeAdjust::None
@@ -1131,16 +1137,17 @@ impl VariantAlternatesList {
}
#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
/// Control over the selection of these alternate glyphs
pub enum FontVariantAlternates {
/// Use alternative glyph from value
Value(VariantAlternatesList),
/// Use system font glyph
+ #[css(skip)]
System(SystemFont),
}
impl FontVariantAlternates {
#[inline]
/// Get initial specified value with VariantAlternatesList
pub fn get_initial_specified_value() -> Self {
FontVariantAlternates::Value(VariantAlternatesList(vec![].into_boxed_slice()))
@@ -1384,16 +1391,17 @@ pub fn assert_variant_east_asian_matches
#[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
#[derive(Clone, Debug, PartialEq, SpecifiedValueInfo, ToCss)]
/// Allows control of glyph substitution and sizing in East Asian text.
pub enum FontVariantEastAsian {
/// Value variant with `variant-east-asian`
Value(VariantEastAsian),
/// System font variant
+ #[css(skip)]
System(SystemFont),
}
impl FontVariantEastAsian {
#[inline]
/// Get default `font-variant-east-asian` with `empty` variant
pub fn empty() -> Self {
FontVariantEastAsian::Value(VariantEastAsian::empty())
@@ -1612,16 +1620,17 @@ pub fn assert_variant_ligatures_matches(
#[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
#[derive(Clone, Debug, PartialEq, SpecifiedValueInfo, ToCss)]
/// Ligatures and contextual forms are ways of combining glyphs
/// to produce more harmonized forms
pub enum FontVariantLigatures {
/// Value variant with `variant-ligatures`
Value(VariantLigatures),
/// System font variant
+ #[css(skip)]
System(SystemFont),
}
impl FontVariantLigatures {
system_font_methods!(FontVariantLigatures, font_variant_ligatures);
/// Default value of `font-variant-ligatures` as `empty`
#[inline]
@@ -1842,16 +1851,17 @@ pub fn assert_variant_numeric_matches()
#[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
#[derive(Clone, Debug, PartialEq, SpecifiedValueInfo, ToCss)]
/// Specifies control over numerical forms.
pub enum FontVariantNumeric {
/// Value variant with `variant-numeric`
Value(VariantNumeric),
/// System font
+ #[css(skip)]
System(SystemFont),
}
impl FontVariantNumeric {
#[inline]
/// Default value of `font-variant-numeric` as `empty`
pub fn empty() -> FontVariantNumeric {
FontVariantNumeric::Value(VariantNumeric::empty())
@@ -1949,16 +1959,17 @@ pub type SpecifiedFontFeatureSettings =
/// Define initial settings that apply when the font defined by an @font-face
/// rule is rendered.
#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
pub enum FontFeatureSettings {
/// Value of `FontSettings`
Value(SpecifiedFontFeatureSettings),
/// System font
+ #[css(skip)]
System(SystemFont),
}
impl FontFeatureSettings {
#[inline]
/// Get default value of `font-feature-settings` as normal
pub fn normal() -> FontFeatureSettings {
FontFeatureSettings::Value(FontSettings::normal())
@@ -2099,16 +2110,17 @@ pub enum FontLanguageOverride {
/// the content language of the element is
/// used to infer the OpenType language system
Normal,
/// Single three-letter case-sensitive OpenType language system tag,
/// specifies the OpenType language system to be used instead of
/// the language system implied by the language of the element
Override(Box<str>),
/// Use system font
+ #[css(skip)]
System(SystemFont),
}
impl FontLanguageOverride {
#[inline]
/// Get default value with `normal`
pub fn normal() -> FontLanguageOverride {
FontLanguageOverride::Normal
@@ -2181,16 +2193,17 @@ pub type SpecifiedFontVariationSettings
/// Define initial settings that apply when the font defined by an @font-face
/// rule is rendered.
#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
pub enum FontVariationSettings {
/// Value of `FontSettings`
Value(SpecifiedFontVariationSettings),
/// System font
+ #[css(skip)]
System(SystemFont),
}
impl FontVariationSettings {
#[inline]
/// Get default value of `font-variation-settings` as normal
pub fn normal() -> FontVariationSettings {
FontVariationSettings::Value(FontSettings::normal())
--- a/servo/components/style_derive/to_css.rs
+++ b/servo/components/style_derive/to_css.rs
@@ -73,16 +73,19 @@ fn derive_variant_arm(
generics: &mut Option<WhereClause>,
) -> Tokens {
let bindings = variant.bindings();
let identifier = cg::to_css_identifier(variant.ast().ident.as_ref());
let ast = variant.ast();
let variant_attrs = cg::parse_variant_attrs_from_ast::<CssVariantAttrs>(&ast);
let separator = if variant_attrs.comma { ", " } else { " " };
+ if variant_attrs.skip {
+ return quote!(Ok(()));
+ }
if variant_attrs.dimension {
assert_eq!(bindings.len(), 1);
assert!(
variant_attrs.function.is_none() && variant_attrs.keyword.is_none(),
"That makes no sense"
);
}
@@ -218,16 +221,17 @@ pub struct CssInputAttrs {
#[darling(attributes(css), default)]
#[derive(Default, FromVariant)]
pub struct CssVariantAttrs {
pub function: Option<Override<String>>,
pub comma: bool,
pub dimension: bool,
pub keyword: Option<String>,
pub aliases: Option<String>,
+ pub skip: bool,
}
#[darling(attributes(css), default)]
#[derive(Default, FromField)]
pub struct CssFieldAttrs {
pub if_empty: Option<String>,
pub field_bound: bool,
pub iterable: bool,