Closed Bug 300485 Opened 15 years ago Closed 15 years ago

Launching SeaMonkey when it's already running, with "open in new tab" preference, should open a new tab rather than window.

Categories

(SeaMonkey :: UI Design, defect)

x86
Windows XP
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: jasonb, Assigned: neil)

References

Details

Attachments

(2 files)

When preferences are set to open links in a new tab, rather than a new window,
SeaMonkey is already running, and you launch it again, you should get a new tab
in the existing window rather than a new browser window.

This is in line with the "single window with multiple tabs" experience people
would expect from using tabbed browsing.

If somebody really wants a new window, they can use File -> New -> Navigator
Window, or Ctrl-N.

Implementing this should also fix the problem with some external links, called
from other applications, still opening in a new window rather than a tab.
oops.  forgot this was already filed.

*** This bug has been marked as a duplicate of 299983 ***
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
This bug isn't about opening a link, per se.  It's about what happens when you
launch another instance of Mozilla.  It's not a URL that's being opened, but the
application itself.

I'm not sure if fixing bug 299983 will fix this one.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
This is also fixed after "updating" the singlewindow extension as described in
bug 300465. 
The question in general is whether this bug will covered by the fix
for bug 121969 or whether this bug is a new rfe.
You can't do clever stuff in app startup, not even NS_NewURI :-(
Assignee: jag → neil.parkwaycc.co.uk
Status: REOPENED → ASSIGNED
Attachment #189715 - Flags: superreview?(jag)
Attachment #189715 - Flags: review?(cbiesinger)
Comment on attachment 189715 [details] [diff] [review]
Proposed patch (Windows)

>Index: nsNativeAppSupportWin.cpp
>===================================================================

>@@ -1772,8 +1779,7 @@
>       printf( "Launching browser on url [%s]...\n", arg.get() );
> #endif
>       if (NS_SUCCEEDED(nativeApp->EnsureProfile(args)))
>-        (void)OpenBrowserWindow( arg.get(), newWindow );
>-      return;
>+        return OpenBrowserWindow( arg.get(), newWindow, aResult );
>     }
> 
> 
>@@ -1785,8 +1791,7 @@
>       printf( "Launching chrome url [%s]...\n", arg.get() );
> #endif
>       if (NS_SUCCEEDED(nativeApp->EnsureProfile(args)))
>-        (void)OpenWindow( arg.get(), "" );
>-      return;
>+        return OpenWindow( arg.get(), "", aResult );
>     }
> 
>     // try for the "-profilemanager" argument, in which case we want the

You've changed the logic here from always returning to only returning on
success.

Just wanted to write this down so I won't forget, I'll look at the rest of the
patch later.
Comment on attachment 189715 [details] [diff] [review]
Proposed patch (Windows)

+	     (void)nsNativeAppSupportWin::HandleRequest( (LPBYTE)cds->lpData,
PR_FALSE, getter_AddRefs( win ) );

why are you passing PR_FALSE here? mmm... ok, so you are passing it so that a
new window isn't forced anymore?

(some docs on these arguments would be really nice)

(and a -p diff would've also been nice)

+	     nsCOMPtr<nsIDOMWindow> win;
+	     HandleRequest( LPBYTE( url.get() ), new_window, getter_AddRefs(
win ) );

maybe HandleRequest should allow passing nsnull if the caller isn't interested
in the window...

     // If a window was opened, then we're done.
     // Note that we keep on trying in the unlikely event of an error.
     if (rv == NS_ERROR_NOT_AVAILABLE || rv == NS_ERROR_ABORT || windowOpened)
{
-      return;
+      return rv;

shouldn't that be return NS_OK here?

@@ -2154,7 +2158,7 @@
   if (wwatch && sarg) {
     nsCOMPtr<nsIDOMWindow> newWindow;

newWindow can be removed

+	 nsCOMPtr<nsIDOMChromeWindow> chromeWin( do_QueryInterface( navWin ) );
+	 if ( !chromeWin ) {

should that maybe be an assertion too?

+	 io->NewURI( nsDependentCString( args ), nsnull, nsnull,
getter_AddRefs( uri ) );

I doubt that args is UTF-8, but oh well...


Was that change jag pointed out intentional?
In OpenBrowserWindow, if newWindow==PR_TRUE, shouldn't you pass OPEN_NEWWINDOW
to nsIBrowserDOMWindow? hm... I guess neither NEWTAB nor NEWWINDOW are really
great, since it should really follow the user prefs.
(sorry about the delay, the patch looked scarier than it was...)
(In reply to comment #5)
> (From update of attachment 189715 [details] [diff] [review])
>>       if (NS_SUCCEEDED(nativeApp->EnsureProfile(args)))
>>-        (void)OpenBrowserWindow( arg.get(), newWindow );
>>-      return;
>>+        return OpenBrowserWindow( arg.get(), newWindow, aResult );
Fixed locally to
rv = nativeApp->EnsureProfile(args);
if (NS_SUCCEEDED(rv))
  rv = OpenBrowserWindow( arg.get(), newWindow, aResult );
return rv;
Similarly for the second change.

(In reply to comment #6)
>(some docs on these arguments would be really nice)
OK, I've tweaked the comments for HandleRequest and OpenBrowserWindow locally.

>(and a -p diff would've also been nice)
Sorry, this is my tree without a .cvsrc, so I have to remember to type it :-/

>maybe HandleRequest should allow passing nsnull if the caller isn't interested
>in the window...
I tried that and decided it was too much effort...

>>     // If a window was opened, then we're done.
>>     // Note that we keep on trying in the unlikely event of an error.
>>     if (rv == NS_ERROR_NOT_AVAILABLE || rv == NS_ERROR_ABORT || windowOpened)
>>     {
>>-      return;
>>+      return rv;
>shouldn't that be return NS_OK here?
Fixed locally.

>>   if (wwatch && sarg) {
>>     nsCOMPtr<nsIDOMWindow> newWindow;
>newWindow can be removed
Yeah, that was left over from my null result attempt.

>>+	 io->NewURI( nsDependentCString( args ), nsnull, nsnull, getter_AddRefs( uri
) );
>I doubt that args is UTF-8, but oh well...
I checked and there are three callers. The first passes "about:blank"; the
second calls nsICmdLineService::GetURLToLoad which thoughtfully doesn't specify
the charset but I expect needs to be fixed to be UTF-8; the third caller used
NS_LossyConvertUTF16toASCII which I changed to NS_ConvertUTF16toUTF8.

(In reply to comment #7)
>In OpenBrowserWindow, if newWindow==PR_TRUE, shouldn't you pass OPEN_NEWWINDOW
>to nsIBrowserDOMWindow? hm... I guess neither NEWTAB nor NEWWINDOW are really
>great, since it should really follow the user prefs.
That would assume that I can find an nsIBrowserDOMWindow...
> That would assume that I can find an nsIBrowserDOMWindow...

oops, I forgot that there was an if (newWindow) break; above
Comment on attachment 189715 [details] [diff] [review]
Proposed patch (Windows)

r=biesi with the changes from comment 9; can you attach a patch with them at
some point? (after the sr if you want)
Attachment #189715 - Flags: review?(cbiesinger) → review+
Attachment #190315 - Flags: superreview+
Comment on attachment 190315 [details] [diff] [review]
Updated for comments

suite-only patch.
Attachment #190315 - Flags: approval1.8b4?
Attachment #190315 - Flags: approval1.8b4? → approval1.8b4+
Summary: Launching SeaMonkey when it's already running, with "open in new tab" preference, should opens a new tab rather than window. → Launching SeaMonkey when it's already running, with "open in new tab" preference, should open a new tab rather than window.
Fix checked in :-)
Status: ASSIGNED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Excellent work!  This also fixed bug 300465 and bug 121969.
Status: RESOLVED → VERIFIED
Attachment #189715 - Flags: superreview?(jag)
Depends on: 312489
You need to log in before you can comment on or make changes to this bug.