777974 - Consolidate logic for reporting errors without XPCOM loaded (`Output()` implemented in various places)

Status: NEW

(Core :: General, defect)

Description

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

The `Output()` function is implemented in at least 6 different places:
  https://mxr.mozilla.org/mozilla-central/source/xulrunner/stub/nsXULStub.cpp?rev=a15d75939cd5#51
  https://mxr.mozilla.org/mozilla-central/source/b2g/app/nsBrowserApp.cpp?rev=a15d75939cd5#36
  https://mxr.mozilla.org/mozilla-central/source/browser/app/nsBrowserApp.cpp?rev=601e2a3564ac#41
  https://mxr.mozilla.org/mozilla-central/source/mobile/xul/app/nsBrowserApp.cpp?rev=a15d75939cd5#43
  https://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#342
  https://mxr.mozilla.org/mozilla-central/source/xulrunner/app/nsXULRunnerApp.cpp?rev=3f408698a03f#38

Bugs are present in at least some of the implementations (see bug 762310).

Let's consolidate this logic into a single implementation.

Comment 1

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

My Win8 VM is taking forever to sync elm, so I'll look at this for the next hour or so!

Comment 2

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch

So far I've built and tested "browser" and "xulrunner" with this patch.  I still need to figure out how to test b2g, mobile xul, and AppRunner

Comment 3

[[e-mail]]

While reviewing the port of bug 762310 to SeaMonkey, I found two more bugs related to Output(). I don't know whether you would like to try to fix them at the same time or whether I should file separate bugs.

The first is that some of the strings passed to Output() include a trailing newline and others don't. (This seems to be a simple fix.)

The second is that MOZ_WINCONSOLE is a Makefile variable, not a C++ #define.

(In reply to Tim Abraldes from comment #2)
> So far I've built and tested "browser" and "xulrunner" with this patch.  I
> still need to figure out how to test b2g, mobile xul, and AppRunner
To build confidence in your copy & paste skills, you might find it easier to test them out on Thunderbird and/or SeaMonkey.

Comment 4

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to [e-mail] from comment #3)
> While reviewing the port of bug 762310 to SeaMonkey, I found two more bugs
> related to Output(). I don't know whether you would like to try to fix them
> at the same time or whether I should file separate bugs.

Sorry for the delayed response!  I've been head-down in Firefox for Metro work.  I don't think there's a need to file separate bugs; we can fix these issues here.  In all honesty though I'm probably not going to work on this patch until well into 2013.  If you know of someone willing to pick up this work, I'd be happy to relinquish ownership of the bug.

> The first is that some of the strings passed to Output() include a trailing
> newline and others don't. (This seems to be a simple fix.)

We'll probably want to update Output() to append a trailing newline in the non-Windows case and in the MOZ_WINCONSOLE case, then update all consumers to call Output() without a trailing newline.  That way the output will look correct in consoles and in the dialog box.

> The second is that MOZ_WINCONSOLE is a Makefile variable, not a C++ #define.

MOZ_WINCONSOLE is both a Makefile variable AND a preprocessor definition.  If you "export MOZ_WINCONSOLE=1" and then build in objdir/browser/app, you should notice that "-DMOZ_WINCONSOLE=1" appears in the arguments to cl.  The "/D" command line argument defines preprocessor definitions [1].

[1] http://msdn.microsoft.com/en-us/library/hhzbb5c8%28v=vs.100%29.aspx

Comment 5

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

It's been a year since I've looked at this. I'd like to think that someday I'll find the time to finish this up but it's time to clear it out of my dashboard.