Diff
Modified: trunk/LayoutTests/ChangeLog (115226 => 115227)
--- trunk/LayoutTests/ChangeLog 2012-04-25 17:32:24 UTC (rev 115226)
+++ trunk/LayoutTests/ChangeLog 2012-04-25 17:44:29 UTC (rev 115227)
@@ -1,3 +1,18 @@
+2012-04-24 Ryosuke Niwa <rn...@webkit.org>
+
+ REGRESSION(r112177): listStyleType CSS property gets converted into listStyle
+ https://bugs.webkit.org/show_bug.cgi?id=83026
+
+ Reviewed by Darin Adler.
+
+ Added test cases to cssText-shorthand.html and rebaselined some tests.
+
+ * fast/css/cssText-shorthand-expected.txt:
+ * fast/css/cssText-shorthand.html: Added more test cases.
+ * fast/css/remove-shorthand-expected.txt: Correctly adds both border-width and border-style
+ when border-color is missing.
+ * printing/page-rule-css-text-expected.txt: Correctly uses border-width instead of border.
+
2012-04-25 Nate Chapin <jap...@chromium.org>
Test for https://bugs.webkit.org/show_bug.cgi?id=83918.
Modified: trunk/LayoutTests/fast/css/cssText-shorthand-expected.txt (115226 => 115227)
--- trunk/LayoutTests/fast/css/cssText-shorthand-expected.txt 2012-04-25 17:32:24 UTC (rev 115226)
+++ trunk/LayoutTests/fast/css/cssText-shorthand-expected.txt 2012-04-25 17:44:29 UTC (rev 115227)
@@ -8,18 +8,21 @@
PASS normalizeCssText(element.style.cssText) is "border: 1px red"
PASS normalizeCssText(element.style.cssText) is "border: red"
PASS normalizeCssText(element.style.cssText) is "border: 1px"
-PASS normalizeCssText(element.style.cssText) is "border-bottom-width: 3px; border-left-width: 4px; border-right-width: 2px; border-top-width: 1px"
-PASS normalizeCssText(element.style.cssText) is "border-bottom-width: 1px; border-left-width: 1px; border-right-width: 1px; border-top-width: 2px"
+PASS normalizeCssText(element.style.cssText) is "border-width: 1px 2px 3px 4px"
+PASS normalizeCssText(element.style.cssText) is "border-width: 2px 1px 1px"
PASS normalizeCssText(element.style.cssText) is "border-bottom-width: 1px; border-left-width: 1px; border-right-width: 1px; border-top-width: 1px !important"
-PASS normalizeCssText(element.style.cssText) is "border-bottom-width: 1px; border-left-width: 1px; border-right-width: 1px; border-top-color: red; border-top-width: 1px"
+PASS normalizeCssText(element.style.cssText) is "border-top-color: red; border-width: 1px"
PASS normalizeCssText(element.style.cssText) is "border: dotted"
+PASS normalizeCssText(element.style.cssText) is "border-width: 1px"
PASS normalizeCssText(element.style.cssText) is "border-spacing: 1px 2px"
PASS normalizeCssText(element.style.cssText) is "font-family: sans-serif; font-size: 3em; font-style: italic; font-weight: bold; line-height: 2em"
PASS normalizeCssText(element.style.cssText) is "list-style: circle inside"
PASS normalizeCssText(element.style.cssText) is "margin: 1px 2px 3px 4px"
PASS normalizeCssText(element.style.cssText) is "outline: blue dotted 2px"
FAIL normalizeCssText(element.style.cssText) should be overflow: scroll hidden. Was overflow-x: scroll; overflow-y: hidden.
+PASS normalizeCssText(element.style.cssText) is "overflow: scroll"
PASS normalizeCssText(element.style.cssText) is "padding: 1px 2px 3px 4px"
+PASS normalizeCssText(element.style.cssText) is "list-style-type: lower-alpha"
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/fast/css/cssText-shorthand.html (115226 => 115227)
--- trunk/LayoutTests/fast/css/cssText-shorthand.html 2012-04-25 17:32:24 UTC (rev 115226)
+++ trunk/LayoutTests/fast/css/cssText-shorthand.html 2012-04-25 17:44:29 UTC (rev 115227)
@@ -13,15 +13,14 @@
['border: 1px red;', 'border: 1px red;'],
['border: red;', 'border: red;'],
['border-top: 1px; border-right: 1px; border-bottom: 1px; border-left: 1px;', 'border: 1px;'],
- ['border-top: 1px; border-right: 2px; border-bottom: 3px; border-left: 4px;',
- 'border-top-width: 1px; border-right-width: 2px; border-bottom-width: 3px; border-left-width: 4px;'],
- ['border: 1px; border-top: 2px;',
- 'border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-width: 2px;'],
+ ['border-top: 1px; border-right: 2px; border-bottom: 3px; border-left: 4px;', 'border-width: 1px 2px 3px 4px;'],
+ ['border: 1px; border-top: 2px;', 'border-width: 2px 1px 1px;'],
['border: 1px; border-top: 1px !important;',
'border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-width: 1px !important;'],
- ['border: 1px; border-top-color: red;',
- 'border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-width: 1px; border-top-color: red;'],
+
+ ['border: 1px; border-top-color: red;', 'border-width: 1px; border-top-color: red;'],
['border: solid; border-style: dotted', 'border: dotted;'],
+ ['border-width: 1px;', 'border-width: 1px;'],
['-webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 2px;', 'border-spacing: 1px 2px;'],
@@ -33,7 +32,10 @@
['margin-top: 1px; margin-right: 2px; margin-bottom: 3px; margin-left: 4px;', 'margin: 1px 2px 3px 4px;'],
['outline-width: 2px; outline-style: dotted; outline-color: blue;', 'outline: blue dotted 2px;'],
['overflow-x: scroll; overflow-y: hidden;', 'overflow: scroll hidden;'],
+ ['overflow-x: scroll; overflow-y: scroll;', 'overflow: scroll;'],
['padding-top: 1px; padding-right: 2px; padding-bottom: 3px; padding-left: 4px;', 'padding: 1px 2px 3px 4px;'],
+
+ ['list-style-type: lower-alpha;', 'list-style-type: lower-alpha;']
];
function normalizeCssText(text) { return text.trim().split(/;\s*/).sort().slice(1).join("; "); }
Modified: trunk/LayoutTests/fast/css/remove-shorthand-expected.txt (115226 => 115227)
--- trunk/LayoutTests/fast/css/remove-shorthand-expected.txt 2012-04-25 17:32:24 UTC (rev 115226)
+++ trunk/LayoutTests/fast/css/remove-shorthand-expected.txt 2012-04-25 17:44:29 UTC (rev 115227)
@@ -25,13 +25,13 @@
and adds "border-top-width, border-right-width, border-bottom-width, border-top-style, border-right-style, border-bottom-style, border-top-color, border-right-color, border-bottom-color".
Removing border-color
removes "border"
-and adds "border".
+and adds "border-width, border-style".
Removing border-style
removes "border"
-and adds "border".
+and adds "border-width, border-color".
Removing border-width
removes "border"
-and adds "border".
+and adds "border-style, border-color".
Removing border-radius
removes "border-top-left-radius, border-top-right-radius, border-bottom-right-radius, border-bottom-left-radius"
and adds "".
Modified: trunk/LayoutTests/printing/page-rule-css-text-expected.txt (115226 => 115227)
--- trunk/LayoutTests/printing/page-rule-css-text-expected.txt 2012-04-25 17:32:24 UTC (rev 115226)
+++ trunk/LayoutTests/printing/page-rule-css-text-expected.txt 2012-04-25 17:44:29 UTC (rev 115227)
@@ -1,7 +1,7 @@
@page { margin-top: 5cm; margin-bottom: 10cm; }
@page :left { margin-right: 3cm; }
@page :right { margin-left: 3cm; }
-@page :first { border: 1px; }
+@page :first { border-width: 1px; }
@page hello { color: green; }
@page world:right { background-color: green; }
@media print { @page somepage:first { margin: 3cm; } }
Modified: trunk/Source/WebCore/ChangeLog (115226 => 115227)
--- trunk/Source/WebCore/ChangeLog 2012-04-25 17:32:24 UTC (rev 115226)
+++ trunk/Source/WebCore/ChangeLog 2012-04-25 17:44:29 UTC (rev 115227)
@@ -1,3 +1,25 @@
+2012-04-24 Ryosuke Niwa <rn...@webkit.org>
+
+ REGRESSION(r112177): listStyleType CSS property gets converted into listStyle
+ https://bugs.webkit.org/show_bug.cgi?id=83026
+
+ Reviewed by Darin Adler.
+
+ Fixed the bug by not using shorthand notations when some values are missing.
+
+ However, we still want to return a value when shorthand border property is explicitly
+ requested so extract borderPropertyValue with a flag to support both behaviors.
+
+ * css/StylePropertySet.cpp:
+ (WebCore::borderPropertyValue): Extracted from getPropertyValue.
+ (WebCore::StylePropertySet::getPropertyValue):
+ (WebCore::StylePropertySet::get4Values): Don't return values when priority don't match.
+ (WebCore::StylePropertySet::getShorthandValue):
+ (WebCore::StylePropertySet::getCommonValue): Don't return null string for initial values
+ to disambiguate missing values and "initial" in getPropertyValue. Also check propriety.
+ (WebCore::StylePropertySet::asText): Support emitting border-width, border-style, and
+ border-color when border doesn't work but the former properties do.
+
2012-04-25 Ian Vollick <voll...@chromium.org>
[chromium] Do not clobber synchronized start times.
Modified: trunk/Source/WebCore/css/StylePropertySet.cpp (115226 => 115227)
--- trunk/Source/WebCore/css/StylePropertySet.cpp 2012-04-25 17:32:24 UTC (rev 115226)
+++ trunk/Source/WebCore/css/StylePropertySet.cpp 2012-04-25 17:44:29 UTC (rev 115227)
@@ -117,19 +117,8 @@
return getLayeredShorthandValue(backgroundRepeatShorthand());
case CSSPropertyBackground:
return getLayeredShorthandValue(backgroundShorthand());
- case CSSPropertyBorder: {
- const StylePropertyShorthand properties[3] = { borderWidthShorthand(), borderStyleShorthand(), borderColorShorthand() };
- String res;
- for (size_t i = 0; i < WTF_ARRAY_LENGTH(properties); ++i) {
- String value = getCommonValue(properties[i]);
- if (!value.isNull()) {
- if (!res.isNull())
- res += " ";
- res += value;
- }
- }
- return res;
- }
+ case CSSPropertyBorder:
+ return borderPropertyValue(OmitUncommonValues);
case CSSPropertyBorderTop:
return getShorthandValue(borderTopShorthand());
case CSSPropertyBorderRight:
@@ -264,26 +253,30 @@
String StylePropertySet::get4Values(const StylePropertyShorthand& shorthand) const
{
// Assume the properties are in the usual order top, right, bottom, left.
- RefPtr<CSSValue> topValue = getPropertyCSSValue(shorthand.properties()[0]);
- RefPtr<CSSValue> rightValue = getPropertyCSSValue(shorthand.properties()[1]);
- RefPtr<CSSValue> bottomValue = getPropertyCSSValue(shorthand.properties()[2]);
- RefPtr<CSSValue> leftValue = getPropertyCSSValue(shorthand.properties()[3]);
+ const CSSProperty* top = findPropertyWithId(shorthand.properties()[0]);
+ const CSSProperty* right = findPropertyWithId(shorthand.properties()[1]);
+ const CSSProperty* bottom = findPropertyWithId(shorthand.properties()[2]);
+ const CSSProperty* left = findPropertyWithId(shorthand.properties()[3]);
// All 4 properties must be specified.
- if (!topValue || !rightValue || !bottomValue || !leftValue)
+ if (!top || !top->value() || !right || !right->value() || !bottom || !bottom->value() || !left || !left->value())
return String();
+ if (top->value()->isInitialValue() || right->value()->isInitialValue() || bottom->value()->isInitialValue() || bottom->value()->isInitialValue())
+ return String();
+ if (top->isImportant() != right->isImportant() || right->isImportant() != bottom->isImportant() || bottom->isImportant() != left->isImportant())
+ return String();
- bool showLeft = rightValue->cssText() != leftValue->cssText();
- bool showBottom = (topValue->cssText() != bottomValue->cssText()) || showLeft;
- bool showRight = (topValue->cssText() != rightValue->cssText()) || showBottom;
+ bool showLeft = right->value()->cssText() != left->value()->cssText();
+ bool showBottom = (top->value()->cssText() != bottom->value()->cssText()) || showLeft;
+ bool showRight = (top->value()->cssText() != right->value()->cssText()) || showBottom;
- String res = topValue->cssText();
+ String res = top->value()->cssText();
if (showRight)
- res += " " + rightValue->cssText();
+ res += " " + right->value()->cssText();
if (showBottom)
- res += " " + bottomValue->cssText();
+ res += " " + bottom->value()->cssText();
if (showLeft)
- res += " " + leftValue->cssText();
+ res += " " + left->value()->cssText();
return res;
}
@@ -395,10 +388,10 @@
for (unsigned i = 0; i < shorthand.length(); ++i) {
if (!isPropertyImplicit(shorthand.properties()[i])) {
RefPtr<CSSValue> value = getPropertyCSSValue(shorthand.properties()[i]);
- if (value && value->isInitialValue())
- continue;
if (!value)
return String();
+ if (value->isInitialValue())
+ continue;
if (!res.isNull())
res += " ";
res += value->cssText();
@@ -411,10 +404,11 @@
String StylePropertySet::getCommonValue(const StylePropertyShorthand& shorthand) const
{
String res;
+ bool lastPropertyWasImportant = false;
for (unsigned i = 0; i < shorthand.length(); ++i) {
RefPtr<CSSValue> value = getPropertyCSSValue(shorthand.properties()[i]);
// FIXME: CSSInitialValue::cssText should generate the right value.
- if (!value || value->isInitialValue())
+ if (!value)
return String();
String text = value->cssText();
if (text.isNull())
@@ -423,10 +417,36 @@
res = text;
else if (res != text)
return String();
+
+ bool currentPropertyIsImportant = propertyIsImportant(shorthand.properties()[i]);
+ if (i && lastPropertyWasImportant != currentPropertyIsImportant)
+ return String();
+ lastPropertyWasImportant = currentPropertyIsImportant;
}
return res;
}
+String StylePropertySet::borderPropertyValue(CommonValueMode valueMode) const
+{
+ const StylePropertyShorthand properties[3] = { borderWidthShorthand(), borderStyleShorthand(), borderColorShorthand() };
+ StringBuilder result;
+ for (size_t i = 0; i < WTF_ARRAY_LENGTH(properties); ++i) {
+ String value = getCommonValue(properties[i]);
+ if (value.isNull()) {
+ if (valueMode == ReturnNullOnUncommonValues)
+ return String();
+ ASSERT(valueMode == OmitUncommonValues);
+ continue;
+ }
+ if (value == "initial")
+ continue;
+ if (!result.isEmpty())
+ result.append(" ");
+ result.append(value);
+ }
+ return result.toString();
+}
+
PassRefPtr<CSSValue> StylePropertySet::getPropertyCSSValue(CSSPropertyID propertyID) const
{
const CSSProperty* property = findPropertyWithId(propertyID);
@@ -588,6 +608,8 @@
const CSSProperty& prop = m_properties[n];
CSSPropertyID propertyID = prop.id();
CSSPropertyID shorthandPropertyID = CSSPropertyInvalid;
+ CSSPropertyID borderFallbackShorthandProperty = CSSPropertyInvalid;
+ String value;
switch (propertyID) {
case CSSPropertyBackgroundPositionX:
@@ -602,44 +624,36 @@
case CSSPropertyBackgroundRepeatY:
repeatYProp = ∝
continue;
- case CSSPropertyBorderWidth:
case CSSPropertyBorderTopWidth:
case CSSPropertyBorderRightWidth:
case CSSPropertyBorderBottomWidth:
case CSSPropertyBorderLeftWidth:
- case CSSPropertyBorderStyle:
+ if (!borderFallbackShorthandProperty)
+ borderFallbackShorthandProperty = CSSPropertyBorderWidth;
case CSSPropertyBorderTopStyle:
case CSSPropertyBorderRightStyle:
case CSSPropertyBorderBottomStyle:
case CSSPropertyBorderLeftStyle:
- case CSSPropertyBorderColor:
+ if (!borderFallbackShorthandProperty)
+ borderFallbackShorthandProperty = CSSPropertyBorderStyle;
case CSSPropertyBorderTopColor:
case CSSPropertyBorderRightColor:
case CSSPropertyBorderBottomColor:
case CSSPropertyBorderLeftColor:
+ if (!borderFallbackShorthandProperty)
+ borderFallbackShorthandProperty = CSSPropertyBorderColor;
+
// FIXME: Deal with cases where only some of border-(top|right|bottom|left) are specified.
- shorthandPropertyID = CSSPropertyBorder;
- if (shorthandPropertyAppeared.get(CSSPropertyBorder - firstCSSProperty))
- break;
- for (unsigned i = 0; i < borderAbridgedShorthand().length() && shorthandPropertyID; i++) {
- const StylePropertyShorthand& shorthand = *(borderAbridgedShorthand().propertiesForInitialization()[i]);
- String commonValue;
- bool commonImportance = false;
- for (size_t j = 0; j < shorthand.length(); ++j) {
- CSSPropertyID id = shorthand.properties()[j];
- RefPtr<CSSValue> value = getPropertyCSSValue(id);
- String currentValue = value ? value->cssText() : String();
- bool isImportant = propertyIsImportant(id);
- if (j && (currentValue != commonValue || commonImportance != isImportant)) {
- shorthandPropertyID = CSSPropertyInvalid;
- break;
- }
- if (!j) {
- commonValue = currentValue;
- commonImportance = isImportant;
- }
- }
- }
+ if (!shorthandPropertyAppeared.get(CSSPropertyBorder - firstCSSProperty)) {
+ value = borderPropertyValue(ReturnNullOnUncommonValues);
+ if (value.isNull())
+ shorthandPropertyAppeared.ensureSizeAndSet(CSSPropertyBorder - firstCSSProperty, numCSSProperties);
+ else
+ shorthandPropertyID = CSSPropertyBorder;
+ } else if (shorthandPropertyUsed.get(CSSPropertyBorder - firstCSSProperty))
+ shorthandPropertyID = CSSPropertyBorder;
+ if (!shorthandPropertyID)
+ shorthandPropertyID = borderFallbackShorthandProperty;
break;
case CSSPropertyWebkitBorderHorizontalSpacing:
case CSSPropertyWebkitBorderVerticalSpacing:
@@ -724,12 +738,11 @@
break;
}
- String value;
unsigned shortPropertyIndex = shorthandPropertyID - firstCSSProperty;
if (shorthandPropertyID) {
if (shorthandPropertyUsed.get(shortPropertyIndex))
continue;
- if (!shorthandPropertyAppeared.get(shortPropertyIndex))
+ if (!shorthandPropertyAppeared.get(shortPropertyIndex) && value.isNull())
value = getPropertyValue(shorthandPropertyID);
shorthandPropertyAppeared.ensureSizeAndSet(shortPropertyIndex, numCSSProperties);
}
Modified: trunk/Source/WebCore/css/StylePropertySet.h (115226 => 115227)
--- trunk/Source/WebCore/css/StylePropertySet.h 2012-04-25 17:32:24 UTC (rev 115226)
+++ trunk/Source/WebCore/css/StylePropertySet.h 2012-04-25 17:44:29 UTC (rev 115227)
@@ -123,6 +123,8 @@
String getShorthandValue(const StylePropertyShorthand&) const;
String getCommonValue(const StylePropertyShorthand&) const;
+ enum CommonValueMode { OmitUncommonValues, ReturnNullOnUncommonValues };
+ String borderPropertyValue(CommonValueMode) const;
String getLayeredShorthandValue(const StylePropertyShorthand&) const;
String get4Values(const StylePropertyShorthand&) const;
String borderSpacingValue(const StylePropertyShorthand&) const;
