Console.jsm should wrap the WebIDL console

RESOLVED DUPLICATE of bug 1425574

Status

RESOLVED DUPLICATE of bug 1425574
3 years ago
4 months ago

People

(Reporter: fitzgen, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

It is so annoying not having the real console. No time/timeEnd and a bunch of other methods.

Let's kill it from devtools and use the Real Thing.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Brian has previous argued[1] that we should use Console.jsm more, as it can offer a more configurable logging environment.

Maybe we need more discussion here first?

[1]: https://groups.google.com/d/msg/mozilla.dev.developer-tools/9plcJGyRdP4/SqilxCL0Jy8J
Flags: needinfo?(bgrinstead)
I'm not going to fully remove Console.jsm here because:

1) The addon-sdk seems to depend on it and its funkiness pretty heavily, and
2) There are a few tests that seem to depend on the funkiness of Console.jsm as well.

This work will however make bug 988636 a lot easier.
Created attachment 8644033 [details] [diff] [review]
Part 0: Expose console to system globals
Created attachment 8644034 [details] [diff] [review]
Part 1: Remove Console.jsm from the devtools loader
Created attachment 8644035 [details] [diff] [review]
Part 2: Remove most Console.jsm usage from toolkit/devtools

This removes the usage of the Console.jsm's console as a generic console in
devtools. There are a few uses of Console.jsm still in toolkit/devtools, but
they are more specialized edge cases. Examples include tests for Console.jsm,
webconsole tests that want pretty output to stdout for tbpl logs, etc.
Created attachment 8644036 [details] [diff] [review]
Part 2: Remove most Console.jsm usage from browser/devtools

This removes most Console.jsm as a generic console usage from
browser/devtools. There were a few places that I didn't feel confident changing,
most seemed related to tests related Console.jsm and profiler interaction with
Console.jsm, etc.
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1)
> Brian has previous argued[1] that we should use Console.jsm more, as it can
> offer a more configurable logging environment.
> 
> Maybe we need more discussion here first?
> 
> [1]:
> https://groups.google.com/d/msg/mozilla.dev.developer-tools/9plcJGyRdP4/
> SqilxCL0Jy8J

I will bring up discussion, but I don't think we need to block work here. I'm not removing Console.jsm, and this wouldn't inhibit work towards s/Console.jsm/DevToolsTracerThing.jsm.

Console.jsm is an inferior console compared to the webidl one, and is frequently inhibiting my debugging and profiling experience.

Whether it lives on as some custom devtools prefix logger doesn't make it a better console.
Attachment #8644033 - Flags: review?(bobbyholley)
Attachment #8644034 - Flags: review?(jryans)
Attachment #8644035 - Flags: review?(jryans)
Attachment #8644036 - Flags: review?(jryans)
+---------------+--------------------------+-------------+
|               | WebIDL console           | Console.jsm |
+---------------+--------------------------+-------------+
|assert         | Yes                      | No          |
|clear          | Yes                      | Yes         |
|count          | Yes                      | No          |
|debug          | Yes                      | Yes         |
|dir            | Yes                      | Yes         |
|dirxml         | Yes                      | Yes         |
|error          | Yes                      | Yes         |
|exception      | Yes                      | Yes         |
|group          | Yes                      | Yes         |
|groupCollapsed | Yes                      | No          |
|groupEnd       | Yes                      | Yes         |
|info           | Yes                      | Yes         |
|log            | Yes                      | Yes         |
|markTimeline   | Yes                      | Yes         |
|profile        | Yes                      | No          |
|profileEnd     | Yes                      | No          |
|table          | Yes                      | No          |
|time           | Yes                      | Yes*        |
|timeEnd        | Yes                      | Yes*        |
|timeStamp      | Yes                      | No          |
|timeline       | Yes                      | No          |
|timelineEnd    | Yes                      | No          |
|trace          | Yes                      | Yes         |
|warn           | Yes                      | Yes         |
+---------------+--------------------------+-------------+

Yes* = the method is there but doesn't seem to work correctly and doesn't hook into the profiler's waterfall.
Additionally, group/groupEnd don't seem to work the same way they do on the WebIDL console; Console.jsm throws errors where the normal console does not.
Additionally, Console.jsm does not support printf style %s substitutions.
Created attachment 8644088 [details] [diff] [review]
Part 0: Expose console to system globals
Attachment #8644033 - Attachment is obsolete: true
Attachment #8644033 - Flags: review?(bobbyholley)
Created attachment 8644089 [details] [diff] [review]
Part 1: Remove Console.jsm from the devtools loader
Attachment #8644034 - Attachment is obsolete: true
Attachment #8644034 - Flags: review?(jryans)
part 0 needs tests.
Ok, so after talking to devtools people, we are going to make Console.jsm wrap the WebIDL console as an easy way to preserve Console.jsm's prefix support and dumping to stdout.
Summary: Stop using Console.jsm in devtools → Console.jsm should wrap the WebIDL console
Comment on attachment 8644035 [details] [diff] [review]
Part 2: Remove most Console.jsm usage from toolkit/devtools

Canceling review per recent discussions until there's an updated version.
Attachment #8644035 - Flags: review?(jryans)
Attachment #8644036 - Flags: review?(jryans)
(In reply to Nick Fitzgerald [:fitzgen][:nf] from comment #16)
> Ok, so after talking to devtools people, we are going to make Console.jsm
> wrap the WebIDL console as an easy way to preserve Console.jsm's prefix
> support and dumping to stdout.

I'm in favor of that plan
Flags: needinfo?(bgrinstead)
Ok so even if we wrap the WebIDL console, time/timeEnd don't properly create markers for the waterfall. In fact it crashes because the console assumes that if we don't have mWindow, then we must be in a worker. So I started fixing that, and then got deep into refactoring the way that global markers work.

In the end, there are too many things to do here before we can move forward and I don't have the time. I was hoping this would be relatively short and sweet, but it is clear to me now that it is not that.
Assignee: nfitzgerald → nobody
Status: ASSIGNED → NEW

Updated

10 months ago
Depends on: 1425463
I have a better approach that wrapping Console API in console.jsm: in bug 1425463, I wrote a set of patches that allows JSM code to use Console API directly. It also introduces prefix, dump function, console ID and all the features that Console.jsm currently has.

Updated

10 months ago
See Also: → bug 1425574
Status: NEW → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1425574
Depends on: 1445772

Updated

4 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.