not really needed, but it'll mean we can avoid xpcom in some places in later patches

this still works with the external interface definition of RGBColor, but will now also when work when its native type is nsDOMRGBColor.

Comment on attachment 692179 [details] [diff] [review] part 1 add RGBColor webidl r=me

Comment on attachment 692180 [details] [diff] [review] part 2 add webidl api to RGBColor and store its members as nsROCSSPrimitiveValue* not nsIDOMCSSPrimitiveValue* r=me

Comment on attachment 692180 [details] [diff] [review] part 2 add webidl api to RGBColor and store its members as nsROCSSPrimitiveValue* not nsIDOMCSSPrimitiveValue* Review of attachment 692180 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/style/nsDOMCSSRGBColor.h @@ +30,5 @@ > > bool HasAlpha() const { return mHasAlpha; } > > + // RGBColor webidl interface > + nsROCSSPrimitiveValue *Red() const * to the left, please

Comment on attachment 692180 [details] [diff] [review] part 2 add webidl api to RGBColor and store its members as nsROCSSPrimitiveValue* not nsIDOMCSSPrimitiveValue* Silly bugzilla... Resetting r+

(In reply to :Ms2ger from comment #7) > Comment on attachment 692180 [details] [diff] [review] > part 2 add webidl api to RGBColor and store its members as > nsROCSSPrimitiveValue* not nsIDOMCSSPrimitiveValue* > > Review of attachment 692180 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/style/nsDOMCSSRGBColor.h > @@ +30,5 @@ > > > > bool HasAlpha() const { return mHasAlpha; } > > > > + // RGBColor webidl interface > > + nsROCSSPrimitiveValue *Red() const > > * to the left, please ok fine ;\ (I'll "fix" all these patches at once)

nsDOMCSSRGBColor can loose the nsISupports too, but I just asume do that after bug 819215

Comment on attachment 692382 [details] [diff] [review] part 3 v2 add downcasting from CSSValue to nsROCSSPrimitiveValue I would rather the implementation did the test of CssValueType() and returned null if not, and we documented that, instead of relying on all callers checking + assert. r=me if you do that.

Comment on attachment 692383 [details] [diff] [review] part 4 v2 GetRGBColorValue() should return nsDOMCSSRGBColor* >+++ b/dom/bindings/Bindings.conf >+ "nativeType": "nsROCSSPrimitiveValue", >+ "resultNotAddRefed": ["GetRGBColorValue"] Please fix the indent. And it should be "getRGBColorValue". r=me with that.

Comment on attachment 692386 [details] [diff] [review] part 5 cycle collect and wrapper cache r=me

Comment on attachment 692387 [details] [diff] [review] part 5 using webidl for RGBColor >+++ b/dom/bindings/Bindings.conf >+ 'resultNotAddRefed': [ "Alpha", "Blue", "Green", "Red" ] These should use the IDL (lowercase) names, not the C++ ones. >+++ b/layout/style/nsDOMCSSRGBColor.cpp >+ JSObject* >+ nsDOMCSSRGBColor::WrapObject(JSContext *aCx, JSObject *aScope, bool *aTried) Please fix the indent. r=me with that.

Comment on attachment 692389 [details] [diff] [review] part 6 remove dom classinfo stuff r=me

Comment on attachment 692390 [details] [diff] [review] part 7 stop using nsIDOMCSSValue / nsIDOMRGBColor in editor r=me with the changes to AsPrimitiveValue I asked for.

Comment on attachment 692393 [details] [diff] [review] part 8 remove nsIDOMCSSPrimitiveValue:::GetRGBColor() r=me

Comment on attachment 692395 [details] [diff] [review] part 9 don't use nsIDOMRGBColor xpcom methods in nsROCSSPrimitiveValue r=me

Comment on attachment 692397 [details] [diff] [review] part 10 remove the nsIDOMRGBColor xpidl now that nothing uses it r=me. Thank you for breaking this up into pieces!

(In reply to Boris Zbarsky (:bz) from comment #24) > Comment on attachment 692390 [details] [diff] [review] > part 7 stop using nsIDOMCSSValue / nsIDOMRGBColor in editor > > r=me with the changes to AsPrimitiveValue I asked for. sure, I did AsPrimitiveValue() the way I doid to be similar to AsElement() / AsContent() but that doesn't seem hugely important. should this editor stuff still check the type before trying to cast? btw thanks for quick reviews :)

> should this editor stuff still check the type before trying to cast? If AsPrimitiveValue() returns null on incorrect type, then no. I guess you're right that what you had was similar to AsElement(); the other parent is used for the various element FromContent() methods...

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/9ea3572c80b3 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7c8138b6848a remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7645d0b506dc remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/477a1c6b95f9 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/290224a8e5fa remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/6272bd19f451 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ed93f3e4a53f remote: Trying to insert into pushlog.

https://hg.mozilla.org/mozilla-central/rev/9ea3572c80b3 https://hg.mozilla.org/mozilla-central/rev/7c8138b6848a https://hg.mozilla.org/mozilla-central/rev/7645d0b506dc https://hg.mozilla.org/mozilla-central/rev/477a1c6b95f9 https://hg.mozilla.org/mozilla-central/rev/290224a8e5fa https://hg.mozilla.org/mozilla-central/rev/6272bd19f451 https://hg.mozilla.org/mozilla-central/rev/ed93f3e4a53f

landed the rest https://hg.mozilla.org/integration/mozilla-inbound/rev/443df7602f6e remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/270ce71000f3 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/6c3f79ca958b remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/bc10be068117

was backed out for bustage in 101f652fe7a5 and relanded as remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/08fb8e6e8986 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/e238a2c12230 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/c097a455c88e remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/105aaffcfab3

https://hg.mozilla.org/mozilla-central/rev/08fb8e6e8986 https://hg.mozilla.org/mozilla-central/rev/e238a2c12230 https://hg.mozilla.org/mozilla-central/rev/c097a455c88e https://hg.mozilla.org/mozilla-central/rev/105aaffcfab3