From 4d5744ae693cdc4dc67e6dfebdb5d451d002c1ee Mon Sep 17 00:00:00 2001 From: Birunthan Mohanathas Date: Tue, 2 Jul 2013 08:10:43 -0400 Subject: [PATCH] Bug 887502 - Part 1: Coalesce corners rather than repeating when serializing specified values of 'border-radius'. r=dbaron --- layout/style/Declaration.cpp | 51 ++++++++++++++----- .../test/test_priority_preservation.html | 8 +-- .../test/test_shorthand_property_getters.html | 9 ++++ 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/layout/style/Declaration.cpp b/layout/style/Declaration.cpp index 0699715bedb..e3f723337a2 100644 --- a/layout/style/Declaration.cpp +++ b/layout/style/Declaration.cpp @@ -107,6 +107,37 @@ Declaration::AppendValueToString(nsCSSProperty aProperty, return true; } +// Helper to append |aString| with the shorthand sides notation used in e.g. +// 'padding'. |aProperties| and |aValues| are expected to have 4 elements. +static void +AppendSidesShorthandToString(const nsCSSProperty aProperties[], + const nsCSSValue* aValues[], + nsAString& aString) +{ + const nsCSSValue& value1 = *aValues[0]; + const nsCSSValue& value2 = *aValues[1]; + const nsCSSValue& value3 = *aValues[2]; + const nsCSSValue& value4 = *aValues[3]; + + NS_ABORT_IF_FALSE(value1.GetUnit() != eCSSUnit_Null, "null value 1"); + value1.AppendToString(aProperties[0], aString); + if (value1 != value2 || value1 != value3 || value1 != value4) { + aString.Append(PRUnichar(' ')); + NS_ABORT_IF_FALSE(value2.GetUnit() != eCSSUnit_Null, "null value 2"); + value2.AppendToString(aProperties[1], aString); + if (value1 != value3 || value2 != value4) { + aString.Append(PRUnichar(' ')); + NS_ABORT_IF_FALSE(value3.GetUnit() != eCSSUnit_Null, "null value 3"); + value3.AppendToString(aProperties[2], aString); + if (value2 != value4) { + aString.Append(PRUnichar(' ')); + NS_ABORT_IF_FALSE(value4.GetUnit() != eCSSUnit_Null, "null value 4"); + value4.AppendToString(aProperties[3], aString); + } + } + } +} + void Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const { @@ -240,28 +271,22 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const // For compatibility, only write a slash and the y-values // if they're not identical to the x-values. bool needY = false; + const nsCSSValue* xVals[4]; + const nsCSSValue* yVals[4]; for (int i = 0; i < 4; i++) { if (vals[i]->GetUnit() == eCSSUnit_Pair) { needY = true; - vals[i]->GetPairValue().mXValue.AppendToString(subprops[i], aValue); + xVals[i] = &vals[i]->GetPairValue().mXValue; + yVals[i] = &vals[i]->GetPairValue().mYValue; } else { - vals[i]->AppendToString(subprops[i], aValue); + xVals[i] = yVals[i] = vals[i]; } - if (i < 3) - aValue.Append(PRUnichar(' ')); } + AppendSidesShorthandToString(subprops, xVals, aValue); if (needY) { aValue.AppendLiteral(" / "); - for (int i = 0; i < 4; i++) { - if (vals[i]->GetUnit() == eCSSUnit_Pair) { - vals[i]->GetPairValue().mYValue.AppendToString(subprops[i], aValue); - } else { - vals[i]->AppendToString(subprops[i], aValue); - } - if (i < 3) - aValue.Append(PRUnichar(' ')); - } + AppendSidesShorthandToString(subprops, yVals, aValue); } break; } diff --git a/layout/style/test/test_priority_preservation.html b/layout/style/test/test_priority_preservation.html index 346f4390da6..080a4651c76 100644 --- a/layout/style/test/test_priority_preservation.html +++ b/layout/style/test/test_priority_preservation.html @@ -117,22 +117,22 @@ is(s.getPropertyPriority("z-index"), "", "z-index priority still stored"); s.setProperty("border-radius", "2em", ""); -is(s.getPropertyValue("border-radius"), "2em 2em 2em 2em", +is(s.getPropertyValue("border-radius"), "2em", "border-radius serialization 1") s.setProperty("border-top-left-radius", "3em 4em", ""); is(s.getPropertyValue("border-radius"), - "3em 2em 2em 2em / 4em 2em 2em 2em", + "3em 2em 2em / 4em 2em 2em", "border-radius serialization 2"); s.setProperty("border-radius", "2em / 3em", ""); is(s.getPropertyValue("border-radius"), - "2em 2em 2em 2em / 3em 3em 3em 3em", + "2em / 3em", "border-radius serialization 3") s.setProperty("border-top-left-radius", "4em", ""); is(s.getPropertyValue("border-radius"), - "4em 2em 2em 2em / 4em 3em 3em 3em", + "4em 2em 2em / 4em 3em 3em", "border-radius serialization 3"); diff --git a/layout/style/test/test_shorthand_property_getters.html b/layout/style/test/test_shorthand_property_getters.html index a8a8bceb884..355ecb723a7 100644 --- a/layout/style/test/test_shorthand_property_getters.html +++ b/layout/style/test/test_shorthand_property_getters.html @@ -96,6 +96,15 @@ is(e.style.cssText, "border-color: currentcolor;", "should condense to canonical e.setAttribute("style", "border-style: ridge none none none"); is(e.style.borderStyle, "ridge none none", "should condense"); is(e.style.cssText, "border-style: ridge none none;", "should condense"); +e.setAttribute("style", "border-radius: 1px 1px 1px 1px"); +is(e.style.borderRadius, "1px", "should condense to shortest possible"); +is(e.style.cssText, "border-radius: 1px;", "should condense to shortest possible"); +e.setAttribute("style", "border-radius: 1px 1px 1px 1px / 2px 2px 2px 2px"); +is(e.style.borderRadius, "1px / 2px", "should condense to shortest possible"); +is(e.style.cssText, "border-radius: 1px / 2px;", "should condense to shortest possible"); +e.setAttribute("style", "border-radius: 1px 2px 1px 2px / 1% 2% 3% 2%"); +is(e.style.borderRadius, "1px 2px / 1% 2% 3%", "should condense to shortest possible"); +is(e.style.cssText, "border-radius: 1px 2px / 1% 2% 3%;", "should condense to shortest possible"); // Test that we refuse to serialize the 'background' and 'font' // shorthands when some subproperties that can't be expressed in the