From: macpherson@chromium.org Date: Thu, 5 Jul 2012 00:15:54 +0000 (+0000) Subject: Inspector crashes when trying to inspect a page with CSS variables X-Git-Tag: 070512121124~24 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d432d9ed3f929e21ed1b75c095e35881f43c035e;p=profile%2Fivi%2Fwebkit-efl.git 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): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121874 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- diff --git a/LayoutTests/inspector/styles/variables/css-variables-expected.txt b/LayoutTests/inspector/styles/variables/css-variables-expected.txt new file mode 100644 index 0000000..b9886e3 --- /dev/null +++ b/LayoutTests/inspector/styles/variables/css-variables-expected.txt @@ -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; + + + diff --git a/LayoutTests/inspector/styles/variables/css-variables.html b/LayoutTests/inspector/styles/variables/css-variables.html new file mode 100644 index 0000000..95be549 --- /dev/null +++ b/LayoutTests/inspector/styles/variables/css-variables.html @@ -0,0 +1,40 @@ + + + + + + + + + + +

+Tests that webkit css variables can be loaded correctly. +

+ +

Text

+ + + diff --git a/LayoutTests/platform/chromium/TestExpectations b/LayoutTests/platform/chromium/TestExpectations index ae36986..456c72f 100644 --- a/LayoutTests/platform/chromium/TestExpectations +++ b/LayoutTests/platform/chromium/TestExpectations @@ -158,6 +158,7 @@ BUGGBILLOCK SKIP : webintents/intent-tag.html = PASS // 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 diff --git a/LayoutTests/platform/gtk/TestExpectations b/LayoutTests/platform/gtk/TestExpectations index 1d6a4a4..29cf8a7 100644 --- a/LayoutTests/platform/gtk/TestExpectations +++ b/LayoutTests/platform/gtk/TestExpectations @@ -318,6 +318,7 @@ BUGWK79203 SKIP : fast/mediastream = TEXT // 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 diff --git a/LayoutTests/platform/mac/TestExpectations b/LayoutTests/platform/mac/TestExpectations index bdeeaac..d296f61 100644 --- a/LayoutTests/platform/mac/TestExpectations +++ b/LayoutTests/platform/mac/TestExpectations @@ -11,6 +11,7 @@ BUGWK76439 DEBUG : fast/dom/shadow/content-element-outside-shadow.html = TEXT // 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 diff --git a/LayoutTests/platform/qt/TestExpectations b/LayoutTests/platform/qt/TestExpectations index c1c8970..20937a0 100644 --- a/LayoutTests/platform/qt/TestExpectations +++ b/LayoutTests/platform/qt/TestExpectations @@ -73,6 +73,7 @@ BUGWK88198 : svg/stroke/non-scaling-stroke-pattern.svg = PASS IMAGE // 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 diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index a9a647e..3dcfeb3 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,29 @@ +2012-07-04 Luke Macpherson + + 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 Web Inspector [JSC]: Implement ScriptCallStack::stackTrace diff --git a/Source/WebCore/css/CSSGrammar.y b/Source/WebCore/css/CSSGrammar.y index 827abed..c7c10b5 100644 --- a/Source/WebCore/css/CSSGrammar.y +++ b/Source/WebCore/css/CSSGrammar.y @@ -1360,7 +1360,7 @@ declaration: CSSParser* p = static_cast(parser); p->storeVariableDeclaration($1, p->sinkFloatingValueList($4), $5); $$ = true; - p->markPropertyEnd($5, $$); + p->markPropertyEnd($5, true); #else $$ = false; #endif diff --git a/Source/WebCore/css/CSSParser.cpp b/Source/WebCore/css/CSSParser.cpp index 5060331..015d930 100644 --- a/Source/WebCore/css/CSSParser.cpp +++ b/Source/WebCore/css/CSSParser.cpp @@ -3012,13 +3012,16 @@ bool CSSParser::cssVariablesEnabled() const void CSSParser::storeVariableDeclaration(const CSSParserString& name, PassOwnPtr 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 @@ restartAfterComment: } 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) diff --git a/Source/WebCore/css/CSSProperty.cpp b/Source/WebCore/css/CSSProperty.cpp index 01519f3..76cc8ea 100644 --- a/Source/WebCore/css/CSSProperty.cpp +++ b/Source/WebCore/css/CSSProperty.cpp @@ -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 @@ struct SameSizeAsCSSProperty { 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(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() diff --git a/Source/WebCore/css/CSSProperty.h b/Source/WebCore/css/CSSProperty.h index b746d80..7124146 100644 --- a/Source/WebCore/css/CSSProperty.h +++ b/Source/WebCore/css/CSSProperty.h @@ -51,6 +51,7 @@ public: CSSValue* value() const { return m_value.get(); } + String cssName() const; String cssText() const; void wrapValueInCommaSeparatedList(); diff --git a/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp b/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp index 2ad4d20..5eb297c 100644 --- a/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp +++ b/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp @@ -139,7 +139,7 @@ String PropertySetCSSStyleDeclaration::item(unsigned i) const { if (i >= m_propertySet->propertyCount()) return ""; - return getPropertyName(m_propertySet->propertyAt(i).id()); + return m_propertySet->propertyAt(i).cssName(); } String PropertySetCSSStyleDeclaration::cssText() const diff --git a/Source/WebCore/css/StylePropertySet.cpp b/Source/WebCore/css/StylePropertySet.cpp index 7fc836f..0c25a5a 100644 --- a/Source/WebCore/css/StylePropertySet.cpp +++ b/Source/WebCore/css/StylePropertySet.cpp @@ -662,6 +662,11 @@ String StylePropertySet::asText() const String value; switch (propertyID) { +#if ENABLE(CSS_VARIABLES) + case CSSPropertyVariable: + result.append(prop.cssText()); + continue; +#endif case CSSPropertyBackgroundPositionX: positionXProp = ∝ continue; diff --git a/Source/WebCore/inspector/InspectorStyleSheet.cpp b/Source/WebCore/inspector/InspectorStyleSheet.cpp index 3cd9150..5f5bb05 100644 --- a/Source/WebCore/inspector/InspectorStyleSheet.cpp +++ b/Source/WebCore/inspector/InspectorStyleSheet.cpp @@ -1116,7 +1116,7 @@ bool InspectorStyleSheet::ensureSourceData() return false; RefPtr newStyleSheet = StyleSheetContents::create(); - CSSParser p(CSSStrictMode); + CSSParser p(m_pageStyleSheet->ownerDocument()); OwnPtr ruleSourceDataResult = adoptPtr(new RuleSourceDataList()); p.parseSheet(newStyleSheet.get(), m_parsedStyleSheet->text(), 0, ruleSourceDataResult.get()); m_parsedStyleSheet->setSourceData(ruleSourceDataResult.release()); diff --git a/Tools/Scripts/webkitpy/layout_tests/port/webkit.py b/Tools/Scripts/webkitpy/layout_tests/port/webkit.py index d954e76..fad6f7a 100755 --- a/Tools/Scripts/webkitpy/layout_tests/port/webkit.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/webkit.py @@ -298,7 +298,7 @@ class WebKitPort(Port): "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): diff --git a/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py index aab6543..6b45d42 100755 --- a/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py @@ -100,6 +100,7 @@ class WebKitPortTest(port_testcase.PortTestCase): "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']))