RESOLVED INVALID 112415
CSS Variables are not properly set via Web Inspector 
https://bugs.webkit.org/show_bug.cgi?id=112415
Summary CSS Variables are not properly set via Web Inspector
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
DetailsFormatted DiffDiff
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
DetailsFormatted DiffDiff
Patch (8.15 KB, patch)
2013-03-27 03:40 PDT, Alan Cutter 		no flags
DetailsFormatted DiffDiff
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.