Unless I completely misunderstood the CSS2 specs, Mozilla supports a DOM2 CSS2Properties interface for style information in a CSSStyleDeclaration (i.e., you can write o.style.left instead of o.style.getPropertyValue('left')). The same should happen for getComputedStyle(), which returns a CSSStyleDeclaration, too, but it is not true currently (o.getComputedStyle().left gives a JS error).

Correct. At the moment you have to use .getPropertyValue() on the style declaration returned by getComputedStyle. The reason is that most of the CSS2 properties are in fact not implemented... So most accesses would just throw exceptions. Taking this.

⁃ Added a parameter to the CSS_PROP macro in nsCSSPropList.h for the CSS2Properties method name ⁃ Added CSS_PROP_INTERNAL macro defaulting to mirror CSS_PROP unless specified by the caller. Designed for internal properties we don't expose via nsIDOMCSS2Properties, e.g. '-x-clip-left'. NOTE: SVG properties are in this grouping for now until it gets a better home. nsIDOMSVGCSS2Properties? ⁃ Added CSS_PROP_NOTIMPLEMENTED macro for properties that are part of the CSS spec which we just don't handle yet, e.g. 'outline'. This mainly exists to quiet down compiler warnings for things we want to expose publicly, yet don't handle internally. ⁃ Renamed eCSSProperty_outline* to eCSSProperty__moz_outline* to avoid name conflicts. Ditto for eCSSProperty_counter*. ⁃ Added missing properties to nsIDOMNSCSS2Properties.

So my last comment got mangled, apparently by bug 163921. Anyway, taking this.

David, care to review this? The patch is fairly large in size, but most of it is removal of #if 0 placeholder code in nsComputedDOMStyle.cpp which assumed that each of the CSS2Properties properties would get its own implementation. The macro-fu I am using obsoletes that. The other large chunk of the patch is the nsCSSPropList.h change which is pretty straightforward.

Comment on attachment 99361 [details] [diff] [review] Patch v1.0 r=dbaron. A fun followup patch would be to use COM aggregation to reduce code size by sharing all the CSS2Properties stubs between the two implementations. (Any idea how much code size this adds?) It would probably be good to get this in sooner rather than later.

> (Any idea how much code size this adds?) Not an exact figure, but ~58k. I have 2 trees but with somehwat different sources: the tree with my 'background-clip' and 'background-origin' changes has a libgkcontent size of 6109757, and the tree with this change has a size of 6167851 I'll work on getting that back down.

But get this patch in first, so we don't need to worry about merging. :-)

The patch looks good. I have a couple of questions: -- Is it really wise to change the symbols to __moz_blah_blah instead of _blah_blah? Some of the -moz properties will lose their prefix if CSS3 is finalized and our properties behave correctly. It would be nice to be able to do that without changing much code. I don't think it buys us much to have the __moz stuff in our C++ code where only we can see it; that prefix is really just a notice to Web authors. -- Would it be possible to use libxptcall to avoid having to explicitly implement all these properties? You could have a QueryInterface for the CSS properties interface just return an internal object derived from nsXPTCStubBase where you override CallMethod to do a table lookup. I dunno, something like that might work :-).

> Is it really wise to change the symbols to __moz_blah_blah instead of _blah_blah? Some of the -moz properties will lose their prefix if CSS3 is finalized and our properties behave correctly. I only changed the outline and counter symbols because they are CSS2 properties which we implement as -moz properties because our behavior is wrong. > Would it be possible to use libxptcall to avoid having to explicitly implement all these properties? Possible. I don't really know much about libxptcall (yet). Looking into it.

> I only changed the outline and counter symbols because they are CSS2 properties > which we implement as -moz properties because our behavior is wrong. Yeah, well, we'd like to change them back sometime :-).

> Yeah, well, we'd like to change them back sometime :-) Sure. That will be trivial to do when the time comes. If you volunteer to fix the property behaviors, I'll volunteer to swap the symbols back. :-) I debated on just duplicating the symbols that existed in the CSS_PROP macro onto the CSS_PROP_NOTIMPLEMENTED macro, since my patch makes us ignore the not implemented stuff except for the CSS2Properties stuff. However, that seems more to me wrong than switching symbols since it's always remotely possible we'll change our usages and then have clashes.

"seems more to me wrong" in my previous comment was supposed to be "seems more wrong to me"

Comment on attachment 99361 [details] [diff] [review] Patch v1.0 sr=jst

Comment on attachment 99361 [details] [diff] [review] Patch v1.0 sr=jst

Checked in. I spun off bug 170895 to take care of the code size reduction.

What was the result of your analysis of libxptcall?