609955 - Remove IDL from ConsoleAPI component

Status: RESOLVED WORKSFORME

(DevTools :: General, defect)

Description

[David Dahl :ddahl]

We should be able to treat our ConsoleAPI componentas a regular JS object that can deal with dynamic arguments lengths. Right now, we need an IDL that specifies a number of args passed into console.log/info/warn/error

I thought that once the brain transplants landed we would be able to do this by removing the idl and adding the __exposedProps__ property to the component. 

I am creating this bug to make sure - perhaps there are other things we need to do?

CCing mrbkap and jst

Comment 1

[Johnathan Nightingale [:johnath]]

Can I get more about why this should block? Does it prevent an important behaviour we want to enable in FF4?

Comment 2

[David Dahl :ddahl]

(In reply to comment #1)
> Can I get more about why this should block? Does it prevent an important
> behaviour we want to enable in FF4?

Because we have a hard limit on the number of arguments you can pass console.log in the IDL.

Comment 3

[Johnny Stenback (:jst)]

With my patch in bug 568629 you should be able to make window.console a vanilla JS object and thus not need any IDL for it at all. I'm not aware of anything else that's needed.

Comment 4

[David Dahl :ddahl]

(In reply to comment #3)
> With my patch in bug 568629 you should be able to make window.console a vanilla
> JS object and thus not need any IDL for it at all. I'm not aware of anything
> else that's needed.

oh cool. It still needs to be a component, correct?

Comment 5

[Johnny Stenback (:jst)]

You'll still need 'a' component, but all that component needs to do is to implement the interface nsIDOMGlobalPropertyInitializer and the init() method in it and then it can return whatever object you actually want to have be the value of window.console. The object you return doesn't need any XPCOM goo at all, it can be a vanilla JS object, with __exposedProps__ etc on it.

Comment 6

[David Dahl :ddahl]

Ok, this appears to be working great, thanks.

Comment 7

[David Dahl :ddahl]

(In reply to comment #6)
> Ok, this appears to be working great, thanks.

uh, hmm. no it is not. I keep getting one of the following errors: 

"__exposedProps must be undefined, null or and object"

or 

"console.log is not a function"

I tried just returning nothing from init() and then tried creating a new object and returning it, but it was a closure (which I thought might be sub-optimal in this case)

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

The first one is bug 610078.

Comment 9

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #1)
> Can I get more about why this should block? Does it prevent an important
> behaviour we want to enable in FF4?

This should block because console.log as it's defined elsewhere allows for multiple arguments to be passed to it and they'll all just get appended to the output. Ours currently has a fixed number of arguments and if we hit one of these many-argumented log functions on the web, logging will break.

(not sure what that breakage would look like, probably just lopped-off output at best, at worst, an exception).

Comment 10

[David Dahl :ddahl]

I spent some time trying to roll this into the patch in bug 587734 - but I keep hitting those errors. It sounds like the patch is slightly more involved than I thought. I thought I could just add an __exposedProps__ property to the component, and the component would only expose the console api. Do I have to create an entirely distinct object to return from init?

Comment 11

[:Gavin Sharp [email: [e-mail]]]

The latest patch in bug 568629 doesn't include the IDL, and things seem to be working fine based on preliminary testing.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

Bug 568629 landed without the IDL.