The properties of the CSSStyleDeclaration are enumerable by the for in loop but the property descriptor reports them as non enumerable. var style = document.getComputedStyle(document.body); var descr = Object.getOwnPropertyDescriptor(style, 'color'); shouldBeTrue('descr.enumerable');
(In reply to comment #0) > The properties of the CSSStyleDeclaration are enumerable by the for in loop but the property descriptor reports them as non enumerable. > > var style = document.getComputedStyle(document.body); > var descr = Object.getOwnPropertyDescriptor(style, 'color'); > shouldBeTrue('descr.enumerable'); There's also more weird stuff regarding CSS2Properties in CSSStyleDeclaration. For example if you get the descriptor of "color" property with Object.getOwnPropertyDescriptor() it returns: Object {value: "", writable: false, enumerable: false, configurable: false} So it appears as "writable: false" however things like the next line works properly: style.color = "blue"; According to the spec (http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf) if writable is false you shouldn't be allowed to modify the value. So it's clear that the descriptor is wrong. BTW, the descriptor in Chrome is: Object {value: "", writable: true, enumerable: true, configurable: true}
Created attachment 204064 [details] Patch
Comment on attachment 204064 [details] Patch This patch changes the property descriptor. However there's still an issue, as even when the descriptor now say "configurable: true" (like in Chrome), if you use Object.defineProperty() to change the descriptor the change is not working. For example with the following code you don't get any message in the JavaScript console: Object.defineProperty(style, "color", { get : function(){ console.log("get color"); }, set : function(x){ console.log("set color: " + x); }, }); Anyway I think that we could report this as a different bug as it seems clear that current descriptor is not accurate with the behavior of CSS2Properties in CSSStyleDeclaration object.
Comment on attachment 204064 [details] Patch Removing ReadOnly and DontEnum here is good. But let's keep DontDelete (configurable: true) for now, Since the property isn't delete-able, that's the most honest thing to report.
Created attachment 208111 [details] Patch
Comment on attachment 208111 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208111&action=review The patch looks good. Some issue with the test. This will need a rebaseline. > LayoutTests/ChangeLog:12 > + * fast/js/cssstyledeclaration-properties-descriptor-expected.txt: Added. > + * fast/js/cssstyledeclaration-properties-descriptor.html: Added. > + * fast/js/script-tests/cssstyledeclaration-properties-descriptor.js: Added. The test is at the wrong place, it should be in LayoutTests//fast/dom/CSSStyleDeclaration > LayoutTests/fast/js/cssstyledeclaration-properties-descriptor.html:1 > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> Please use the HTML5 doctype. > LayoutTests/fast/js/cssstyledeclaration-properties-descriptor.html:4 > +<script src="resources/js-test-pre.js"></script> This won't work anymore, the path has changed. :( > LayoutTests/fast/js/cssstyledeclaration-properties-descriptor.html:7 > +<script src="script-tests/cssstyledeclaration-properties-descriptor.js"></script> Please put the test code here instead of using a separate file. The separate file thingy is the old style of WebKit. It makes it more painful to work on tests. > LayoutTests/fast/js/script-tests/cssstyledeclaration-properties-descriptor.js:3 > +description( > +"This tests the descriptor of CSSStyleDeclaration properties." > +); This should be on a single line.
Created attachment 229275 [details] Patch Thanks for the review. Patch for landing applying review suggestions.
Comment on attachment 229275 [details] Patch Clearing flags on attachment: 229275 Committed r167240: <http://trac.webkit.org/changeset/167240>
All reviewed patches have been landed. Closing bug.