The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in seamonkey2.0b1

Status

SeaMonkey
Startup & Profiles
--
enhancement
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.0b1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

8 years ago
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>
(Assignee)

Comment 1

8 years ago
Created attachment 363478 [details] [diff] [review]
proposed patch
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #363478 - Flags: superreview?(neil)
Attachment #363478 - Flags: review?(neil)
(Assignee)

Comment 2

8 years ago
Created attachment 363481 [details] [diff] [review]
patch v2 (with newline)

My bad, didn't see that with column width 80 in vim :-(
Attachment #363478 - Attachment is obsolete: true
Attachment #363481 - Flags: superreview?(neil)
Attachment #363481 - Flags: review?(neil)
Attachment #363478 - Flags: superreview?(neil)
Attachment #363478 - Flags: review?(neil)

Comment 3

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

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

Comment 4

8 years ago
(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

8 years ago
(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

8 years ago
Or indeed -browser, since -browser <URL> works the same way as -new-window.
(Assignee)

Comment 7

8 years ago
(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

8 years ago
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.
(Assignee)

Comment 9

8 years ago
Created attachment 366007 [details] [diff] [review]
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.
Attachment #363481 - Attachment is obsolete: true
Attachment #366007 - Flags: superreview?(neil)
Attachment #366007 - Flags: review?(neil)
Attachment #363481 - Flags: superreview?(neil)
Attachment #363481 - Flags: review?(neil)

Updated

8 years ago
Attachment #366007 - Flags: superreview?(neil)
Attachment #366007 - Flags: superreview+
Attachment #366007 - Flags: review?(neil)
Attachment #366007 - Flags: review+
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)
(Assignee)

Comment 11

8 years ago
Created attachment 366343 [details] [diff] [review]
patch v4, r+sr=neil
Attachment #366007 - Attachment is obsolete: true
Attachment #366343 - Flags: superreview+
Attachment #366343 - Flags: review+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Comment 12

8 years ago
http://hg.mozilla.org/comm-central/rev/4d2686878445
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b1
You need to log in before you can comment on or make changes to this bug.