Output console.jsm API calls to the browser console

RESOLVED FIXED in Firefox 23

Status

()

Firefox
Developer Tools
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: msucan, Assigned: msucan)

Tracking

({dev-doc-needed})

Trunk
Firefox 23
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
With the new Browser Console landing soon we want to better interact with the Console.jsm messages. We should output all calls to the browser console, in addition to dumping into the terminal as plaintext.
(Assignee)

Comment 1

5 years ago
Created attachment 725052 [details] [diff] [review]
first cut

Changes for Console.jsm to make it output to the Browser Console. I also added support for the time() and timeEnd() methods.

Remaining things to do:

- add comments.
- do we have Console.jsm tests?
- potentially use ConsoleAPIStorage.jsm to keep API calls in memory, so they don't get lost if the Browser Console is not open. My worry is that we currently group console API calls by window IDs, and jsm's have no window ID, and I need to check when we can purge objects from the storage. Should we do it in this patch?


Thoughts?
Attachment #725052 - Flags: review?(jwalker)
(In reply to Mihai Sucan [:msucan] from comment #1)
> Created attachment 725052 [details] [diff] [review]
> first cut
> 
> Changes for Console.jsm to make it output to the Browser Console. I also
> added support for the time() and timeEnd() methods.
> 
> Remaining things to do:
> 
> - add comments.
> - do we have Console.jsm tests?

No, it started as a quick hack and has never been done properly.

> - potentially use ConsoleAPIStorage.jsm to keep API calls in memory, so they
> don't get lost if the Browser Console is not open. My worry is that we
> currently group console API calls by window IDs, and jsm's have no window
> ID, and I need to check when we can purge objects from the storage. Should
> we do it in this patch?

I'd be happy to do it as a followup
Comment on attachment 725052 [details] [diff] [review]
first cut

Review of attachment 725052 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great so far
Attachment #725052 - Flags: review?(jwalker) → review+
(Assignee)

Comment 4

5 years ago
Created attachment 736896 [details] [diff] [review]
proposed patch

Cleaned-up the patch, added comments, made some bug fixes and added a test.

The test uses the new waitForMessages() helper, which I still add features to. Trying to keep actual tests as simple as possible. Suggestions are welcome.

Caching in ConsoleAPIStorage.jsm should be a follow up. I will open a bug report about that.

Thank you!
Attachment #725052 - Attachment is obsolete: true
Attachment #736896 - Flags: review?(jwalker)
(Assignee)

Comment 5

5 years ago
Try push: https://tbpl.mozilla.org/?tree=Try&rev=2b63f2db523c
(Assignee)

Updated

5 years ago
Blocks: 861338
Comment on attachment 736896 [details] [diff] [review]
proposed patch

Review of attachment 736896 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/Console.jsm
@@ +317,5 @@
> + *         An array of {filename, lineNumber, functionName, language} objects.
> + *         These objects follow the same format as other console-api-log-event
> + *         messages.
> + */
> +function getStackForConsoleAPI(aFrame, aMaxDepth = 0) {

I don't think we need both getStackForConsoleAPI and getStack do we? Can't we make callers of getStack use the data structure returned by getStackForConsoleAPI ?
Attachment #736896 - Flags: review?(jwalker) → review+
(Assignee)

Comment 7

5 years ago
Created attachment 737536 [details] [diff] [review]
updated patch

Thanks for the review. Yes, it makes sense to remove the old getStack(). I actually renamed getStackForConsoleAPI() into getStack(), since we don't keep both.

Landed in fx-team:
https://hg.mozilla.org/integration/fx-team/rev/2cecd940c219

(try push was green)
Attachment #736896 - Attachment is obsolete: true
(Assignee)

Comment 8

5 years ago
It would be nice if there would be some MDN page where devs are told about this Console.jsm and that extension authors can Cu.import it. Using the console API with the Browser Console is much nicer than alert() or dump().
Keywords: dev-doc-needed
For devtools bugs marked dev-doc-needed we should also cc Will Bamberg just to make the bug a little more visible for him.
https://hg.mozilla.org/mozilla-central/rev/2cecd940c219
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
You need to log in before you can comment on or make changes to this bug.