Title: [121874] trunk
Revision
121874
Author
macpher...@chromium.org
Date
2012-07-04 17:15:54 -0700 (Wed, 04 Jul 2012)

Log Message

Inspector crashes when trying to inspect a page with CSS variables
https://bugs.webkit.org/show_bug.cgi?id=89818

Reviewed by Antti Koivisto.

Patch works by fixing treating handling of CSSPropertyID == CSSPropertyVariable as a special case,
and looking up the author-defined property name from the CSSValue.

Added test inspector/styles/variables/css-variables.html that inspects an element using CSS variables.
Test is skipped when variables are compiled out.

* css/CSSProperty.cpp:
(WebCore::CSSProperty::cssName):
(WebCore):
(WebCore::CSSProperty::cssText):
* css/CSSProperty.h:
(CSSProperty):
* css/PropertySetCSSStyleDeclaration.cpp:
(WebCore::PropertySetCSSStyleDeclaration::item):
* css/StylePropertySet.cpp:
(WebCore::StylePropertySet::asText):
* inspector/InspectorStyleSheet.cpp:
(WebCore::InspectorStyle::populateAllProperties):

Modified Paths

Added Paths

Diff

Added: trunk/LayoutTests/inspector/styles/variables/css-variables-expected.txt (0 => 121874)


--- trunk/LayoutTests/inspector/styles/variables/css-variables-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector/styles/variables/css-variables-expected.txt	2012-07-05 00:15:54 UTC (rev 121874)
@@ -0,0 +1,24 @@
+Tests that webkit css variables can be loaded correctly.
+
+Text
+[expanded] 
+color: green;
+    #inspected - -webkit-var(a) css-variables.html:7
+display: block;
+    div - block user agent stylesheet
+
+[expanded] 
+element.style  { ()
+
+======== Matched CSS Rules ========
+[expanded] 
+#inspected  { (css-variables.html:7)
+-webkit-var-a: green;
+color: -webkit-var(a);
+
+[expanded] 
+div  { (user agent stylesheet)
+display: block;
+
+
+

Added: trunk/LayoutTests/inspector/styles/variables/css-variables.html (0 => 121874)


--- trunk/LayoutTests/inspector/styles/variables/css-variables.html	                        (rev 0)
+++ trunk/LayoutTests/inspector/styles/variables/css-variables.html	2012-07-05 00:15:54 UTC (rev 121874)
@@ -0,0 +1,40 @@
+<html>
+<head>
+<script>
+internals.settings.setCSSVariablesEnabled(true);
+</script>
+<style>
+#inspected {
+    -webkit-var-a: green;
+    color: -webkit-var(a);
+}
+
+</style>
+<script src=""
+<script src=""
+<script>
+
+function test()
+{
+    WebInspector.showPanel("elements");
+    InspectorTest.selectNodeAndWaitForStylesWithComputed("inspected", dumpAllStyles);
+
+    function dumpAllStyles()
+    {
+        InspectorTest.dumpSelectedElementStyles();
+        InspectorTest.completeTest();
+    }
+}
+
+</script>
+</head>
+
+<body _onload_="runTest()">
+<p>
+Tests that webkit css variables can be loaded correctly.
+</p>
+
+<div id="inspected">Text</div>
+
+</body>
+</html>

Modified: trunk/LayoutTests/platform/chromium/TestExpectations (121873 => 121874)


--- trunk/LayoutTests/platform/chromium/TestExpectations	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/LayoutTests/platform/chromium/TestExpectations	2012-07-05 00:15:54 UTC (rev 121874)
@@ -158,6 +158,7 @@
 
 // CSS Variables are not yet enabled.
 BUGWK85580 SKIP : fast/css/variables = PASS
+BUGWK85580 SKIP : inspector/styles/variables = PASS
 
 // CSS image-resolution is not yet enabled.
 BUGWK85262 SKIP : fast/css/image-resolution = PASS

Modified: trunk/LayoutTests/platform/gtk/TestExpectations (121873 => 121874)


--- trunk/LayoutTests/platform/gtk/TestExpectations	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/LayoutTests/platform/gtk/TestExpectations	2012-07-05 00:15:54 UTC (rev 121874)
@@ -318,6 +318,7 @@
 
 // CSS Variables are not yet enabled.
 BUGWK85580 SKIP : fast/css/variables = PASS TEXT
+BUGWK85580 SKIP : inspector/styles/variables = PASS
 
 // CSS image-resolution is not yet enabled.
 BUGWK85262 SKIP : fast/css/image-resolution = PASS TEXT

Modified: trunk/LayoutTests/platform/mac/TestExpectations (121873 => 121874)


--- trunk/LayoutTests/platform/mac/TestExpectations	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2012-07-05 00:15:54 UTC (rev 121874)
@@ -11,6 +11,7 @@
 
 // CSS Variables are not yet enabled.
 BUGWK85580 SKIP : fast/css/variables = PASS
+BUGWK85580 SKIP : inspector/styles/variables = PASS
 
 // UndoManager is not yet enabled.
 BUGWK87908 SKIP : editing/undomanager = PASS

Modified: trunk/LayoutTests/platform/qt/TestExpectations (121873 => 121874)


--- trunk/LayoutTests/platform/qt/TestExpectations	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/LayoutTests/platform/qt/TestExpectations	2012-07-05 00:15:54 UTC (rev 121874)
@@ -73,6 +73,7 @@
 
 // CSS Variables are not yet enabled.
 BUGWK85580 SKIP : fast/css/variables = PASS
+BUGWK85580 SKIP : inspector/styles/variables = PASS
 
 // UndoManager is not yet enabled.
 BUGWK87908 SKIP : editing/undomanager = PASS

Modified: trunk/Source/WebCore/ChangeLog (121873 => 121874)


--- trunk/Source/WebCore/ChangeLog	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Source/WebCore/ChangeLog	2012-07-05 00:15:54 UTC (rev 121874)
@@ -1,3 +1,29 @@
+2012-07-04  Luke Macpherson  <macpher...@chromium.org>
+
+        Inspector crashes when trying to inspect a page with CSS variables
+        https://bugs.webkit.org/show_bug.cgi?id=89818
+
+        Reviewed by Antti Koivisto.
+
+        Patch works by fixing treating handling of CSSPropertyID == CSSPropertyVariable as a special case,
+        and looking up the author-defined property name from the CSSValue.
+
+        Added test inspector/styles/variables/css-variables.html that inspects an element using CSS variables.
+        Test is skipped when variables are compiled out.
+
+        * css/CSSProperty.cpp:
+        (WebCore::CSSProperty::cssName):
+        (WebCore):
+        (WebCore::CSSProperty::cssText):
+        * css/CSSProperty.h:
+        (CSSProperty):
+        * css/PropertySetCSSStyleDeclaration.cpp:
+        (WebCore::PropertySetCSSStyleDeclaration::item):
+        * css/StylePropertySet.cpp:
+        (WebCore::StylePropertySet::asText):
+        * inspector/InspectorStyleSheet.cpp:
+        (WebCore::InspectorStyle::populateAllProperties):
+
 2012-07-04  Anthony Scian  <asc...@rim.com>
 
         Web Inspector [JSC]: Implement ScriptCallStack::stackTrace

Modified: trunk/Source/WebCore/css/CSSGrammar.y (121873 => 121874)


--- trunk/Source/WebCore/css/CSSGrammar.y	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Source/WebCore/css/CSSGrammar.y	2012-07-05 00:15:54 UTC (rev 121874)
@@ -1360,7 +1360,7 @@
         CSSParser* p = static_cast<CSSParser*>(parser);
         p->storeVariableDeclaration($1, p->sinkFloatingValueList($4), $5);
         $$ = true;
-        p->markPropertyEnd($5, $$);
+        p->markPropertyEnd($5, true);
 #else
         $$ = false;
 #endif

Modified: trunk/Source/WebCore/css/CSSParser.cpp (121873 => 121874)


--- trunk/Source/WebCore/css/CSSParser.cpp	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Source/WebCore/css/CSSParser.cpp	2012-07-05 00:15:54 UTC (rev 121874)
@@ -3012,13 +3012,16 @@
 
 void CSSParser::storeVariableDeclaration(const CSSParserString& name, PassOwnPtr<CSSParserValueList> value, bool important)
 {
+    ASSERT(name.length > 12);
+    AtomicString variableName = String(name.characters + 12, name.length - 12);
+
     StringBuilder builder;
     for (unsigned i = 0, size = value->size(); i < size; i++) {
         if (i)
             builder.append(' ');
         builder.append(value->valueAt(i)->createCSSValue()->cssText());
     }
-    addProperty(CSSPropertyVariable, CSSVariableValue::create(name, builder.toString()), important, false);
+    addProperty(CSSPropertyVariable, CSSVariableValue::create(variableName, builder.toString()), important, false);
 }
 #endif
 
@@ -8981,21 +8984,16 @@
     }
 
     case CharacterDash:
-#if ENABLE(CSS_VARIABLES)
-        if (cssVariablesEnabled() && isEqualToCSSIdentifier(m_currentCharacter, "webkit-var") && m_currentCharacter[10] == '-' && isIdentifierStartAfterDash(m_currentCharacter + 11)) {
-            // handle variable declarations
-            m_currentCharacter += 11;
-            parseIdentifier(result, hasEscape);
-            m_token = VAR_DEFINITION;
-            yylval->string.characters = m_tokenStart;
-            yylval->string.length = result - m_tokenStart;
-        } else
-#endif
         if (isIdentifierStartAfterDash(m_currentCharacter)) {
             --m_currentCharacter;
             parseIdentifier(result, hasEscape);
             m_token = IDENT;
 
+#if ENABLE(CSS_VARIABLES)
+            if (cssVariablesEnabled() && isEqualToCSSIdentifier(m_tokenStart + 1, "webkit-var") && m_tokenStart[11] == '-' && isIdentifierStartAfterDash(m_tokenStart + 12))
+                m_token = VAR_DEFINITION;
+            else
+#endif
             if (*m_currentCharacter == '(') {
                 m_token = FUNCTION;
                 if (!hasEscape)

Modified: trunk/Source/WebCore/css/CSSProperty.cpp (121873 => 121874)


--- trunk/Source/WebCore/css/CSSProperty.cpp	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Source/WebCore/css/CSSProperty.cpp	2012-07-05 00:15:54 UTC (rev 121874)
@@ -26,6 +26,10 @@
 #include "RenderStyleConstants.h"
 #include "StylePropertyShorthand.h"
 
+#if ENABLE(CSS_VARIABLES)
+#include "CSSVariableValue.h"
+#endif
+
 namespace WebCore {
 
 struct SameSizeAsCSSProperty {
@@ -35,9 +39,20 @@
 
 COMPILE_ASSERT(sizeof(CSSProperty) == sizeof(SameSizeAsCSSProperty), CSSProperty_should_stay_small);
 
+String CSSProperty::cssName() const
+{
+#if ENABLE(CSS_VARIABLES)
+    if (id() == CSSPropertyVariable) {
+        ASSERT(value()->isVariableValue());
+        return "-webkit-var-" + static_cast<CSSVariableValue*>(value())->name();
+    }
+#endif
+    return String(getPropertyName(id()));
+}
+
 String CSSProperty::cssText() const
 {
-    return String(getPropertyName(id())) + ": " + m_value->cssText() + (isImportant() ? " !important" : "") + "; ";
+    return cssName() + ": " + m_value->cssText() + (isImportant() ? " !important" : "") + "; ";
 }
 
 void CSSProperty::wrapValueInCommaSeparatedList()

Modified: trunk/Source/WebCore/css/CSSProperty.h (121873 => 121874)


--- trunk/Source/WebCore/css/CSSProperty.h	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Source/WebCore/css/CSSProperty.h	2012-07-05 00:15:54 UTC (rev 121874)
@@ -51,6 +51,7 @@
 
     CSSValue* value() const { return m_value.get(); }
 
+    String cssName() const;
     String cssText() const;
 
     void wrapValueInCommaSeparatedList();

Modified: trunk/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp (121873 => 121874)


--- trunk/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp	2012-07-05 00:15:54 UTC (rev 121874)
@@ -139,7 +139,7 @@
 {
     if (i >= m_propertySet->propertyCount())
         return "";
-    return getPropertyName(m_propertySet->propertyAt(i).id());
+    return m_propertySet->propertyAt(i).cssName();
 }
 
 String PropertySetCSSStyleDeclaration::cssText() const

Modified: trunk/Source/WebCore/css/StylePropertySet.cpp (121873 => 121874)


--- trunk/Source/WebCore/css/StylePropertySet.cpp	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Source/WebCore/css/StylePropertySet.cpp	2012-07-05 00:15:54 UTC (rev 121874)
@@ -662,6 +662,11 @@
         String value;
 
         switch (propertyID) {
+#if ENABLE(CSS_VARIABLES)
+        case CSSPropertyVariable:
+            result.append(prop.cssText());
+            continue;
+#endif
         case CSSPropertyBackgroundPositionX:
             positionXProp = &prop;
             continue;

Modified: trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp (121873 => 121874)


--- trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp	2012-07-05 00:15:54 UTC (rev 121874)
@@ -1116,7 +1116,7 @@
         return false;
 
     RefPtr<StyleSheetContents> newStyleSheet = StyleSheetContents::create();
-    CSSParser p(CSSStrictMode);
+    CSSParser p(m_pageStyleSheet->ownerDocument());
     OwnPtr<RuleSourceDataList> ruleSourceDataResult = adoptPtr(new RuleSourceDataList());
     p.parseSheet(newStyleSheet.get(), m_parsedStyleSheet->text(), 0, ruleSourceDataResult.get());
     m_parsedStyleSheet->setSourceData(ruleSourceDataResult.release());

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (121873 => 121874)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py	2012-07-05 00:15:54 UTC (rev 121874)
@@ -298,7 +298,7 @@
             "WebCoreHas3DRendering": ["animations/3d", "transforms/3d"],
             "WebGLShader": ["fast/canvas/webgl", "compositing/webgl", "http/tests/canvas/webgl"],
             "MHTMLArchive": ["mhtml"],
-            "CSSVariableValue": ["fast/css/variables"],
+            "CSSVariableValue": ["fast/css/variables", "inspector/styles/variables"],
         }
 
     def _has_test_in_directories(self, directory_lists, test_list):

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py (121873 => 121874)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py	2012-07-04 22:41:17 UTC (rev 121873)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py	2012-07-05 00:15:54 UTC (rev 121874)
@@ -100,6 +100,7 @@
             "http/tests/canvas/webgl",  # Requires WebGLShader
             "mhtml",  # Requires MHTMLArchive
             "fast/css/variables",  # Requires CSS Variables
+            "inspector/styles/variables",  # Requires CSS Variables
         ])
 
         result_directories = set(TestWebKitPort(symbols_string, None)._skipped_tests_for_unsupported_features(test_list=['mathml/foo.html']))
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to