Closed
Bug 1385032
Opened 7 years ago
Closed 7 years ago
add a way to log to web console from toolbox
Categories
(DevTools :: Framework, enhancement)
DevTools
Framework
Tracking
(firefox56 fixed)
RESOLVED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: tromey, Assigned: tromey)
References
Details
Attachments
(1 file)
For bug 1345533, we need a way for the toolbox to be able to log something to the web console. I'm splitting this out to a separate bug so I can make some incremental progress.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8891009 [details] Bug 1385032 - add logErrorInPage to tab target in devtools; https://reviewboard.mozilla.org/r/162184/#review167486 ::: devtools/client/framework/target.js:722 (Diff revision 1) > + * Log a warning message to the tab's console. > + * > + * @param {String} text > + * The text to log. > + */ > + logMessage: function (text) { We should also add a `logMessage` function for WorkerTarget - at least to prevent errors (it could be a noop if we don't need it to do anything) Also, I'm thinking we should rename "logMessage" throughout to something more specific. Any ideas? I thought of "logErrorInPage, "logErrorInDebuggee", "logErrorInServer" ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_logmessage.js:16 (Diff revision 1) > + > +add_task(async function () { > + const hud = await openNewTabAndConsole(TEST_URI); > + const toolbox = hud.ui.newConsoleOutput.toolbox; > + > + const receivedMessages = waitForMessages({ You can drop the waitForMessages call / `await receivedMessages` since you are doing `await waitFor(() => findMessage(...)` instead ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_logmessage.js:23 (Diff revision 1) > + messages: [ > + { text: "beware the octopus" }, > + ] > + }); > + > + toolbox._target.logMessage("beware the octopus"); Nit: there's a `toolbox.target` getter ::: devtools/server/actors/tab.js:671 (Diff revision 1) > + onLogMessage(request) { > + let text = request.text; > + let scriptErrorClass = Cc["@mozilla.org/scripterror;1"]; > + let scriptError = scriptErrorClass.createInstance(Ci.nsIScriptError); > + scriptError.initWithWindowID(text, null, null, 0, 0, 1, > + "content javascript", getInnerId(this.window)); It would be helpful if we could pass a custom category with the request instead of using "content javascript", so that we can create Learn More links, etc. This would allow us to add a new "Source Map Load Error" category that has a link to an MDN page with tips for fixing the problem, like so: https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/errordocs.js#95. It appears that we'd also want to update documentation for new categories here (and also make sure that the list doesn't have unused ones): https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIScriptError#Categories_the_Web_Console_displays.
Attachment #8891009 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 3•7 years ago
|
||
> This would allow us to add a new "Source Map Load Error" category that has a link to an MDN page with tips for fixing the problem Great idea. I filed bug 1385337 for the MDN page; and maybe I can land the change to errordocs.js there as well (or file another followup).
Assignee | ||
Comment 4•7 years ago
|
||
> We should also add a `logMessage` function for WorkerTarget - at least to prevent errors (it could be a noop if we don't need it to do anything)
I'm going to make this a no-op for now and file a followup bug to deal with workers --
I think source maps and workers need a few fixes and more investigation on my part.
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Tom Tromey :tromey from comment #4) > I'm going to make this a no-op for now and file a followup bug to deal with > workers -- > I think source maps and workers need a few fixes and more investigation on > my part. I'm going to add a note to bug 1368680 to deal with the worker bits.
Assignee | ||
Comment 6•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8891009 [details] Bug 1385032 - add logErrorInPage to tab target in devtools; https://reviewboard.mozilla.org/r/162184/#review167486 > We should also add a `logMessage` function for WorkerTarget - at least to prevent errors (it could be a noop if we don't need it to do anything) > > Also, I'm thinking we should rename "logMessage" throughout to something more specific. Any ideas? I thought of "logErrorInPage, "logErrorInDebuggee", "logErrorInServer" I went with logErrorInPage.
Comment hidden (mozreview-request) |
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8891009 [details] Bug 1385032 - add logErrorInPage to tab target in devtools; https://reviewboard.mozilla.org/r/162184/#review167928 Thanks, looks good
Attachment #8891009 -
Flags: review?(bgrinstead) → review+
Pushed by ttromey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bfd30365fa0b add logErrorInPage to tab target in devtools; r=bgrins
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bfd30365fa0b
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•