Bug 1330172 part 2 - Fix serialization of property declaration with variable reference. r=heycam
MozReview-Commit-ID: ATDj4lHrtR1
--- a/layout/style/Declaration.cpp
+++ b/layout/style/Declaration.cpp
@@ -246,28 +246,32 @@ Declaration::HasProperty(nsCSSPropertyID
? mImportantData : mData;
const nsCSSValue *val = data->ValueFor(aProperty);
return !!val;
}
bool
Declaration::AppendValueToString(nsCSSPropertyID aProperty,
nsAString& aResult,
- nsCSSValue::Serialization aSerialization) const
+ nsCSSValue::Serialization aSerialization,
+ bool* aIsTokenStream) const
{
MOZ_ASSERT(0 <= aProperty && aProperty < eCSSProperty_COUNT_no_shorthands,
"property ID out of range");
nsCSSCompressedDataBlock *data = GetPropertyIsImportantByID(aProperty)
? mImportantData : mData;
const nsCSSValue *val = data->ValueFor(aProperty);
if (!val) {
return false;
}
+ if (aIsTokenStream) {
+ *aIsTokenStream = val->GetUnit() == eCSSUnit_TokenStream;
+ }
val->AppendToString(aProperty, aResult, aSerialization);
return true;
}
static void
AppendSingleImageLayerPositionValue(const nsCSSValue& aPositionX,
const nsCSSValue& aPositionY,
const nsCSSPropertyID aTable[],
@@ -531,23 +535,26 @@ Declaration::GetImageLayerPositionValue(
aValue.Append(char16_t(','));
aValue.Append(char16_t(' '));
}
}
void
Declaration::GetPropertyValueInternal(
nsCSSPropertyID aProperty, nsAString& aValue,
- nsCSSValue::Serialization aSerialization) const
+ nsCSSValue::Serialization aSerialization, bool* aIsTokenStream) const
{
aValue.Truncate(0);
+ if (aIsTokenStream) {
+ *aIsTokenStream = false;
+ }
// simple properties are easy.
if (!nsCSSProps::IsShorthand(aProperty)) {
- AppendValueToString(aProperty, aValue, aSerialization);
+ AppendValueToString(aProperty, aValue, aSerialization, aIsTokenStream);
return;
}
// DOM Level 2 Style says (when describing CSS2Properties, although
// not CSSStyleDeclaration.getPropertyValue):
// However, if there is no shorthand declaration that could be added
// to the ruleset without changing in any way the rules already
// declared in the ruleset (i.e., by adding longhand rules that were
@@ -635,16 +642,19 @@ Declaration::GetPropertyValueInternal(
unsetCount != 0 || nonMatchingTokenStreamCount != 0) {
// Case (2): partially initial, inherit, unset or token stream.
return;
}
if (tokenStream) {
if (matchingTokenStreamCount == totalCount) {
// Shorthand was specified using variable references and all of its
// longhand components were set by the shorthand.
+ if (aIsTokenStream) {
+ *aIsTokenStream = true;
+ }
aValue.Append(tokenStream->GetTokenStreamValue()->mTokenStream);
} else {
// In all other cases, serialize to the empty string.
}
return;
}
nsCSSCompressedDataBlock *data = importantCount ? mImportantData : mData;
@@ -1600,31 +1610,39 @@ Declaration::GetPropertyIsImportantByID(
return false;
}
}
return true;
}
void
Declaration::AppendPropertyAndValueToString(nsCSSPropertyID aProperty,
+ nsAString& aResult,
nsAutoString& aValue,
- nsAString& aResult) const
+ bool aValueIsTokenStream) const
{
MOZ_ASSERT(0 <= aProperty && aProperty < eCSSProperty_COUNT,
"property enum out of range");
MOZ_ASSERT((aProperty < eCSSProperty_COUNT_no_shorthands) == aValue.IsEmpty(),
"aValue should be given for shorthands but not longhands");
AppendASCIItoUTF16(nsCSSProps::GetStringValue(aProperty), aResult);
- aResult.AppendLiteral(": ");
- if (aValue.IsEmpty())
- AppendValueToString(aProperty, aResult, nsCSSValue::eNormalized);
- else
- aResult.Append(aValue);
+ if (aValue.IsEmpty()) {
+ AppendValueToString(aProperty, aValue,
+ nsCSSValue::eNormalized, &aValueIsTokenStream);
+ }
+ aResult.Append(':');
+ if (!aValueIsTokenStream) {
+ aResult.Append(' ');
+ }
+ aResult.Append(aValue);
if (GetPropertyIsImportantByID(aProperty)) {
- aResult.AppendLiteral(" !important");
+ if (!aValueIsTokenStream) {
+ aResult.Append(' ');
+ }
+ aResult.AppendLiteral("!important");
}
aResult.AppendLiteral("; ");
}
void
Declaration::AppendVariableAndValueToString(const nsAString& aName,
nsAString& aResult) const
{
@@ -1733,42 +1751,47 @@ Declaration::ToString(nsAString& aString
for (const nsCSSPropertyID *shorthands =
nsCSSProps::ShorthandsContaining(property);
*shorthands != eCSSProperty_UNKNOWN; ++shorthands) {
// ShorthandsContaining returns the shorthands in order from those
// that contain the most subproperties to those that contain the
// least, which is exactly the order we want to test them.
nsCSSPropertyID shorthand = *shorthands;
- GetPropertyValueByID(shorthand, value);
+ bool isTokenStream;
+ GetPropertyValueInternal(shorthand, value,
+ nsCSSValue::eNormalized, &isTokenStream);
// in the system font case, skip over font-variant shorthand, since all
// subproperties are already dealt with via the font shorthand
if (shorthand == eCSSProperty_font_variant &&
value.EqualsLiteral("-moz-use-system-font")) {
continue;
}
- // If GetPropertyValueByID gives us a non-empty string back, we can
+ // If GetPropertyValueInternal gives us a non-empty string back, we can
// use that value; otherwise it's not possible to use this shorthand.
if (!value.IsEmpty()) {
- AppendPropertyAndValueToString(shorthand, value, aString);
+ AppendPropertyAndValueToString(shorthand, aString,
+ value, isTokenStream);
shorthandsUsed.AppendElement(shorthand);
doneProperty = true;
break;
}
if (shorthand == eCSSProperty_font) {
if (haveSystemFont && !didSystemFont) {
// Output the shorthand font declaration that we will
// partially override later. But don't add it to
// |shorthandsUsed|, since we will have to override it.
systemFont->AppendToString(eCSSProperty__x_system_font, value,
nsCSSValue::eNormalized);
- AppendPropertyAndValueToString(eCSSProperty_font, value, aString);
+ isTokenStream = systemFont->GetUnit() == eCSSUnit_TokenStream;
+ AppendPropertyAndValueToString(eCSSProperty_font, aString,
+ value, isTokenStream);
value.Truncate();
didSystemFont = true;
}
// That we output the system font is enough for this property if:
// (1) it's the hidden system font subproperty (which either
// means we output it or we don't have it), or
// (2) its value is the hidden system font value and it matches
@@ -1781,17 +1804,17 @@ Declaration::ToString(nsAString& aString
break;
}
}
}
if (doneProperty)
continue;
MOZ_ASSERT(value.IsEmpty(), "value should be empty now");
- AppendPropertyAndValueToString(property, value, aString);
+ AppendPropertyAndValueToString(property, aString, value, false);
}
if (! aString.IsEmpty()) {
// if the string is not empty, we have trailing whitespace we
// should remove
aString.Truncate(aString.Length() - 1);
}
}
--- a/layout/style/Declaration.h
+++ b/layout/style/Declaration.h
@@ -305,29 +305,31 @@ public:
return nullptr;
}
private:
Declaration& operator=(const Declaration& aCopy) = delete;
bool operator==(const Declaration& aCopy) const = delete;
void GetPropertyValueInternal(nsCSSPropertyID aProperty, nsAString& aValue,
- nsCSSValue::Serialization aValueSerialization)
- const;
+ nsCSSValue::Serialization aValueSerialization,
+ bool* aIsTokenStream = nullptr) const;
bool GetPropertyIsImportantByID(nsCSSPropertyID aProperty) const;
static void AppendImportanceToString(bool aIsImportant, nsAString& aString);
// return whether there was a value in |aValue| (i.e., it had a non-null unit)
bool AppendValueToString(nsCSSPropertyID aProperty, nsAString& aResult) const;
bool AppendValueToString(nsCSSPropertyID aProperty, nsAString& aResult,
- nsCSSValue::Serialization aValueSerialization) const;
+ nsCSSValue::Serialization aValueSerialization,
+ bool* aIsTokenStream = nullptr) const;
// Helper for ToString with strange semantics regarding aValue.
void AppendPropertyAndValueToString(nsCSSPropertyID aProperty,
+ nsAString& aResult,
nsAutoString& aValue,
- nsAString& aResult) const;
+ bool aValueIsTokenStream) const;
// helper for ToString that serializes a custom property declaration for
// a variable with the specified name
void AppendVariableAndValueToString(const nsAString& aName,
nsAString& aResult) const;
void GetImageLayerValue(nsCSSCompressedDataBlock *data,
nsAString& aValue,
nsCSSValue::Serialization aSerialization,
--- a/layout/style/test/test_value_storage.html
+++ b/layout/style/test/test_value_storage.html
@@ -150,16 +150,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 colon = value == "var(--a)" ? ":" : ": ";
gDeclaration.setProperty(property, value, "");
var idx;
var step1val = gDeclaration.getPropertyValue(property);
var step1vals = [];
var step1ser = gDeclaration.cssText;
if ("subproperties" in info)
@@ -188,30 +189,30 @@ function test_property(property)
}
}
// We don't care particularly about the whitespace or the placement of
// semicolons, but for simplicity we'll test the current behavior.
var expected_serialization = "";
if (step1val != "") {
if ("alias_for" in info) {
- expected_serialization = info.alias_for + ": " + step1val + ";";
+ expected_serialization = info.alias_for + colon + step1val + ";";
} else {
- expected_serialization = property + ": " + step1val + ";";
+ expected_serialization = property + colon + step1val + ";";
}
}
is(step1ser, expected_serialization,
"serialization should match property value");
gDeclaration.removeProperty(property);
gDeclaration.setProperty(property, step1val, "");
is(gDeclaration.getPropertyValue(property), step1val,
"parse+serialize should be idempotent for '" +
- property + ": " + value + "'");
+ property + colon + value + "'");
if (info.type != CSS_TYPE_TRUE_SHORTHAND) {
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
@@ -232,17 +233,17 @@ function test_property(property)
for (idx in info.subproperties) {
var subprop = info.subproperties[idx];
is(gComputedStyle.getPropertyValue(subprop), step1comps[idx],
"serialize(" + subprop + ")+parse should be the identity " +
"transform for '" + property + ": " + value + "'");
}
is(gDeclaration.getPropertyValue(property), step1val,
"parse+split+serialize should be idempotent for '" +
- property + ": " + value + "'");
+ property + colon + value + "'");
}
if (info.type != CSS_TYPE_TRUE_SHORTHAND &&
property != "mask") {
gDeclaration.removeProperty(property);
gDeclaration.setProperty(property, step1comp, "");
var func = xfail_compute(property, value) ? todo_is : is;
func(gComputedStyle.getPropertyValue(property), step1comp,
--- a/layout/style/test/test_variables.html
+++ b/layout/style/test/test_variables.html
@@ -97,19 +97,19 @@ var tests = [
test6.style.color = "white";
is(declaration.getPropertyValue("-x-system-font"), " var(--var6) hangul mongolian");
},
function() {
// https://bugzilla.mozilla.org/show_bug.cgi?id=1154356
var test7 = document.getElementById("test7");
test7.textContent = "p { --weird\\;name: green; }";
- is(test7.sheet.cssRules[0].style.cssText, "--weird\\;name: green;");
+ is(test7.sheet.cssRules[0].style.cssText, "--weird\\;name: green;");
test7.textContent = "p { --0: green; }";
- is(test7.sheet.cssRules[0].style.cssText, "--0: green;");
+ is(test7.sheet.cssRules[0].style.cssText, "--0: green;");
},
function() {
// https://bugzilla.mozilla.org/show_bug.cgi?id=1330172
var test8 = document.getElementById("test8");
test8.textContent = "p { --a:inHerit; }";
is(test8.sheet.cssRules[0].style.cssText, "--a: inherit;");
test8.textContent = "p { --b: initial!important; }";