1838155 - Cleanup leftover \n in console.error logs after migration from Cu.reportError

Status: RESOLVED FIXED

(Firefox :: General, task, P1)

Description

[Julian Descottes [:jdescottes]]

Eg "Warning: unrecognized command line flag " + curarg + "\n"

from https://searchfox.org/mozilla-central/rev/962a843f6d96283c45162c788dc72bf217fca7df/browser/components/BrowserContentHandler.sys.mjs#1246

Which shows up as

console.error: "Warning: unrecognized command line flag -remote-allow-origins\n"

in stdout. I imagine this is a leftover from switching to console.error?

Mark: I can see that Cu.reportError was sometimes adding trailing \n. Do you know if this was intentional (eg the original version of the one I spotted here: https://searchfox.org/mozilla-central/rev/59f0cd13ab364ea4a5c4061c6c311691a9b1b01d/browser/components/BrowserContentHandler.sys.mjs#1171-1173)

Comment 1

[Mark Banner (:standard8)]

(In reply to Julian Descottes [:jdescottes] from comment #0)

Mark: I can see that Cu.reportError was sometimes adding trailing \n. Do you know if this was intentional (eg the original version of the one I spotted here: https://searchfox.org/mozilla-central/rev/59f0cd13ab364ea4a5c4061c6c311691a9b1b01d/browser/components/BrowserContentHandler.sys.mjs#1171-1173)

I don't think that was intentional - looking further back at this diff shows that it was changed from dump (which doesn't automatically append newlines), to Cu.reportError, so I guess that has probably been broken for even longer and no-one has realised it. It could be that Cu.reportError automatically hid it, but either way, I think these are safe to tidy up now.

We should also split into multiple args rather than using string concatenation i.e. console.error("Warning: unrecognized command line flag", curarg);.

Comment 2

[Julian Descottes [:jdescottes]]

Thanks for the feedback Mark! I will rename the bug for clarity.

Comment 3

[Mark Banner (:standard8)]

To clarify, I don't think we replaced any of them with console.warn, so this applies to console.error only.

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1838155 - Clean up some console.error calls that had been migrated from Cu.reportError. r?jdescottes!

This goes through the previous changes in the dependencies of bug 877389, and does two things:

1. Remove instances of \n
2. Change reporting of exceptions so that they are passed as separate arguments. This should result
   in an improved display of the exception in the browser console, should it occur.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6c42ef642d0b
Clean up some console.error calls that had been migrated from Cu.reportError. r=jdescottes,perftest-reviewers,geckoview-reviewers,credential-management-reviewers,search-reviewers,sgalich,owlish,jteow,sparky

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6c42ef642d0b