542936 - improve nsCommandLineServiceMac, runs at startup

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Josh Aas]

Attachment: fix v1.0

I was working on a different bug and I noticed that we might not need nsMacCommandLine. Getting rid of it would remove some work done during startup and make some code cleaner. I wrote a patch to remove it and it doesn't seem to change our behavior but I might be missing something.

We only seem to use it for two things - opening files on disk and printing. For opening files on disk we can just use nsICommandLineRunner directly, and the printing stuff wasn't implemented anyway.

Comment 1

[Josh Aas]

Attachment: fix v1.1

Remove an old comment.

Comment 2

[Josh Aas]

Attachment: fix v1.2

Another small fix.

Comment 3

[Josh Aas]

We can't simply remove this, though we can probably improve it and reduce its scope.

Comment 4

[Josh Aas]

Attachment: fix v2.0

We can greatly simplify this code, make it easier to understand and maybe faster.

Comment 5

[Markus Stange [:mstange]]

(In reply to comment #3)
> We can't simply remove this

Why not, after all?

Comment 6

[Josh Aas]

It does some command line monkey business, which will be easier to understand and improve in the future if this patch lands. It's pretty clear what it does with this patch applied.

Comment 7

[Markus Stange [:mstange]]

Comment on attachment 443807 [details] [diff] [review]
fix v2.0

>diff --git a/toolkit/xre/nsCommandLineServiceMac.cpp b/toolkit/xre/nsCommandLineServiceMac.cpp

>-nsresult nsMacCommandLine::HandleOpenOneDoc(const CFURLRef file, OSType inFileType)

>-  if (!mStartedUp) {
...
>-    return AddToCommandLine("-url", file);
>-  }

Why can we remove this block of code? I'm not really sure what it does...

r=me with that answered.

Comment 8

[Josh Aas]

(In reply to comment #7)

That code was used on classic Mac OS where there was no terminal - people would put args and env variables in a file. I have never seen someone use command-line file. Also, that code already wouldn't run given that we only call HandleOpenOneDoc after startup.

Comment 9

[Josh Aas]

Attachment: fix v2.1

Updated to current trunk.

Comment 10

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/c448572f61cf