619069 - Command line options --new-tab and --new-window not documented

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Matěj Cepl]

Neither --help message or firefox's man page contains any mention
about (quite handy) command line options for opening the page in
a new tab or window. I think it would be nice if we get at least
these two (as I believe there are some more) properly documented.

Version-Release number of selected component (if applicable):
firefox-3.6.12-1.fc14.x86_64

Comment 1

[Pascal Chevrel:pascalc]

Attachment: patch adding the options for --help

Here is a patch adding those options to --help for trunk, not sure who I should ask a review though.

Comment 2

[Pascal Chevrel:pascalc]

Comment on attachment 502237 [details] [diff] [review]
patch adding the options for --help

bug 458631 shows that gavin was the last reviewer for this area of the code :)

Comment 3

[Steffen Wilberg]

Comment on attachment 502237 [details] [diff] [review]
patch adding the options for --help

-new-tab and -new-window only applies to Firefox and Seamonkey:
https://developer.mozilla.org/en/Command_Line_Options#Browser

So adding them for all toolkit apps is wrong. You'll want to add that to e.g. nsBrowserContentHandler.js instead.
Hint: http://mxr.mozilla.org/mozilla-central/ident?i=helpInfo

Also add -search and -preferences while you're at it.

Comment 4

[Pascal Chevrel:pascalc]

oh ok, i'll provide a new patch soon, thanks Steffen

Comment 5

[Pascal Chevrel:pascalc]

Attachment: updated patch

Here is an updated patch hopefully correct now :)

Comment 6

[Steffen Wilberg]

I'm not a formal reviewer, but it looks technically ok to me.

Two little nits though:
1. Put -preferences before -search to not regress alphabetical ordering further.
2. I don't like "Options/Preferences window". It's a dialog, not a window, and it's Options on Windows and Preferences on Linux and Mac. You could duplicate that line and use #ifdef XP_WIN #else #endif to write the correct text for each platform.

Comment 7

[Pascal Chevrel:pascalc]

Attachment: updated patch

Comment 8

[Pascal Chevrel:pascalc]

Comment on attachment 503845 [details] [diff] [review]
updated patch

Updated patch including Steffen recommandations.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 503845 [details] [diff] [review]
updated patch

This isn't exhaustive, but the other flags aren't really worth exposing so that's fine.

Comment 10

[Pascal Chevrel:pascalc]

Is the push-needed keyword enough to get it landed or should I ask for a specific approval in the patch details page?

Comment 11

[Steffen Wilberg]

The correct keyword is checkin-needed (cvs terminology). push-needed is only used for website/webtool bugs (click on the "keywords" link to get a description).

And yes, it should be enough to get it landed.

Comment 12

[Pascal Chevrel:pascalc]

Thanks Steffen (I am a webdev and usually work on websites, that's probably why I made this mistake being used to put push-needed in my bugs!)

Comment 13

[(no longer active)]

http://hg.mozilla.org/projects/cedar/rev/d086123baa23

Comment 14

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/555486eb0c03

Comment 15

[Simona Badau, Desktop QA]

Verified on: 
Build identifier: Mozilla/5.0 (X11; Linux i686; rv:2.2a1pre) Gecko/20110405 Firefox/4.2a1pre