Changeset 112321 in webkit

Timestamp:

Mar 27, 2012 3:01:03 PM (12 years ago)

Author:

rniwa@webkit.org

Message:

cssText should not generate literal 'initial' in shorthand properties
​https://bugs.webkit.org/show_bug.cgi?id=82364

Reviewed by Antti Koivisto.

Source/WebCore:

Fixed the bug by treating initial value as if the value is not set.
While this is incorrect for properties that inherits by default,
it's strictly better than generating unparsable value as we do today.

The proper fix is for CSSInitialValue::cssText to fetch the respective
default value from what's currently in CSSStyleSelector code but that requires
a considerable amount of refactoring and work.

• css/StylePropertySet.cpp:

(WebCore::StylePropertySet::getShorthandValue):
(WebCore::StylePropertySet::getCommonValue):

LayoutTests:

• fast/css/cssText-shorthand-expected.txt: Rebaselined.
• fast/css/cssText-shorthand.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/cssText-shorthand-expected.txt (modified) (1 diff)
• LayoutTests/fast/css/cssText-shorthand.html (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/StylePropertySet.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-03-27  Ryosuke Niwa  <rniwa@webkit.org>
+
+        cssText should not generate literal 'initial' in shorthand properties
+        https://bugs.webkit.org/show_bug.cgi?id=82364
+
+        Reviewed by Antti Koivisto.
+
+        * fast/css/cssText-shorthand-expected.txt: Rebaselined.
+        * fast/css/cssText-shorthand.html:
+
 2012-03-27  Dirk Pranke  <dpranke@chromium.org>
 

trunk/LayoutTests/fast/css/cssText-shorthand-expected.txt

@@ -4 +4 @@
 
 
-PASS normalizeCssText(element.style.cssText) is "border: 1px initial initial"
+PASS normalizeCssText(element.style.cssText) is "border: 1px"
 PASS normalizeCssText(element.style.cssText) is "border: 1px solid red"
-PASS normalizeCssText(element.style.cssText) is "border: 1px initial red"
-PASS normalizeCssText(element.style.cssText) is "border: initial initial red"
-PASS normalizeCssText(element.style.cssText) is "border: 1px initial initial"
+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-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: initial dotted initial"
+PASS normalizeCssText(element.style.cssText) is "border: dotted"
 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"

trunk/LayoutTests/fast/css/cssText-shorthand.html

@@ -9 +9 @@
 var tests = [
     // FIXME: This exhibits a bug. We shouldn't be outputing border-image here.
-    ['border: 1px; border-top: 1px;', 'border: 1px initial initial;'],
+    ['border: 1px; border-top: 1px;', 'border: 1px;'],
     ['border: 1px solid red;', 'border: 1px solid red;'],
-    ['border: 1px red;', 'border: 1px initial red;'],
-    ['border: red;', 'border: initial initial red;'],
-    ['border-top: 1px; border-right: 1px; border-bottom: 1px; border-left: 1px;', 'border: 1px initial initial;'],
+    ['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;'],
@@ -22 +22 @@
     ['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: solid; border-style: dotted', 'border: initial dotted initial;'],
+    ['border: solid; border-style: dotted', 'border: dotted;'],
 
     ['-webkit-border-horizontal-spacing: 1px; -webkit-border-vertical-spacing: 2px;', 'border-spacing: 1px 2px;'],

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-03-27  Ryosuke Niwa  <rniwa@webkit.org>
+
+        cssText should not generate literal 'initial' in shorthand properties
+        https://bugs.webkit.org/show_bug.cgi?id=82364
+
+        Reviewed by Antti Koivisto.
+
+        Fixed the bug by treating initial value as if the value is not set.
+        While this is incorrect for properties that inherits by default,
+        it's strictly better than generating unparsable value as we do today.
+
+        The proper fix is for CSSInitialValue::cssText to fetch the respective
+        default value from what's currently in CSSStyleSelector code but that requires
+        a considerable amount of refactoring and work.
+
+        * css/StylePropertySet.cpp:
+        (WebCore::StylePropertySet::getShorthandValue):
+        (WebCore::StylePropertySet::getCommonValue):
+
 2012-03-26  Adam Klein  <adamk@chromium.org>
 

trunk/Source/WebCore/css/StylePropertySet.cpp

@@ -383 +383 @@
         if (!isPropertyImplicit(longhand.properties()[i])) {
             RefPtr<CSSValue> value = getPropertyCSSValue(longhand.properties()[i]);
-            // FIXME: provide default value if !value
-            if (value) {
+            // FIXME: provide default value if !value or value is initial value
+            if (value && !value->isInitialValue()) {
                 if (!res.isNull())
                     res += " ";
@@ -400 +400 @@
     for (unsigned i = 0; i < longhand.length(); ++i) {
         RefPtr<CSSValue> value = getPropertyCSSValue(longhand.properties()[i]);
-        if (!value)
+        // FIXME: CSSInitialValue::cssText should generate the right value.
+        if (!value || value->isInitialValue())
             return String();
         String text = value->cssText();