479600 - Support -new-tab and -new-window command line options

Status: RESOLVED FIXED

(SeaMonkey :: Startup & Profiles, enhancement)

Description

[Jens Hatlak (:InvisibleSmiley)]

SeaMonkey already supports this another way, using -remote "openURL(http://some.url, new-tab)" (also working on Windows for some time) but it is not very discoverable and probably hard to remember for the average user.

Looking at the FF code it seems all it takes is the few lines starting here:
<http://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserContentHandler.js#419>
Should be easy to port if we have similar (same?) functions to open tabs and windows--at least the -remote code looks similar to me:
<http://mxr.mozilla.org/comm-central/source/suite/browser/nsBrowserContentHandler.js#276>

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Attachment: proposed patch

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2 (with newline)

My bad, didn't see that with column width 80 in vim :-(

Comment 3

[[e-mail]]

Comment on attachment 363481 [details] [diff] [review]
patch v2 (with newline)

Hmm, we don't use while loops on our other params...

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #3)
> (From update of attachment 363481 [details] [diff] [review])
> Hmm, we don't use while loops on our other params...

Yes but we don't handle similar command line options yet. Without the loop this won't work as expected:
seamonkey.exe -new-tab mozilla.org -new-tab planet.mozilla.org -new-window mozillazine.org -new-window seamonkey-project.org

I'm only thinking whether uriparam should be renamed to uriParam for consistency with the other cases. Please let me know what you think.

Comment 5

[[e-mail]]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 363481 [details] [diff] [review])
> > Hmm, we don't use while loops on our other params...
> Yes but we don't handle similar command line options yet.
I would count -remote and -url as similar, and we don't use while loops there.

Comment 6

[[e-mail]]

Or indeed -browser, since -browser <URL> works the same way as -new-window.

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #5)
> > > Hmm, we don't use while loops on our other params...
> > Yes but we don't handle similar command line options yet.
> I would count -remote and -url as similar, and we don't use while 
> loops there.

Granted, but maybe that is only for historic reasons. I don't think it should prevent us from implementing the new options with more flexibility (I hope the example I provided shows that this can be handy) and compatible with FF.

(In reply to comment #6)
> Or indeed -browser, since -browser <URL> works the same way as -new-window.

Almost. FF doesn't accept an URL there and for us it's optional, unlike with the proposed -new-window behavior.

Comment 8

[[e-mail]]

Comment on attachment 363481 [details] [diff] [review]
patch v2 (with newline)

>+    var uriparam;
"param" on its own would be fine too.

>+        if (!shouldLoadURI(uri))
>+          continue;
Everywhere else we still call preventDefault if this fails.

>+        openWindow(null, getBrowserURL(), features, uri.spec);
Or consider using handURIToExistingBrowser instead.

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v3

(In reply to comment #8)
> (From update of attachment 363481 [details] [diff] [review])
> >+    var uriparam;
> "param" on its own would be fine too.

I tried to be consistent (browserParam, urlParam, chromeParam) but since this one is used for two cases I'm also fine with a more generic name.

> >+        if (!shouldLoadURI(uri))
> >+          continue;
> Everywhere else we still call preventDefault if this fails.
> 
> >+        openWindow(null, getBrowserURL(), features, uri.spec);
> Or consider using handURIToExistingBrowser instead.

Actually you're right, handURIToExistingBrowser() also does the check and allows to request opening new windows so I switched to that. I also corrected a mistake introduced by the previous patches where I passed cmdLine to handURIToExistingBrowser() in the new-tab case which is correct for FF but not for us--we have to pass in the features variable. I further checked that the -width and -height parameters are actually reflected in the features variable but somehow they seem to have no effect (neither in SM nor FF), at least here on Windows. I lost track in nsWindowWatcher.cpp but anyway that's another issue.

Comment 10

[[e-mail]]

Comment on attachment 366007 [details] [diff] [review]
patch v3

>+      while (param = cmdLine.handleFlagWithParam("new-window", false)) {
Isn't this a strict JS warning? Please put while ((param = ...) != null)

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v4, r+sr=neil

Comment 12

[Stefan [:stefanh]]

http://hg.mozilla.org/comm-central/rev/4d2686878445