Bug 112415

Summary:

CSS Variables are not properly set via Web Inspector

Product:

WebKit

Reporter:

Alan Cutter <alancutter>

Component:

Web Inspector (Deprecated)

Assignee:

Alan Cutter <alancutter>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

apavlov, buildbot, esprehn+autocc, keishi, kling, loislo, macpherson, menard, ojan.autocc, pfeldman, pmuellr, rniwa, vivekg, vsevik, web-inspector-bugs, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

85580

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-01 for mac-future	none
Patch	none
Patch	none

Alan Cutter

Reported 2013-03-15 00:28:50 PDT

When setting CSS variables in the Web Inspector only the last variable takes effect. This is most likely due to CSS variables all sharing the same CSSPropertyID. Example: #test { padding-left: -webkit-var(a); padding-top: -webkit-var(b); -webkit-var-a: 100px; -webkit-var-b: 200px; } If these values are entered via the Web Inspector only padding-top will have the correct value. padding-left will remain unset.

Attachments
Patch (9.70 KB, patch) 2013-03-25 21:58 PDT, Alan Cutter	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (491.43 KB, application/zip) 2013-03-26 01:58 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-01 for mac-future (946.84 KB, application/zip) 2013-03-26 11:55 PDT, Build Bot	no flags	Details
Patch (8.39 KB, patch) 2013-03-26 18:28 PDT, Alan Cutter	no flags	Details Formatted Diff Diff
Patch (8.15 KB, patch) 2013-03-27 03:40 PDT, Alan Cutter	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Alan Cutter

Comment 1 2013-03-25 21:58:07 PDT

Created attachment 194999 [details] Patch

Build Bot

Comment 2 2013-03-26 01:58:55 PDT

Comment on attachment 194999 [details] Patch Attachment 194999 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17295382 New failing tests: inspector/styles/add-multiple-inline-css-variables.html

Build Bot

Comment 3 2013-03-26 01:58:57 PDT

Created attachment 195033 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.2

Build Bot

Comment 4 2013-03-26 11:55:07 PDT

Comment on attachment 194999 [details] Patch Attachment 194999 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17289514 New failing tests: inspector/styles/add-multiple-inline-css-variables.html

Build Bot

Comment 5 2013-03-26 11:55:10 PDT

Created attachment 195130 [details] Archive of layout-test-results from webkit-ews-01 for mac-future The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-future Platform: Mac OS X 10.8.2

Alan Cutter

Comment 6 2013-03-26 18:28:28 PDT

Created attachment 195205 [details] Patch

Alan Cutter

Comment 7 2013-03-26 18:29:45 PDT

(In reply to comment #6) > Created an attachment (id=195205) [details] > Patch Removed test from Mac port (CSS variables not enabled there).

Alexander Pavlov (apavlov)

Comment 8 2013-03-27 03:00:43 PDT

Comment on attachment 195205 [details] Patch The Web Inspector test looks good. View in context: https://bugs.webkit.org/attachment.cgi?id=195205&action=review > Source/WebCore/css/StylePropertySet.cpp:759 > + ASSERT(value->isVariableValue()); toHTMLElement() and friends check for !value as well. Do you want to do the same here? > Source/WebCore/css/StylePropertySet.cpp:765 > + ASSERT(value->isVariableValue()); Ditto > Source/WebCore/css/StylePropertySet.cpp:771 > + for (int i = propertyCount() - 1; i >= 0; i--) { Most for-loops in this file seem to use the prefix-decrement. > LayoutTests/inspector/styles/variables/add-inline-css-variable.html:14 > + var idToDOMNode = WebInspector.domAgent._idToDOMNode; You can use InspectorTest.expandedNodeWithId(idValue) for brevity (since you have selected it in the tree) from elements-test.js (and then use node.id)

Alan Cutter

Comment 9 2013-03-27 03:40:40 PDT

Created attachment 195261 [details] Patch

Alan Cutter

Comment 10 2013-03-27 03:43:46 PDT

(In reply to comment #8) > (From update of attachment 195205 [details]) > The Web Inspector test looks good. > > View in context: https://bugs.webkit.org/attachment.cgi?id=195205&action=review > Thanks for the review. > > Source/WebCore/css/StylePropertySet.cpp:759 > > + ASSERT(value->isVariableValue()); > > toHTMLElement() and friends check for !value as well. Do you want to do the same here? > > > Source/WebCore/css/StylePropertySet.cpp:765 > > + ASSERT(value->isVariableValue()); > > Ditto > Added check for !value. > > Source/WebCore/css/StylePropertySet.cpp:771 > > + for (int i = propertyCount() - 1; i >= 0; i--) { > > Most for-loops in this file seem to use the prefix-decrement. > Changed to prefix. > > LayoutTests/inspector/styles/variables/add-inline-css-variable.html:14 > > + var idToDOMNode = WebInspector.domAgent._idToDOMNode; > > You can use InspectorTest.expandedNodeWithId(idValue) for brevity (since you have selected it in the tree) from elements-test.js (and then use node.id) I'm glad there was an easier way, I'm not so familiar with the Web Inspector to know much better, just copied what another test was doing. Changed to use expandedNodeWithId.

Andreas Kling

Comment 11 2014-02-07 03:41:09 PST

CSS variables got yanked.

Note You need to log in before you can comment on or make changes to this bug.