Closed
Bug 1439686
Opened 7 years ago
Closed 7 years ago
Always print console API commands from chrome callers to stdout if browser.dom.window.dump.enabled is true
Categories
(DevTools :: Console, enhancement, P3)
DevTools
Console
Tracking
(firefox60 fixed)
RESOLVED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: bgrins, Assigned: baku)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
bgrins
:
review+
|
Details | Diff | Splinter Review |
Right now, only messages coming from consoles created via console.createInstance print to stdout.
I've discussed this with Andrea, and for consistency with Console.jsm and to allow for replacing things like NS_ASSERTION it would be good to always print to stdout if browser.dom.window.dump.enabled is true, as it is in local builds: https://searchfox.org/mozilla-central/rev/0c0ddaa7e859a2b76a56a0e2e9c0de88af166812/modules/libpref/init/all.js#1099
Assignee | ||
Comment 1•7 years ago
|
||
Assignee: nobody → amarchesini
Attachment #8952784 -
Flags: review?(bgrinstead)
Reporter | ||
Comment 2•7 years ago
|
||
Comment on attachment 8952784 [details] [diff] [review]
console1.patch
Review of attachment 8952784 [details] [diff] [review]:
-----------------------------------------------------------------
I was hoping this would only affect logs coming from chrome callers, i.e. loading `data:text/html,<script>console.log("hello");</script>` would not print to stdout. I'm not sure if there's a particular harm in it doing so, other than it just being noisy and people wanting to filter it out as evidenced by feature requests like Bug 1260877. Would it be easy to restrict this so content messages don't get printed?
Attachment #8952784 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8952784 -
Attachment is obsolete: true
Attachment #8952792 -
Flags: review?(bgrinstead)
Reporter | ||
Comment 4•7 years ago
|
||
Comment on attachment 8952792 [details] [diff] [review]
console1.patch
Review of attachment 8952792 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/console/Console.cpp
@@ +826,5 @@
> }
> }
>
> + // Let's enable the dumping to stdout by default for JSM.
> + if (!mWindow && NS_IsMainThread()) {
I also want to see logging to stdout for scripts loaded in chrome windows (i.e. browser.js). So could this be just `NS_IsMainThread()?
Sorry for not being clear on the requirements in the bug description - I thought I wrote this down but it must've been on IRC.
Attachment #8952792 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8952792 -
Attachment is obsolete: true
Attachment #8952804 -
Flags: review?(bgrinstead)
Reporter | ||
Comment 6•7 years ago
|
||
Comment on attachment 8952804 [details] [diff] [review]
console1.patch
Review of attachment 8952804 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks, this works for me! Please update the commit message to indicate this is for chrome contexts only
Attachment #8952804 -
Flags: review?(bgrinstead) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b192696c08a2
Console API should print logs on stdout when used by chrome code and if browser.dom.window.dump.enabled is true, r=bgrins
Comment 8•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•