Commit 43f06f17 authored by Chris Nardi's avatar Chris Nardi Committed by Commit Bot

Serialize font-stretch values correctly in the font shorthand

Per the CSS Fonts 4 spec [1], only keyword values for font-stretch are
valid in the font shorthand. Our current serialization code ignores
this, and outputs percentage values as well in the shorthand, meaning
that the generated rule cannot be reparsed. We now check if the
percentage can be converted to a keyword, and if so, output it as that
keyword. Otherwise, we do not output a serialization for the font
shorthand, as per the CSSOM spec [2].

[1]: https://drafts.csswg.org/css-fonts-4/#font-prop
[2]: https://drafts.csswg.org/cssom/#serializing-css-values

Bug: 850092
Change-Id: I7e3eec64723966b15abfa819213b95cba6cbc3d5
Reviewed-on: https://chromium-review.googlesource.com/1103856
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: default avatarDominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568383}
parent 047adda3
<!doctype html>
<meta charset="utf-8">
<title>CSS Test: font shorthand serialization with font-stretch values</title>
<link rel="help" href="https://drafts.csswg.org/css-fonts-4/#propdef-font">
<link rel="help" href="https://drafts.csswg.org/cssom-1/#serializing-css-values">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<div id="test" style="font: medium serif"></div>
<script>
test(function() {
const div = document.getElementById("test");
div.style.fontStretch = "50%";
assert_equals(div.style.font, "ultra-condensed medium serif");
div.style.fontStretch = "62.5%";
assert_equals(div.style.font, "extra-condensed medium serif");
div.style.fontStretch = "75%";
assert_equals(div.style.font, "condensed medium serif");
div.style.fontStretch = "87.5%";
assert_equals(div.style.font, "semi-condensed medium serif");
div.style.fontStretch = "100%";
assert_equals(div.style.font, "medium serif", "The keyword normal should be omitted");
div.style.fontStretch = "112.5%";
assert_equals(div.style.font, "semi-expanded medium serif");
div.style.fontStretch = "125%";
assert_equals(div.style.font, "expanded medium serif");
div.style.fontStretch = "150%";
assert_equals(div.style.font, "extra-expanded medium serif");
div.style.fontStretch = "200%";
assert_equals(div.style.font, "ultra-expanded medium serif");
}, "Percentages which can be transformed into keywords should be for serialization");
test(function() {
const div = document.getElementById("test");
div.style.fontStretch = "25%";
assert_equals(div.style.font, "");
div.style.fontStretch = "101%";
assert_equals(div.style.font, "");
}, "Percentages which cannot be transformed into keywords should prevent the font shorthand from serializing");
</script>
...@@ -526,16 +526,53 @@ String StylePropertySerializer::SerializeShorthand( ...@@ -526,16 +526,53 @@ String StylePropertySerializer::SerializeShorthand(
} }
} }
void StylePropertySerializer::AppendFontLonghandValueIfNotNormal( // The font shorthand only allows keyword font-stretch values. Thus, we check if
// a percentage value can be parsed as a keyword, and if so, serialize it as
// that keyword.
const CSSValue* GetFontStretchKeyword(const CSSValue* font_stretch_value) {
if (font_stretch_value->IsIdentifierValue())
return font_stretch_value;
if (font_stretch_value->IsPrimitiveValue()) {
double value = ToCSSPrimitiveValue(font_stretch_value)->GetDoubleValue();
if (value == 50)
return CSSIdentifierValue::Create(CSSValueUltraCondensed);
if (value == 62.5)
return CSSIdentifierValue::Create(CSSValueExtraCondensed);
if (value == 75)
return CSSIdentifierValue::Create(CSSValueCondensed);
if (value == 87.5)
return CSSIdentifierValue::Create(CSSValueSemiCondensed);
if (value == 100)
return CSSIdentifierValue::Create(CSSValueNormal);
if (value == 112.5)
return CSSIdentifierValue::Create(CSSValueSemiExpanded);
if (value == 125)
return CSSIdentifierValue::Create(CSSValueExpanded);
if (value == 150)
return CSSIdentifierValue::Create(CSSValueExtraExpanded);
if (value == 200)
return CSSIdentifierValue::Create(CSSValueUltraExpanded);
}
return nullptr;
}
// Returns false if the value cannot be represented in the font shorthand
bool StylePropertySerializer::AppendFontLonghandValueIfNotNormal(
const CSSProperty& property, const CSSProperty& property,
StringBuilder& result) const { StringBuilder& result) const {
int found_property_index = property_set_.FindPropertyIndex(property); int found_property_index = property_set_.FindPropertyIndex(property);
DCHECK_NE(found_property_index, -1); DCHECK_NE(found_property_index, -1);
const CSSValue* val = property_set_.PropertyAt(found_property_index).Value(); const CSSValue* val = property_set_.PropertyAt(found_property_index).Value();
if (property.IDEquals(CSSPropertyFontStretch)) {
const CSSValue* keyword = GetFontStretchKeyword(val);
if (!keyword)
return false;
val = keyword;
}
if (val->IsIdentifierValue() && if (val->IsIdentifierValue() &&
ToCSSIdentifierValue(val)->GetValueID() == CSSValueNormal) ToCSSIdentifierValue(val)->GetValueID() == CSSValueNormal)
return; return true;
char prefix = '\0'; char prefix = '\0';
switch (property.PropertyID()) { switch (property.PropertyID()) {
...@@ -570,10 +607,11 @@ void StylePropertySerializer::AppendFontLonghandValueIfNotNormal( ...@@ -570,10 +607,11 @@ void StylePropertySerializer::AppendFontLonghandValueIfNotNormal(
"no-common-ligatures no-discretionary-ligatures " "no-common-ligatures no-discretionary-ligatures "
"no-historical-ligatures no-contextual"; "no-historical-ligatures no-contextual";
} else { } else {
value = property_set_.PropertyAt(found_property_index).Value()->CssText(); value = val->CssText();
} }
result.Append(value); result.Append(value);
return true;
} }
String StylePropertySerializer::FontValue() const { String StylePropertySerializer::FontValue() const {
...@@ -637,7 +675,10 @@ String StylePropertySerializer::FontValue() const { ...@@ -637,7 +675,10 @@ String StylePropertySerializer::FontValue() const {
AppendFontLonghandValueIfNotNormal(GetCSSPropertyFontVariantCaps(), result); AppendFontLonghandValueIfNotNormal(GetCSSPropertyFontVariantCaps(), result);
AppendFontLonghandValueIfNotNormal(GetCSSPropertyFontWeight(), result); AppendFontLonghandValueIfNotNormal(GetCSSPropertyFontWeight(), result);
AppendFontLonghandValueIfNotNormal(GetCSSPropertyFontStretch(), result); bool font_stretch_valid =
AppendFontLonghandValueIfNotNormal(GetCSSPropertyFontStretch(), result);
if (!font_stretch_valid)
return String();
if (!result.IsEmpty()) if (!result.IsEmpty())
result.Append(' '); result.Append(' ');
result.Append(font_size_property.Value()->CssText()); result.Append(font_size_property.Value()->CssText());
......
...@@ -54,7 +54,7 @@ class StylePropertySerializer { ...@@ -54,7 +54,7 @@ class StylePropertySerializer {
String separator = " ") const; String separator = " ") const;
String FontValue() const; String FontValue() const;
String FontVariantValue() const; String FontVariantValue() const;
void AppendFontLonghandValueIfNotNormal(const CSSProperty&, bool AppendFontLonghandValueIfNotNormal(const CSSProperty&,
StringBuilder& result) const; StringBuilder& result) const;
String OffsetValue() const; String OffsetValue() const;
String BackgroundRepeatPropertyValue() const; String BackgroundRepeatPropertyValue() const;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment