Options for where to open URLs from other applications (reuse tab, new tab, new window)

RESOLVED FIXED in Firefox1.0

Status

()

Firefox
General
P3
enhancement
RESOLVED FIXED
15 years ago
2 years ago

People

(Reporter: Duey, Assigned: Dan M)

Tracking

({conversion, fixed-aviary1.0})

unspecified
Firefox1.0
conversion, fixed-aviary1.0
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(9 attachments, 9 obsolete attachments)

1.17 KB, patch
jst
: review+
Details | Diff | Splinter Review
19.04 KB, patch
jst
: review+
peterv
: superreview+
Details | Diff | Splinter Review
5.36 KB, patch
jst
: review+
peterv
: superreview+
Details | Diff | Splinter Review
6.74 KB, patch
Brian Ryner (not reading)
: review+
Ben Goodger (use ben at mozilla dot org for email)
: superreview+
Details | Diff | Splinter Review
5.03 KB, patch
Brian Ryner (not reading)
: review+
Details | Diff | Splinter Review
7.35 KB, patch
Mike Pinkerton (not reading bugmail)
: review+
neil@parkwaycc.co.uk
: superreview+
neil@parkwaycc.co.uk
: superreview+
Details | Diff | Splinter Review
6.97 KB, patch
blizzard
: review+
Brian Ryner (not reading)
: superreview+
Details | Diff | Splinter Review
2.74 KB, patch
Details | Diff | Splinter Review
20.18 KB, image/png
Details
(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2b) Gecko/20021006 Phoenix/0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2b) Gecko/20021006 Phoenix/0.3

Tbis is a dupe of http://bugzilla.mozilla.org/show_bug.cgi?id=75138 (for
Mozilla).  There's a great discussion of it over there.  However, that RFE has
been listed for over a year now and is still marked for "future".

I think this feature would be important.  I, myself, prefer links to open new
windows.  Others would prefer them to use an existing window.  Now that you can
also browse with tabs, other people might like a combination of the two (open a
new tab in an existing window).

Right now it's pretty much random for me.  Some applications open links in a new
window while others seem to always use the top most window.  However, I think
it's important that opening new links be consitant.  I can't get all of my
applications to do the same thing, so it's logical that I should be able to set
my browser to act only one way.

I would like to open up this enhancement for Phoenix since (I think) the
developers of Phoenix can add features independant of Mozilla.  However, I'll
understand it if you feel this isn't that important.  At least you will have
considered it.

Reproducible: Always

Steps to Reproduce:

Comment 1

15 years ago
Changing summary.
Summary: [RFE} add an option to determine how other applications can open new windows in Phoenix → [RFE] Option to determine how other applications open new windows

Comment 2

15 years ago
Maybe later. Hyatt expressed interest. Confirming. 
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: [RFE] Option to determine how other applications open new windows → Option to determine how other applications open new windows

Comment 3

15 years ago
*** Bug 174928 has been marked as a duplicate of this bug. ***

Comment 4

15 years ago
*** Bug 173978 has been marked as a duplicate of this bug. ***

Comment 5

15 years ago
Related Mozilla bugs:
bug  35578 Mailnews should open http/ftp links in new browser window by default
bug  75138 pref to open links from other apps/components in a new window
bug  90992 link in mail msg should open in last-active browser window, not oldest
bug 121969 URL requested by other apps should open new tab, not window

Updated

15 years ago
Target Milestone: --- → Phoenix0.7

Comment 6

15 years ago
I think this bug is important for IE compatability, as its one of the noticable
IE features that is not in Phoenix and is frequently annoying for those IE uses
that *don't* select the "reuse windows for launching shortcuts" option (which I
think is selected by default.

I would like to see (as mentioned in the related Mozilla bug report):

o Open a new window for shortcut
o Open a new tab for shortcut
o Reuse selected tab/window for shortcut

The last text makes the behavior of the shortcut clear (which is sometimes a
complaint with people).

Comment 7

15 years ago
This is one of the few features that are missing from phoenix that I miss from
when I was still using IE!  The biggest problem I have with tabs/windows being
reused for launching shortcuts, is that it can cause you to lose whatever you
might have been working on in that tab/window.  It's happened to me far to many
times.

This bug gets my vote.  Any chance we can get this sooner than 0.7, or do we
have to wait for them to handle it on Mozilla and pick up that fix?

Comment 8

15 years ago
I actually created a bugznilla account to vote for this bug. I switched my g/f
to Phoenix after we both became frustrated with the latest IE. She loves it,
except for the fact that it reuses windows. Frequently, she'll be posting to a
message baord, and I'll AIM her a URL to include. She'll click the link, which
will obliterate her half-written post. Since the message board software
helpfully suggests that the posting URL shouln't be cached, it's now gone forever. 

This bug is her *only* complaint with Phoenix. She is not a geek, she doesn't
care about open-source, and with the exception of this bug, she loves Phoenix.
(Reporter)

Comment 9

15 years ago
A temporary work-around to stop URL's from using existing windows is to add this
line:

user_pref("advanced.system.supportDDEExec", false);

...to your user.js file (it might not exist, so create it, it belongs in your
profile directory where prefs.js is).  I'd recommend using user.js over prefs.js
because user.js is only read by Phoenix (not written) and overrides prefs.js
settings.

Comment 10

15 years ago
*** Bug 187190 has been marked as a duplicate of this bug. ***

Comment 11

15 years ago
*** Bug 188179 has been marked as a duplicate of this bug. ***

Comment 12

14 years ago
*** Bug 205426 has been marked as a duplicate of this bug. ***

Comment 13

14 years ago
The Tabbedbrowser Extension by Piro handles this.

Comment 14

14 years ago
I have a related issue.  When I start my browser, I want to do one of four things:  

1) The browser is not running.  I want to start the browser and open a blank 
   window.  I'd run the command:
       /usr/local/firebird/MozillaFirebird about:blank
   or just 
       /usr/local/firebird/MozillaFirebird
2) The browser is running, but I've forgotten that.  I want to pull up the 
   browser and open a blank tab, without destroying the already open tabs.
   I'd run the command:
       MozillaFirebird -remote "openurl(about:blank, new-tab)"
3) From another app (email client, whatever), I click on a URL and want it to 
   open in my browser, which is not alrerady running.
       /usr/local/firebird/MozillaFirebird $URL
4) Same thing, but my browser is already running.  I want:
       MozillaFirebird -remote "openurl($URL, new-tab)"

I wrote a script which accomplishes most of these tasks.  I called this script 
/usr/local/bin/firebird:

	#!/bin/sh
	 
	BROWSER=/usr/local/firebird/MozillaFirebird
	 
	LOCAL="about:blank";
	if [ $1 ] ; then LOCAL=$1 ; fi
	 
	$BROWSER -remote "openurl($LOCAL, new-tab)" &> /dev/null || \
	exec $BROWSER $LOCAL &
	
I use this command from the command line, from an icon in my Gnome panel, and 
the Gnome 'htmlview' command.

I suggest that you make this script, or something like it, part of the default
Firebird for linux install.

Comment 15

14 years ago
I checked out the latest version of the Tabbed Browser extension by Piro, and it
does, indeed, accomplish all of this -- no special shell scripts required  (:

the only problem is that it so far seems to only install under the nightly
release for win32, and attempting to install it under linux yielded no result: i
have some other arb extension installed, but this one refuses to actually
install (depsite providing a positive feedback on completion of its install
script). A real pity, because the Tabbed Browser Extension, which used to be
mildy useful, now provides all the functionality i was lacking from the browser
side of things in
phoenix^H^H^H^H^H^H^Hfirebird^H^H^H^H^H^H^Hmozilla-cut-down-browser-thingy.

I live in hope that one day the linux version will install the extension...

Comment 16

14 years ago
Regardless of the fact there's an extension available that does this, this is
really a function that should be handled directly by the browser itself.  It's
something that people should be able to configure out of the "box" without
having to hunt down and install extensions.

As far installing extensions in Linux, I reported bug 199866 a while ago about
having trouble with extensions in all Mozilla browsers, but it never went
anywhere, so it either got ignored, forgotten, or perhaps is invalid and I'm
just stupid.

Comment 17

14 years ago
the supportDDEExec = false trick no longer works as of 20030714 at least, going
back a few days as well. 

Related Mozilla bug <a
href="http://bugzilla.mozilla.org/show_bug.cgi?id=192286">here (192286)</a>

The workaround of deleting/renaming HKCR/http/shell/open/ddeexec works for me in
the meantime...

Comment 18

14 years ago
*** Bug 214501 has been marked as a duplicate of this bug. ***

Updated

14 years ago
Target Milestone: Firebird0.7 → Firebird0.8

Comment 19

14 years ago
*** Bug 218665 has been marked as a duplicate of this bug. ***

Updated

14 years ago
QA Contact: asa

Comment 20

14 years ago
*** Bug 222547 has been marked as a duplicate of this bug. ***

Comment 21

14 years ago
*** Bug 222504 has been marked as a duplicate of this bug. ***

Comment 22

14 years ago
*** Bug 222586 has been marked as a duplicate of this bug. ***

Comment 23

14 years ago
The line user_pref("advanced.system.supportDDEExec", false); placed in user.js
works fine in current builds. 

Might I ask the devs to simply include this line in all.js as an interim step?
For one thing, if the line is in all.js, then it will be available in
about:config where users can change the setting.
-> 0.9
Target Milestone: Firebird0.8 → Firebird0.9

Comment 25

14 years ago
*** Bug 234384 has been marked as a duplicate of this bug. ***

Comment 26

14 years ago
*** Bug 234759 has been marked as a duplicate of this bug. ***

Comment 27

13 years ago
*** Bug 235324 has been marked as a duplicate of this bug. ***
This needs to happen in the DDE stuff in toolkit/xre/nsNativeAppSupportWin.cpp
for windows... don't know about the other platforms. 
Target Milestone: Firefox0.9 → Firefox1.0beta

Comment 29

13 years ago
As most replies to my requests have been 'get tab browser extension' I would
just like to elaborate a little.

I should very much like to be able to have external links open in new tabs
(rather than windows) in Firefox on Mac OS X.  There are *no* tab browser
extensions compatible with OS X, thus replicating this behaviour (which is
present in Safari/Camino/OmniWeb/iCab) is near impossible.

I really think this should be part of some default options, especially as Ben
Goodger points out - Tab browser prefs muck around with Firefox's inner workings.
http://www.washingtonpost.com/wp-dyn/articles/A8165-2004Mar19_2.html

See the paragraph starting, "It will take at least a few more months for Firefox
to hit 1.0, and in that time I hope that this browser's crew of developers
addresses a few..."  The immediate next (and first overall in the article)
complaint is for this feature.
Keywords: conversion
Flags: blocking1.0?
Flags: blocking0.9?

Comment 31

13 years ago
Personally, I would really prefer that this not block 0.9.  It's been coming for
long enough, and there's enough work to do for extensions and updating... not
worth it in my very and always humble opinion.

However, I do think the current behaviour is unpredictable and annoying (without
changing advanced.system.supportDDEExec, that is...) - but that's just me.  I've
always felt that "stealing" windows is a horrible thing for any application to
do, because by and large you lose important information. (winzip, ie, etc.) 
But, some do not agree of course - maybe even most do not. (I'd love to see a
poll on this issue!)

I know there's work that should be done to standardize the way this works.  And
to make it so tabs work.  And yes, all that is very ENHANCEMENT material.  I
know that is essentially what the bug is about.  And there have been tons of
dupes, a decent number of votes, and some interest as well for the mozilla reason.

What I propose is that - whether this is fixed or not for 1.0 (which personally
I wouldn't mind if it isn't...) what would make a LOT of people happy is some
sort of interface for this:

user_pref("advanced.system.supportDDEExec", false);

Besides just about:config - it's a wonderful tool, but most of my clients would
get confused and close the window if they saw it.  If there could be a quick
option (just as a kludge to make people happy!) in Advanced -> Browsing, more
people would find this option.  At least so I think.  And that would make more
people happy.  That to me seems like almost a different bug, one that isn't just
a trivial/enhancement one.

But again, I see no reason to slow down releases for this.  If that option could
be added before 1.0 - just the DDEExec, it would make people a lot happier
without really that much hassle... and it would reduce support/duplicate-marking
work to a degree... and make Firefox look better, imho.

Personally, I believe that proper handling of this is something IE doesn't even
have to any degree; but advanced.system.supportDDEExec is something IE *does*
have in it's advanced options.

Just my cent and a half.  Probably shouldn't be posting this so few hours after
having my 7 (yes, I'm a freak of nature!) wisdom teeth pulled.... still feel
groggy from the anesthetic.

(sorry, this reply was caused by getting an email for a request for blocking
flags on the bug which I'm against... though again it's just my own opinion.)

Thanks for your time,
-[Unknown]
not a 0.9 blocker, not really a 1.0 blocker either.  It'd be nice to have, but 
making this work properly on Windows is really really nontrivial, 
unfortunately.  Remote scripts on *nix can use new-tab/new-window, but that's 
xremote
Flags: blocking1.0?
Flags: blocking1.0-
Flags: blocking0.9?
Flags: blocking0.9-

Comment 33

13 years ago
Implementation being non-trivial or not, many many many people will tell you
that this is a big issue for those migrating from IE, and thus for evangelism
and user adoption. I've seen LOTS of anectodal evidence (/., personal
experience) that first-time firefox users do not continue using the browser
after discovering this bug. 

Comment 34

13 years ago
Look, as I see it we have two options:
A) Keep the current setting which can result in data loss
B) Flip the user pref which makes links open in a new window. This does not
result in data loss

This does not seem like a hard decision to make.  What problem caused by option
B) could possibly trump data loss?

Comment 35

13 years ago
(In reply to comment #34)
> This does not seem like a hard decision to make.  What problem caused by option
> B) could possibly trump data loss?

I agree completely.  If it's as simple as changing a binary preference that
doesn't have the UI representation it should, that is obviously better than
potential data loss.

Who can re-enable blocking for 0.9 and (or, at least) 1.0?
Flags: blocking1.0-
Flags: blocking1.0+
Flags: blocking0.9-
Flags: blocking0.9+
(In reply to comment #35)
> Who can re-enable blocking for 0.9 and (or, at least) 1.0?

With all due respect, not you.  A Firefox developer already came around and
decided that this wasn't critical for 1.0, and as he's the primary Firefox
developer anyways, what he says, goes.  Please don't fiddle with the blocking
flags; they've been set that way for a reason.  (Of course, if you want to work
on programming this I'm sure Ben would be more than willing to consider any
patch you or anyone else makes.)

That said, I wonder whether the enormous effort is required to simply make each
link open in a new window as opposed to the current window/tab.  There's no
dataloss issues with that, though it is somewhat of an inconvenience.  Oh well,
I really shouldn't talk as I have no idea how easy/hard this is to code.
Flags: blocking1.0-
Flags: blocking1.0+
Flags: blocking0.9-
Flags: blocking0.9+

Comment 37

13 years ago
"I wonder whether the enormous effort is required to simply make each link open
in a new window as opposed to the current window/tab.  There's no dataloss
issues with that, though it is somewhat of an inconvenience.  Oh well, I really
shouldn't talk as I have no idea how easy/hard this is to code"

I can set this to function either way.  First, I had the pref
user_pref("advanced.system.supportDDEExec", false); 
in my user.js file.

Then, when I decided I wanted to open everything in a single window, I installed
the newest version of Tabbrowser preferences (0.5). Works with nary a problem.

I would like that line in my user.js file to be a checkbox in Options. I would
like Tabbrowser Preferences to be part of Options. 

Not difficult, now: someone else has already done the work.
advanced.system.supportDDEExec is flaky, but if someone wants to file a separate
bug on changing the default value, please do so/cc me on it.  This has always
been about a comprehensive solution to how a tabbed browser should behave.

also, I'm not the primary Firefox dev, that's ben, but while this would be nice
to have, based on the last time I mentioned this in conversation, it was made
pretty clear that we'd have to really delve into how tabbed browsing works to
make this work "right"  And said delving is not trivial/easy.
How FireFox behaves wrt loading URLs from other apps recently changed. URLs
loaded from other apps used to always open in the last active window, but with
todays branch build (and maybe some that are some number of days old too) they
open in a new window.

I find this change extremely annoying (and evidently others do too), and per a
discussiong with Ben, I'd like to see options for opening URLs loaded by
external apps in:

1) Last active window.
2) New tab in last active window
3) New window

Marking this blocking1.0+ per Ben's request.
Flags: blocking1.0- → blocking1.0+
Enjoy. 
Assignee: firefox → jst
Priority: -- → P3
Pardon my ignorance, but enjoy what?
(In reply to comment #41)
> Pardon my ignorance, but enjoy what?

Look at the bug activity and correlate it with Ben's comment time.  You'll
understand then.

Comment 43

13 years ago
Up until 0.8, Firefox opened links (such as from Thunderbird) into the same
window.  However, as of 0.9rc and 0.9final, it opens links in new windows, which
is kind of frustrating.  I would prefer it to open in the same window, or at
least to open in a new tab.  

Comment 44

13 years ago
Glad to see this made it onto the radar, and is finally blocking.  With this bug
and the four listed in Comment #5, we have almost 200 votes for this issue.

I finally tracked down the culprit in my user preferences; don't know how or
when it got set to a user-defined value, but there is a simple, boolean
preference to control this behavior:

browser.always_reuse_window

true > links from external applications will open in the frontmost window/tab.
false > opens a new window

Through Comment #5, I also found that Bug 75138 Comment #35 and Bug 121969
Comment #16 mention this hidden preference.  Those comments claim this only
works on the Mac, so YMMV.  If only the preference itself had been posted
before, it would have saved us lowly users a lot of headaches and hunting.

Now all we need is a GUI control for this preference.  And unification of the
myriad "open external links in this way" bugs.

Also, shouldn't this be VERIFIED or something?

Comment 45

13 years ago
> browser.always_reuse_window
Looks like this is Mac-specific:
http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/init/all.js#1044
http://lxr.mozilla.org/seamonkey/search?string=browser.always_reuse_window

Comment 46

13 years ago
*** Bug 248504 has been marked as a duplicate of this bug. ***
*** Bug 248449 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Summary: Option to determine how other applications open new windows → Options for where to open URLs from other applications (reuse tab, new tab, new window)
In case this should matter, Tabbrowser Preferences has a relatively simple
function called TBP_Startup() which directly implements the logic, albeit in
JavaScript, that the changed summary elucidates.

The relevant part of the function is as follows:

   switch (targetPref) {
      case 1 : {
	 TBP_Focus(topWin);
	 topWin.window.openNewTabWith(url, null, null, true);
	 window.close(); // Need to fix JS error generated here
	 return;
      }
      case 2 : {
	 TBP_Focus(topWin);
	 topWin.window.loadURI(url);
	 window.close(); // Need to fix JS error generated here
	 return;
      }
      case 3 : {
	 TBP_Focus(topWin);
	 topWin.window._content.addTab(url);
	 window.close(); // Need to fix JS error generated here
	 return;
      }
   }

0: New Window (default), 1: New Tab, 2: Current Browser, 3: New Tab, Unfocused.

Is this what the aim of this bug is meant to be?
Assignee: jst → danm.moz

Comment 49

13 years ago
Is this a difficult bug to address?  Out of curiosity, why has this bug been
open for so long, with so many duplicates, so many votes and so many comments? 
Is it a difficult engineering prospect in this case to enable the user to make
the browser behave as they wish?  If so, why?  It seems like it should be a
relatively straightforward change.
Flags: blocking-aviary1.0RC1+
Flags: blocking-aviary1.0RC1+ → blocking-aviary1.0RC1-
What is the potential l10n impact of this bug? Is it a hidden pref or does it
need UI?
Whiteboard: l10n impact unknown
(Assignee)

Comment 51

13 years ago
comment 49: the current owner really hasn't taken this bug to heart yet, because:
comment 40: busy with more important bug; not enjoying just yet. Holy crap! 94
votes! I promise to look, but not tomorrow-immediately. Someone else could take
this if there's interest.
comment 50: good question.It sounds like there already is a UI-less preference
(comment 44) on one platform that we could adopt elsewhere. But with what seems
to be such a highly visible bug, it does kind of want a localized string,
perhaps under Tools Menu -> Options -> Web Features -> Advanced.
Any changes that affect l10n need to be landed by Aug. 25 (next Wednesday). If
we're not sure we want to do this bug, we can still land the localized strings
before the l10n freeze, and get the code reviewed later.
Flags: blocking-aviary1.0PR- → blocking-aviary1.0PR+
Whiteboard: l10n impact unknown → affects l10n

Comment 53

13 years ago
*** Bug 256720 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 54

13 years ago
Created attachment 157002 [details] [diff] [review]
all the localizable strings we could want

Still I'm not especially looking at this bug. However since today is UI Day,
I'm trying to figure out what localizable text we may need. I've combined this
bug, bug 75138, and bug 121969 to come up with a proposed UI. I'm thinking of
taking a few lines out of the "Block Popup Windows" list in Tools Menu ->
Options -> Web Features to make space, and adding these +ed lines:


  |							       |
  | Add Site   Remove Site   Remove All Sites		       |
+ |							       |
+ | [ ] Open URL from other app in new window | More Options | |
+ |							       |
  | [ ] Enable Java					       |
  | [ ] Enable JavaScript		      | Advanced |     |

| More Options | opens a not-so-little subdialog that goes

| Window Reuse Options						     |
| --------------------						     |
|				 New window   New tab	Current tab  |
| Open URL from other app in	    ( ) 	( )	   ( )	     |
| Open links in webpage in	    ( ) 	( )	   ( )	     |
| Open new windows in		    ( ) 	( )	   ( )	     |
|								     |
|				       | OK |	| Cancel |	     |

Note that the main option in the Web Features dialog is the same as the top
left option in the subdialog. Also note that "Open new windows in" is a
combination of two of Sören's suggestions from bug 121969 comment 9.

I think this smacks of ridiculous overkill. I don't promise to implement the
More Options dialog at all. But I think we at least have all the strings we
could want. With that, here's a patch for Firefox 1.0 l10n purposes.

Comment 55

13 years ago
"Window Reuse Options" is _precisely_ what I want!
(I want everything to open a new tab - 
see also http://bugzilla.mozilla.org/show_bug.cgi?id=18808)
Since this usually relates to "tabbed browsing," I propose a section in the
Tabbed Browsing prefs under "Advanced" like so:

Links sent from other applications should open in: 
 (*) a new window
 ( ) a new tab in the most recent window
 ( ) the current tab/window

[x] Force links that open new windows to open in:
     (*) the same tab/window as the link
     ( ) a new tab

If we expose this feature I think this is probably the best way to do it. I've
not heard the case made for opening of any link (untargeted) ... i.e. regular <a
href="foo.html"> ... opening all of those in new tabs/windows would be really
weird. The UI above covers (I think) the most common use cases. What do people
think?
(In reply to comment #56)
> Links sent from other applications should open in: 
>  (*) a new window
>  ( ) a new tab in the most recent window
>  ( ) the current tab/window

I think the title string is a little clunky, but I'm having trouble creating one
that's any better.  As for the options, the use of "recent" and "current" in the
last two when describing the window is inconsistent; it'd be best if you always
use one or the other.

> [x] Force links that open new windows to open in:
>      (*) the same tab/window as the link
>      ( ) a new tab
> 
> If we expose this feature I think this is probably the best way to do it. I've
> not heard the case made for opening of any link (untargeted) ... i.e. regular
> <a href="foo.html"> ... opening all of those in new tabs/windows would be
> really weird. The UI above covers (I think) the most common use cases. What do
> people think?

I think this is exactly what everyone's wanted; anything beyond should IMO be
TBE-land.  I don't believe there are any other situations that would strictly
fit the concept of "Single Window Mode."

For purposes of Help documentation for these features, I assume this will go
into the Advanced panel, Browsing section.  Can you clarify exactly where in
this section these prefs would go?  Another option (I think a better one) would
be to create a Tabbed Browsing section, move "Hide the tab bar..." and "Select
new tabs..." prefs to it, move the image resize pref to Browsing, and dele
Multimedia, pretty much as below:

[Accessibility]
 ...
[Browsing]
 [x] Resize large images to fit in the browser window
 [ ] Use autoscrolling
 [ ] Use smooth scrolling

[Tabbed Browsing]
 Links sent from other applications should open in: 
  (*) a new window
  ( ) a new tab in the most recent window
  ( ) the current tab/window
 [x] Force links that open new windows to open in:
      (*) the same tab/window as the link
      ( ) a new tab
 [x] Hide the tab bar when only one web site is open
 [ ] Select new tabs opened from links

[Software Update]
 ...

No matter how it's set up in the UI, tho, it will need to be documented in Help,
and without a clear layout of exactly how it'll be done, we can't do that in
time for the l10n freeze.  (Because it sounds like the feature won't actually be
in 1.0PR, I'd guess we'll probably just add the info but <!-- --> comment it out
so that l10n teams know what text will be present there in 1.0.)

Comment 58

13 years ago
(In reply to comment #57)
> (In reply to comment #56)
> > Links sent from other applications should open in: 
> >  (*) a new window
> >  ( ) a new tab in the most recent window
> >  ( ) the current tab/window
> 
> I think the title string is a little clunky, but I'm having trouble creating one
> that's any better. 

old:
> Links sent from other applications should open in:

new:
> Open links from other applications in:

:-)
(Assignee)

Updated

13 years ago
Blocks: 75138
(Assignee)

Updated

13 years ago
Blocks: 99945
(Assignee)

Updated

13 years ago
Depends on: 257011

Comment 59

13 years ago
comment #58 has the wording pretty much spot on in my opinion. I think this is
the most intuitive.

Comment 60

13 years ago
I like how comment 57 suggests the fix along with the wording revisement from 58

Comment 61

13 years ago
*** Bug 257010 has been marked as a duplicate of this bug. ***
Good points, Jeff. I was operating under the assumption there was a tabbed
browsing section, I hadn't checked. I can create the strings for one, should we
want to add it, such as in your example. I'll add the strings for all of this
with revised text by 8/28. 
Whiteboard: affects l10n → affects l10n, ETA: 8/28 for strings.

Comment 63

13 years ago
(In reply to comment #62)
> Good points, Jeff. I was operating under the assumption there was a tabbed
> browsing section, I hadn't checked. I can create the strings for one, should we
> want to add it, such as in your example. I'll add the strings for all of this
> with revised text by 8/28. 

8/30: Ready?

;-)
I'm very busy with EM bugs right now... can someone whip up a patch to add these
strings to pref-advanced.dtd? 

Tabbed Browsing
Open links from other applications in:
a new window
a new tab in the most recent window
the most recent tab/window
Force links that open new windows to open in:
the same tab/window as the link
a new tab

(each line is a separate entity)
This should be a really easy patch to throw together quickly. Do so and I'll
review and land it. 
Keywords: helpwanted
Created attachment 157454 [details] [diff] [review]
the strings


per last comment.
Attachment #157454 - Flags: review?(bugs)
Attachment #157454 - Flags: approval-aviary?

Comment 66

13 years ago
thanks mano.
Whiteboard: affects l10n, ETA: 8/28 for strings. → [have patch] affects l10n - ready for review

Updated

13 years ago
Whiteboard: [have patch] affects l10n - ready for review → [have patch] affects l10n - ready for review ben
*** Bug 248449 has been marked as a duplicate of this bug. ***
Comment on attachment 157454 [details] [diff] [review]
the strings

r=jst to take some load off of Ben's shoulders.
Attachment #157454 - Flags: review?(bugs) → review+

Comment 69

13 years ago
Comment on attachment 157454 [details] [diff] [review]
the strings

a=asa
Attachment #157454 - Flags: approval-aviary? → approval-aviary+
Patch landed on the aviary branch.
Keywords: fixed-aviary1.0
Whiteboard: [have patch] affects l10n - ready for review ben → [have patch] affects l10n - review, waiting for checkin ben
Keywords: fixed-aviary1.0
Whiteboard: [have patch] affects l10n - review, waiting for checkin ben → l10n changes landed on aviary branch
Strings have landed, the remainder is now - for PR. 
Flags: blocking-aviary1.0PR+ → blocking-aviary1.0PR-
(Assignee)

Comment 72

13 years ago
I'm actually looking at this. Expect an implementation patch soonish.
Status: NEW → ASSIGNED

Comment 73

13 years ago
I'm not in the CC-list so why am I still getting mails from this bug??? Get me
off please!

Comment 74

13 years ago
You voted for this bug.  Remove your vote or (I think) change your preferences
to get bugzilla to stop emailing you.
*** Bug 257254 has been marked as a duplicate of this bug. ***

Comment 76

13 years ago
any updates on this? its been about 9 days since Dan M posted about him creating
an implementation patch soonish. I'm hoping to see this in 1.0PR
*Developers, please ignore this message*


(In reply to comment #76)
> any updates on this? its been about 9 days since Dan M posted about him
> creating an implementation patch soonish. I'm hoping to see this in 1.0PR

Please, people!  Give him time!  "Soonish" could mean a lot of things; the only
thing we know it means is at least the requisite review (and rereview, &c. as
necessary) period before Firefox 1.0.

First, Ben only wanted strings in for this for 1.0PR so that localizers could do
this right for other languages for 1.0.  Second, he minused it for 1.0PR after
said strings were checked into source.  Ergo, it will *not* be in 1.0PR no
matter how much you wish it.  Third, as has been posted many times in Bugzilla
in both this bug and bug 227241, these problems aren't simple to fix.  It'll
take time.  Even if jst worked non-stop on this one feature, he might or might
not have it done in nine days.  He's been doing other things, too, and those
other things may be more important right now because this feature has been
pushed to 1.0.  It's all about priorities, and right now short-term problems for
1.0PR will be higher priority than long-term 1.0 problems.  Fourth, please
remember that this bug, which originally only covered how applications open
windows, has been morphed by Ben to include bug 227241.  These two bugs command
over 180 votes right now.  That's an incredible number, and we shouldn't waste
time with extraneous comments.

Fifth, every time someone nags, that's less time that jst could be working on
the fix.  Leave him alone until he's done, or risk having this *enhancement*
(remember -- 0.9 was theoretically feature-complete?  the new features that have
gotten in have done so solely through high-level developer support) be pushed to
After Firefox 1.0.

It's your choice:  Nag and watch the patch come when it comes, nag and watch it
come later than it would have otherwise, nag and watch the patch be pushed to
after 1.0, or stay quiet and wait anxiously ;-).

I know which option has the greatest chance of getting this bug fixed promptly.
 I also know which options have absolutely no chance of getting this bug closer
to being fixed.  I hope you (now if you didn't before) know too.

(If any person here has a response to this, please respond privately.  Then
you're not wasting everyone else's time.)
(Assignee)

Comment 78

13 years ago
Created attachment 158395 [details] [diff] [review]
base code to control where external links are opened

Aviary branch patch: Implements the ability to open external links and (anchor)
targets* in a current window, new window, or new tab, configurable using prefs.
Note the patch can't exactly be applied without modification. It adds a new
file and it assumes (trunk) attachment 149457 [details] [diff] [review] from bug 242237 has been ported
to the branch (with a name change from DOMWindowAccess to DOMWindowUtils, as
discussed in that bug).

* - anchor targets were discussed earlier in this bug. I expect the groundwork
laid here will help with bug 227241.
(Assignee)

Comment 79

13 years ago
Created attachment 158396 [details] [diff] [review]
finish implementation of pref UI

Note the UI is turned on only in Windows builds. Anything else would be a lie,
since the next patch is currently implemented only on that platform.
(Assignee)

Comment 80

13 years ago
Created attachment 158397 [details] [diff] [review]
windows-only native hookup of base patch below

We'll need other platforms too. That means help would be helpful, or maybe
people will find me camped on their machines soon...

Comment 81

13 years ago
As for other platforms, doesn't Linux already have this with the -remote argument?
(Assignee)

Updated

13 years ago
Attachment #158397 - Flags: review?(bryner)
(Assignee)

Updated

13 years ago
Attachment #158396 - Flags: review?(bryner)
(Assignee)

Comment 82

13 years ago
Created attachment 158455 [details] [diff] [review]
finish implementation of pref UI

Slight tweak to pref UI. It's still Windows-only (see comment 79). -remote
needs to be looked at (comment 81) and it isn't hooked up to this code yet.
Attachment #158396 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #158396 - Flags: review?(bryner)
(Assignee)

Updated

13 years ago
Attachment #158455 - Flags: review?(bryner)
(Assignee)

Updated

13 years ago
Attachment #158397 - Attachment description: windows-only native hookup of base patch above → windows-only native hookup of base patch below
(Assignee)

Comment 83

13 years ago
Created attachment 158459 [details] [diff] [review]
base code to control where external links are opened

Updated base patch. Comment 78 still applies.
Attachment #158395 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #158459 - Flags: review?(jst)
(Assignee)

Updated

13 years ago
Blocks: 257011
No longer depends on: 257011

Updated

13 years ago
Blocks: 258076
What exactly is supposed to happen with the prefs UI?

In comment 57 I suggested doing a little reorg so that all the tabbed stuff was
together and so that multimedia would be eliminated and the one pref there added
to browsing.  As I first understood comment 62, I thought this would happen, but
as I reread it it seems the exact decision has been put off to a later date. 
What's the decision here?  In attachment 158455 [details] [diff] [review] from my quick scan the reorg
isn't happening, which means that tabbed prefs are strewn through the browsing
and tabbed browsing sections.

I need to know for bug 258076, which would document this (assuming it's allowed
as late-l10n, which in my completely biased opinion it should be because
otherwise the Help docs will be blatantly wrong).  I posted a halfway patch, but
primarily because the state of a prefs reorg isn't clear it was turned down (not
unexpectedly, but not for the reasons I wanted).
(Assignee)

Comment 85

13 years ago
Created attachment 158695 [details] [diff] [review]
pref UI plus reorg tab topics (comment 57)

Same UI as before, but include a rearrangement of tabbing prefs as discussed in
comment 57. We seem to have implicit approval for this from Ben in comment 62.
(Assignee)

Updated

13 years ago
Attachment #158455 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #158455 - Flags: review?(bryner)
(Assignee)

Comment 86

13 years ago
Created attachment 158696 [details] [diff] [review]
pref UI plus reorg tab topics (comment 57)
Attachment #158695 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #158696 - Flags: review?(bryner)
Is this going to miss the boat for 1.0? If it is, I'd like to know so that I 
can plan to rip out all of the code that myself and another person spent a 
while perfecting ;-)
Comment on attachment 158459 [details] [diff] [review]
base code to control where external links are opened

- In browser/base/content/browser.js:

+const nsCI		  = Components.interfaces;
...
+nsBrowserAccess.prototype =
+{
+  QueryInterface : function(aIID)
+  {
+    if (aIID.equals(Components.interfaces.nsIBrowserWindow) ||
+	 aIID.equals(Components.interfaces.nsISupports))

Since you define nsCI above, you should use it here too...

r=jst
Attachment #158459 - Flags: review?(jst) → review+
(Assignee)

Updated

13 years ago
Attachment #158459 - Flags: superreview?(peterv)

Comment 89

13 years ago
Just couple comments after building and testing Firefox with the latest patches
from this bug.

1) It seems to work pretty well, haven't had any problems so far. It doesn't
force javascript links into new tabs though, but I don't think this was meant to
cover them in the first place? (For example, clicking a link in an gmail email
opens a new window even with 'Force links that open in new windows open in' set
to new tabs)

2) 'Visit Home Page' links in Extensions/Themes managers always open in new
windows, in my opinion these should honor the set Tabbed Browsing options.

3) An option to force Firefox to remain on the background would be nice, that
way you could just click all links (in an email, for example) and then switch to
Firefox instead of clicking a link, switching the focus back to the email client
after Firefox steals it, click another link etc.

Comment 90

13 years ago
Agreed with comment 89.
Thoughts on window.open: this is something where more care should be taken. I
may sound obvious or redundant here, but as I haven't seen this idea being
commented/implemented ever (neither on this bug nor on extensions that reproduce
this behavior), this is my suggestion: only open new tabs instead of windows for
links which don't specify any options (and, maybe, for links that specify
options that reproduce a default-looking browser window). If a link specifies,
for example, width/height or toolbar=no, it most likely intends to open a small
popup window, and creating a new tab window (where most people use maximized
browsers) will look quite ugly.
If the user really desires to open every single window.open call in a new tab,
there could be a hidden preference for that -- or, if you prefer, another
option, though IMHO that would be much more like clutter for the average user.

Comment 91

13 years ago
(In reply to comment #89)
> 3) An option to force Firefox to remain on the background would be nice, that
> way you could just click all links (in an email, for example) and then switch to
> Firefox instead of clicking a link, switching the focus back to the email client
> after Firefox steals it, click another link etc.

IMHO, that might be nice but it's kinda a different bug, no?

(In reply to comment #90)
I totally agree... it doesn't make sense to open *everything* in a new tab, and
often that's the only reason for using javascript instead of just a link -
although sometimes it's for popups.

How do these patches affect Linux/Mac OS X/etc. users?  How crucial is it to
implement this in those?  I mean, if it means that all Linux users get options
in the UI that do nothing, imho it should wait until after 1.0 or something...

-[Unknown]

Comment 92

13 years ago
From comment 89:
> 3) An option to force Firefox to remain on the background would be nice, that
> way you could just click all links (in an email, for example) and then switch to
> Firefox instead of clicking a link, switching the focus back to the email client
> after Firefox steals it, click another link etc.

I agree that this should be a separate bug, though probably not that necessary.
 I was able to click on the link in Thunderbird, and still read e-mail until I
switched to Firefox to read.  How I did this was after clicking on the link, I
switched to another unread message, and Firefox stayed in the background. 
Sometimes, it's better to click and read, so it should be a user pref, key
combo, or an extension.  Definately another bug though.
(Assignee)

Comment 93

13 years ago
comment 89: The patch as it stands has two problems that I know of: it works
only when the frontmost window's primary tab is active; it only works well with
http://-type links, e.g. not file:// or about:. Just a forewarning; you'll find
those bugs quickly enough. I have them corrected in my build but the glacial
speed of reviews discourages me from restarting the process with new patches.
Note to reviewers: the patches given require some small tweaks. Don't let that
stop you.

As you say it doesn't work for javascript: links containing script that opens a
new window. I'm not sure whether that's part of this bug's mission. Maybe. I'm
running with an additional patch that does just that, but I haven't posted it
yet because I'm still evaluating it and I expect troubles from it. For one thing
even that patch is unable to correct the longstanding error that
middleclick-in-new-tab will fail on such links.

An option to force FF to remain in the background while you fill it with tabs
would be slick. I'd use that. But it's outside the scope of this bug and it
could be problematic, depending on whether the choice of activating the program
after it opens the link is entirely up to the program on a given OS.

comment 90, 91, 92: I see your point but I'm not sure what heuristic to use to
determine whether a new window should submit to becoming a tab, since defining
an average coding behaviour for web authors is a task for madmen. Regardless
this would apply only to javascript:window.open-type links, which we've covered
above. Given our time constraints, this may be a post-1.0 tweak we're discussing.

comment 91: This patch is cross-platform for opening links from within the page.
The part about external links is implemented only on Windows. That part of the
UI is turned off on non-Windows platforms. (Actually in the current posted patch
the whole UI is turned off, but that's unnecessary.) I could take a donated
patch for that functionality on those platforms, but I'm prepared to do it
myself and I know pretty much what needs to be done and where. I'm just not
interested in getting started until the overall direction passes review. It's
only halfway there.

Comment 94

13 years ago
Dan (comment 93): my suggestion of heuristics is pretty simple: an option-less
call goes for a new tab. Example:
Open in new tab: window.open("x.html"); window.open("y.html", "somename");
Open in new window: window.open("z.html", "somename", "toolbar=no,status=no");

If you want another approach: open in a new window any call that features any of
these options: left, top, width, height.
I don't think popup windows (the ones that intend to be useful popups, not just
opening new windows) would not have width/height option parameters.

I just made a quick-hack check (replacing top.window.open with a custom
function) and Gmail seems to use window.open() with no parameters other than URL
for external links, so the first approach would work there. But to be more sure
maybe somebody should try using Venkman there.

Comment 95

13 years ago
*Developers, please ignore this message*

I am new to firefox development, and wish to use this new feature what you have
done. Where can I get the working copy of mentioned comment #89. Night build?

Regards

Comment 96

13 years ago
(In reply to comment #91)
> How do these patches affect Linux/Mac OS X/etc. users?  How crucial is it to
> implement this in those?  I mean, if it means that all Linux users get options
> in the UI that do nothing, imho it should wait until after 1.0 or something...

As crucial as it is to do it for the Windows users, IMHO.  Look, I know the
number of windows users of FF spectacularly dwarf the size of the Linux/Mac OS X
crowd, but it's damned unfair to leave us out of the equation.  This feature has
already been available for the Windows crowd in the Tab Browser Extensions
extension, but so buggy for non Windows users, it's almost unusable.  Could this
be also be a priority for the Mac crowd?
(Assignee)

Updated

13 years ago
Attachment #158459 - Flags: superreview?(peterv)
(Assignee)

Comment 97

13 years ago
Created attachment 159442 [details] [diff] [review]
base code to control where URLs are opened

Updated base patch for Aviary branch. I've made changes enough while waiting
that I thought a new patch was in order. This is like the previous patch r+ed
by jst with these changes:

Changed the names of the prefs. Call me Arbitrary.
Made C++/JS contact the chrome window rather than the content window (bug fix).

Use nsIURI rather than nsIURL (bug fix).
Fetch pref every time in nsDocShell, rather than caching. (Caching was
  confusing. Alternatively we could go with
nsIPrefBranchInternal->AddObserver.)
Replaced browser.block.target_new_window, now redundant.

Dangit I'm planning on keeping my r+ from jst on the previous patch but maybe
you guys should look again.
(Assignee)

Updated

13 years ago
Attachment #158459 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #159442 - Flags: superreview?(peterv)
Attachment #159442 - Flags: review?(jst)
(Assignee)

Updated

13 years ago
Attachment #158696 - Flags: review?(bryner)
(Assignee)

Updated

13 years ago
Attachment #158397 - Flags: review?(bryner)
(Assignee)

Comment 98

13 years ago
Created attachment 159443 [details] [diff] [review]
optional addition to divert window.open to a new tab
(Assignee)

Updated

13 years ago
Attachment #157002 - Attachment is obsolete: true
(Assignee)

Comment 99

13 years ago
Created attachment 159444 [details] [diff] [review]
pref UI
Attachment #158696 - Attachment is obsolete: true
(Assignee)

Comment 100

13 years ago
Comment on attachment 159444 [details] [diff] [review]
pref UI

Like the previous (unreviewed) UI patch, but updated to match the latest base
patch above. Also includes a bug fix.
Attachment #159444 - Flags: review?(bryner)
(Assignee)

Comment 101

13 years ago
Created attachment 159445 [details] [diff] [review]
windows-only native hookup of external URL control

comment 96: read comment 93.
Attachment #158397 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #159445 - Flags: review?(bryner)
(Assignee)

Updated

13 years ago
Attachment #159443 - Flags: review?(jst)

Comment 102

13 years ago
For the Linux guys: Edit the firefox shell-script. Search for the line
#_open_type="tab". Remove the # and add it to the line above with "window".

I'd really like to know why this isn't the default in
mozilla/browser/app/mozilla.in.

Comment 103

13 years ago
(In reply to comment #96)
> (In reply to comment #91)
> > How do these patches affect Linux/Mac OS X/etc. users?  How crucial is it to
> > implement this in those?  I mean, if it means that all Linux users get options
> > in the UI that do nothing, imho it should wait until after 1.0 or something...
> 
> As crucial as it is to do it for the Windows users, IMHO.  Look, I know the
> number of windows users of FF spectacularly dwarf the size of the Linux/Mac OS X
> crowd, but it's damned unfair to leave us out of the equation.  This feature has
> already been available for the Windows crowd in the Tab Browser Extensions
> extension, but so buggy for non Windows users, it's almost unusable.  Could this
> be also be a priority for the Mac crowd?

As a FF user on both Mac and Windows, I have to agree with comment #96.  I
realize that platforms would not deliberately be excluded w/o a reason, but I
think a lot of people are unclear as to what the reason is.  TBE takes care of
this functionality with a beautiful amount of configurability, but on Mac TBE is
really slow and buggy.  Is there a way to implement this functionality via a
script/config change?

Also, since there is a need to keep the UI for this feature simple for the
"average Joe" user, it would be nice if any configuration options that are
deemed too confusing/complex be displayed in the UI be enabled in the config w/o
a UI.

Just my 2.

Comment 104

13 years ago
"Look, I know the number of windows users of FF spectacularly dwarf the size of
the Linux/Mac OS X crowd, but it's damned unfair to leave us out of the equation."

It seems to me with the -remote options, Linux users already have the majority
of this, and have had for years. I'm just glad Windows users finally get the
same feature.
attachment 159442 [details] [diff] [review]

for dom/public/idl/base/nsIBrowserWindow.idl, please move the doxygen comment
directly before the interface definition, so that doxygen actually knows that
the comment belongs to the interface

nsDocShell.cpp
+            NS_ASSERTION(0, "Can't get nsIDOMWindowInternal from nsDocShell!");

while touching this, please make it NS_ERROR

attachment 159443 [details] [diff] [review]

+      ios->NewURI(url, 0, baseURI, getter_AddRefs(tabURI));

you could use NS_NewURI; but in any case, please do pass a charset. looks like
you can use nsIDocument::GetDocumentCharacterSet for this

+        if (domReturn && !nameString.EqualsIgnoreCase("_blank"))

.LowerCaseEqualsLiteral

Comment 106

13 years ago
Camino has implemented some of this functionality. Perhaps code to implement
this on Mac OS X can be gotten from the Camino source base.

Comment 107

13 years ago
(In reply to comment #89)
...
> 3) An option to force Firefox to remain on the background would be nice, that
> way you could just click all links (in an email, for example) and then switch to
> Firefox instead of clicking a link, switching the focus back to the email client
> after Firefox steals it, click another link etc.

Didn't it used to work this way? Somehow I remember that being the case. I would
also like to see this.
Comment on attachment 159442 [details] [diff] [review]
base code to control where URLs are opened

r=jst
Attachment #159442 - Flags: review?(jst) → review+
Comment on attachment 159443 [details] [diff] [review]
optional addition to divert window.open to a new tab

- In GlobalWindowImpl::OpenInternal():

       rv = SecurityCheckURL(url.get());
   }
[...]
+  if (divertOpen) {
+
+    // construct URI, in case we need it
+
+    nsCOMPtr<nsIURI> tabURI;
+
+    nsCOMPtr<nsIIOService> ios(do_GetService(NS_IOSERVICE_CONTRACTID));
+    if (ios) {
+      nsCOMPtr<nsIDOMDocument> domdoc;
+      nsCOMPtr<nsIURI> baseURI;
+      GetDocument(getter_AddRefs(domdoc));
+      nsCOMPtr<nsIDocument> doc(do_QueryInterface(domdoc));
+      if (doc)
+	 baseURI = doc->GetBaseURI();
+
+      ios->NewURI(url, 0, baseURI, getter_AddRefs(tabURI));

This uses the base URI of the document in the window on which open() is being
called, but SecurityCheckURL() (rightfully) uses the base URI of the document
in the window where the call comes from. Do the same here. Split out the code
in SecurityCheckURI() into a helper if you want to...

r=jst with that fixed.
Attachment #159443 - Flags: review?(jst) → review+

Comment 110

13 years ago
I was thinking that this will probably eliminate maybe 4 or 5 extensions for me.

One I am not too sure of though is the one for when a new window opens when you
go to download which is fixed here.
http://forums.mozillazine.org/viewtopic.php?t=61134

I'm not sure if this fits with the context of this bug or not, but wanted to
mention it before it was too late.
Attachment #159444 - Flags: review?(bryner) → review+
Comment on attachment 159445 [details] [diff] [review]
windows-only native hookup of external URL control

>--- toolkit/xre/nsNativeAppSupportWin.cpp	5 Aug 2004 03:49:23 -0000	1.6.6.7
>+++ toolkit/xre/nsNativeAppSupportWin.cpp	19 Sep 2004 22:53:31 -0000
>@@ -1572,13 +1583,48 @@ nsNativeAppSupportWin::OpenBrowserWindow
>             // Have to open a new one.
>             break;
>         }
>+
>+        nsCOMPtr<nsIBrowserWindow> bwin;
>+        { // scope a bunch of temporary cruft
>+          nsCOMPtr<nsIWebNavigation> navNav( do_GetInterface( navWin ) );
>+          nsCOMPtr<nsIDocShellTreeItem> navItem( do_QueryInterface( navNav ) );
>+          if ( navItem ) {
>+            nsCOMPtr<nsIDocShellTreeItem> rootItem;
>+            navItem->GetRootTreeItem( getter_AddRefs( rootItem ) );
>+            nsCOMPtr<nsIDOMWindow> rootWin( do_GetInterface( rootItem ) );
>+            if ( rootWin ) {
>+              nsCOMPtr<nsIDOMWindowUtils> utils( do_GetInterface( rootWin ) );
>+              if ( utils )
>+                utils->GetBrowserWindow( getter_AddRefs( bwin ) );
>+            }
>+          }
>+        }
>+        if ( bwin ) {
>+          nsCOMPtr<nsIIOService> ios(do_GetService(NS_IOSERVICE_CONTRACTID));

We should probably just return if this fails (NS_ERROR_OUT_OF_MEMORY, perhaps).
 Also, make this block consistent with the ( parentheses style ) used
elsewhere.

>+          if ( ios ) {
>+            nsCOMPtr<nsIURI> uri;
>+            nsDependentCString urlStr(args);
>+            ios->NewURI( urlStr, 0, 0, getter_AddRefs(uri) );

Same here, return early IMO.

>+            if ( uri ) {
>+              nsCOMPtr<nsIDOMWindow> container;
>+              rv = bwin->OpenURI( uri,
>+                                  nsIBrowserWindow::OPEN_DEFAULTWINDOW,
>+                                  nsIBrowserWindow::OPEN_EXTERNAL,
>+                                  getter_AddRefs(container) );
>+              if ( NS_SUCCEEDED(rv) )
>+                return NS_OK;

If this really can't be expected to fail we might want to just assert and then
unconditionally return here, removing the code below this.  Is there a known
reason it would fail, where the code below is useful?
(Assignee)

Comment 112

13 years ago
Yeah you caught me. The nsIBrowserWindow clause isn't expected to fail, but
still I cowardlyishily left in both fallback clauses. I'm good with putting an
assertion in place of fallback #1.

Actually as far as I can tell fallback #2 shouldn't ever be necessary either but
there are so many ways to get to it I'm uncertain. So I figure I should leave
that one in. How's this? (Note all of fallback #1, navigating the content window
directly, has been removed):

        if ( bwin ) {
          nsCOMPtr<nsIURI> uri;
          nsDependentCString urlStr( args );
          NS_NewURI( getter_AddRefs( uri ), urlStr, 0, 0 );
          if ( uri ) {
            nsCOMPtr<nsIDOMWindow> container;
            rv = bwin->OpenURI( uri,
                                nsIBrowserWindow::OPEN_DEFAULTWINDOW,
                                nsIBrowserWindow::OPEN_EXTERNAL,
                                getter_AddRefs( container ) );
            if ( NS_SUCCEEDED( rv ) )
              return NS_OK;
          }
        }

        NS_ERROR("failed to hand off external URL to extant window\n");
    } while ( PR_FALSE );

    // open a new window if caller requested it or if anything above failed
Comment on attachment 159445 [details] [diff] [review]
windows-only native hookup of external URL control

That sounds ok to me.
Attachment #159445 - Flags: review?(bryner) → review+
Comment on attachment 159445 [details] [diff] [review]
windows-only native hookup of external URL control

That sounds ok to me.
(Assignee)

Updated

13 years ago
Attachment #159442 - Flags: approval-aviary?
(Assignee)

Updated

13 years ago
Attachment #159444 - Flags: superreview?(bugs)
Attachment #159444 - Flags: approval-aviary?
(Assignee)

Updated

13 years ago
Attachment #159445 - Flags: superreview?(blizzard)
Attachment #159445 - Flags: approval-aviary?
(Assignee)

Updated

13 years ago
Attachment #159443 - Flags: superreview?(peterv)
Attachment #159443 - Flags: approval-aviary?
Comment on attachment 159444 [details] [diff] [review]
pref UI

Please do not check in the change to pref-advanced.dtd on the aviary branch.
(In reply to comment #115)
> (From update of attachment 159444 [details] [diff] [review])
> Please do not check in the change to pref-advanced.dtd on the aviary branch.

So, where do you want it to be shown?

Comment 117

13 years ago
*** Bug 245747 has been marked as a duplicate of this bug. ***

Comment 118

13 years ago
Comment on attachment 159445 [details] [diff] [review]
windows-only native hookup of external URL control

let's get some reviews before we consider for approval.
Attachment #159445 - Flags: approval-aviary?

Updated

13 years ago
Attachment #159444 - Flags: approval-aviary?

Updated

13 years ago
Attachment #159443 - Flags: approval-aviary?

Updated

13 years ago
Attachment #159442 - Flags: approval-aviary?

Updated

13 years ago
Flags: blocking0.9-

Updated

13 years ago
Blocks: 161466

Updated

13 years ago
Blocks: 227241
Comment on attachment 159442 [details] [diff] [review]
base code to control where URLs are opened

What biesi said, plus

> Index: docshell/base/nsDocShell.cpp
> ===================================================================

> @@ -1089,22 +1090,60 @@ nsresult nsDocShell::FindTarget(const PR

> +    if (mustMakeNewWindow) {
> +        rv = NS_ERROR_FAILURE;

Is this really necessary? Seems like we'll always set rv below.

> +        if (linkPref == nsIBrowserWindow::OPEN_NEWTAB) {
> +
> +          // try to get our tab-opening interface

I think this file uses 4-spaces indentation.
Attachment #159442 - Flags: superreview?(peterv) → superreview+
Yes we have no bananas. 

Sorry I've been busy this week travelling back to California. Sunday, I promise!
...updating my tree so I can apply these...
ok, where is nsIBrowserWindow? new file? no patch? my build is broken!
OK now this doesn't build for other reasons, I'm going to clobber my whole tree....

Comment 124

13 years ago
*** Bug 261872 has been marked as a duplicate of this bug. ***
Comment on attachment 159444 [details] [diff] [review]
pref UI

sr+a=ben@mozilla.org
Attachment #159444 - Flags: superreview?(bugs)
Attachment #159444 - Flags: superreview+
Attachment #159444 - Flags: approval-aviary+
Comment on attachment 159445 [details] [diff] [review]
windows-only native hookup of external URL control

sr+a=ben@mozilla.org
Attachment #159445 - Flags: superreview?(blizzard) → approval-aviary+
Comment on attachment 159442 [details] [diff] [review]
base code to control where URLs are opened

a=ben@mozilla.org
Attachment #159442 - Flags: approval-aviary+
Attachment #159442 - Flags: approval-aviary+
Attachment #159444 - Flags: approval-aviary+
Attachment #159445 - Flags: approval-aviary+
jst and I are united in our vigilant and tenacious stance against invading
mozilla/browser namespace from mozilla/dom IDL, so
s/nsIBrowserWindow/nsIDOMBrowserWindow/g -- including the filename, natch.

/be

Comment 129

13 years ago
shouldn't that be nsIDOMNSBrowserWindow to avoid invading DOM namespace?
This bug's patch was not held up for all those days danm bitterly keeps count of
over a naming issue, so I won't hold it up now -- I said yesterday, "r+sr+a=me,
have a drink on me" to danm in person.

But according to our own poor-man's namespacing conventions, there ought to be
an nsIDOM on the front of the name.  We can fix this later if ever we care to,
or else ben or whoever promulgates the new base browser window interface can
obfuscate the name that emerges under browser in deference to this patch -- or
we can try to get all browser app owners to agree on common extensions (with
uuid changes) to dom/public/idl/base/nsIBrowserWindow.idl.

I dont' care, but little naming and modularity messes count, they add up and
make the tree hard to change, and harder than necessary for newcomers to learn.

Timeless points out an old Netscape-centric vidur convention for trying to avoid
invading the w3c interface namespace that follows the nsIDOM prefix, but it was
never guaranteed, Netscape is effectively dead, and the w3c DOM working group
has disbanded.

Check it in, already!  If you need 1.7 branch driver approval, dveditz can do that.

/be
Please don't change the nsIBrowserWindow name now, the app-startup patch would
be massively conflicted. We can do it later, if that's necessary.
Comment on attachment 159443 [details] [diff] [review]
optional addition to divert window.open to a new tab

>Index: dom/src/base/nsGlobalWindow.cpp
>===================================================================

>+    nsCOMPtr<nsIIOService> ios(do_GetService(NS_IOSERVICE_CONTRACTID));
>+    if (ios) {
>+      nsCOMPtr<nsIDOMDocument> domdoc;
>+      nsCOMPtr<nsIURI> baseURI;
>+      GetDocument(getter_AddRefs(domdoc));
>+      nsCOMPtr<nsIDocument> doc(do_QueryInterface(domdoc));
>+      if (doc)
>+        baseURI = doc->GetBaseURI();
>+
>+      ios->NewURI(url, 0, baseURI, getter_AddRefs(tabURI));
>+    }

I also would prefer NS_NewURI here.

>+  // lacking specific instructions, or just as an error fallback,
>+  // open a new window.
>+
>+  if (!domReturn) {
>     nsCOMPtr<nsIWindowWatcher> wwatch =
>       do_GetService(NS_WINDOWWATCHER_CONTRACTID, &rv);
> 
>-    if (wwatch) {
>+    if (!domReturn && wwatch) {

You already checked domReturn.

With what biesi and jst said, sr=peterv.
Attachment #159443 - Flags: superreview?(peterv) → superreview+
Amazing!

You guys have just removed the basic core of functionality provided by the
various tabbed browsing extensions. Thanks :) (no  seriously, external link
handling was a major pain to do with just JavaScript)

What exactly does attachment 159443 [details] [diff] [review] do? I see it has r+ and sr+, so it's going
to be checked in, but does it really do what it says - divert window.open calls,
namely the ones made by JavaScript, into tabs?
(Assignee)

Comment 134

13 years ago
Created attachment 160667 [details] [diff] [review]
mac-only native hookup of external URL control (Aviary branch)
(Assignee)

Comment 135

13 years ago
Comment on attachment 160667 [details] [diff] [review]
mac-only native hookup of external URL control (Aviary branch)

Requesting review. Note I also want to remove the now unused, Mac-only,
redundant pref browser.always_reuse_window from all.js (I just forgot to
include it in the diff). If you want to try this patch you must also use the
#ifdef XP_WIN version of
browser/components/prefwindow/content/pref-advanced.xul at line 43. I'll check
in that change once the Mac and Linux backends are all checked in.
Attachment #160667 - Flags: review?(pinkerton)
(Assignee)

Updated

13 years ago
Attachment #160667 - Attachment description: mac-only native hookup of external URL control → mac-only native hookup of external URL control (Aviary branch)

Comment 136

13 years ago
Works great and thank you to everyone that woked on this my only gripe is...When
using the search bar, how can i get it to display results in a new tab?
(In reply to comment #136)
> Works great and thank you to everyone that woked on this my only gripe is...When
> using the search bar, how can i get it to display results in a new tab?

You can't. I've now read the patches and I don't see anything influencing how
the searchbar works. Attachment 159442 [details] [diff], the core of the new code, doesn't appear
to fool around with #searchbar in any way. TBP will continue to offer that bit
of functionality though.

Comment 138

13 years ago
> When using the search bar, how can i get it to display results in a new tab?
Press Alt+Return. Works in the location bar as well.
I know this comment is probably too late (for 1.0) but could:

Select new tabs opened from bookmark or history
Select new tabs opened from links

be changed in

Focus on new tab opened from Bookmark or History
Focus on new tab opened from links

I didn't understand the "select" and I wasn't the only one.


Comment on attachment 160667 [details] [diff] [review]
mac-only native hookup of external URL control (Aviary branch)

looks good to me, r=pink
Attachment #160667 - Flags: review?(pinkerton) → review+
(Assignee)

Updated

13 years ago
Attachment #160667 - Flags: superreview?(neil.parkwaycc.co.uk)

Comment 141

13 years ago
Maybe a regression...can't remember what happend before.

When either left-clicking or middle-clicking the Extension manger on 'Get More
Extensions' a new window opens, clicking on 'Visit Home Page' on the context
menu in the extension manager and Theme manager same thing for 'Get More Themes'. 

and yes all of my settings are correct
(Assignee)

Comment 142

13 years ago
Created attachment 160692 [details] [diff] [review]
linux-only native hookup of external URL control (Aviary branch)

Can't wait to find out what I did wrong in this patch! This leaves the option
to specify new-tab or new-window intact, but makes a remote command lacking any
such spec do as dictated by the prefs.

The "if you want to try this patch" part of comment 135 also applies to this
patch.

PS I replaced the old notification broadcast technique for opening new tabs
because the new technique is more flexible, and I needed that flexibility in
DocShell. I have a table of all the methods that I could find for opening new
tabs used throughout the program, and which are good for what. It's impressive.
I'm adding yet another, and I'm trying to do a little consolidation.
(Assignee)

Updated

13 years ago
Attachment #160692 - Flags: superreview?(bryner)
Attachment #160692 - Flags: review?(blizzard)
(In reply to comment #139)

See bug 246274 comment 13 and bug 246274 comment 14.  (Please also note that
your concern is irrelevant to this bug and should have been voiced elsewhere.)

Comment 144

13 years ago
*** Bug 262409 has been marked as a duplicate of this bug. ***

Comment 145

13 years ago
Finally! Someone points me to the bug to fix one of my longest-standing gripes
about Firefox and browsers in general -- the incredibly annoying window clutter
that results after only a few minutes of browsing because every web developer
out there seems to think they need to open a new window for seemingly every link
on all their sites. Why this is is I don't know, since it's not that difficult
to hold down a key while you click the mouse to open a window/tab if you want to
open a new window.

Anyhow, I'm not a programmer, but I felt some comments would hopefully be
well-received by those who are working on this issue.

On some sites this seems to work for me
(http://forums.vwvortex.com/zerosearch?cmd=recenttopics forces the links to
topics you've posted in recently to load in the same window, for instance) while
not in others (like
http://www.ncbi.nlm.nih.gov/entrez/query.fcgi?cmd=Retrieve&db=PubMed&dopt=Citation&list_uids=11208057
-- clicking on the image that leads to the article spawns a new window). That's
inconsistent. If I've selected options to force links to open in new windows, I
want them to always do so.

I still find that the UI for controlling outside-app links, listed in comment
56, is missing. Presumably it's still being worked on, but I sorely miss it;
xJournal (a Livejournal client) spawns a new window rather than a new tab when
you tell it to check the friends page, thus contributing to window clutter.

My build info is:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20041001
Firefox/0.10

Comment 146

13 years ago
danm, thanks a lot for fixing this.

One thing though, if "Select new tabs opened from links" is unchecked, your code
does not select the tabs it creates, which is quite confusing and inconvienient.

This is reported as bug 262537. As I said there, the "Select new tabs opened
from links" option should apply to middle-clicked links, not left-clicked ones.

Updated

13 years ago
No longer blocks: 227241

Comment 147

13 years ago
*** Bug 227241 has been marked as a duplicate of this bug. ***
*** Bug 227241 has been marked as a duplicate of this bug. ***

Comment 149

13 years ago
I have to say I agree with comment 146, although such a change would require the
already difficult to phrase options to be relabeled to clarify the left vs
middle click behavior.

Comment 150

13 years ago
(In reply to comment #141)
> When either left-clicking or middle-clicking the Extension manger on 'Get More
> Extensions' a new window opens, clicking on 'Visit Home Page' on the context
> menu in the extension manager and Theme manager same thing for 'Get More Themes'. 
> 
> and yes all of my settings are correct
Please see Bug 262575.

Comment 151

13 years ago
This type of URL is still popping up a new window with "new tab" enabled.

<a href="#"
onClick="window.open('http://sports.espn.go.com/nfl/gamecast/index','Popup','width=780,height=540,scrollbars=no,noresize');
return false">GameCast</a>

Comment 152

13 years ago
mmortal03:
see comment 94 and comment 98. The pref to change behavior is
browser.link.open_newwindow.restrict
0 means no restrictions. 1 means only if no options

Comment 153

13 years ago
(In reply to comment #152)
> mmortal03:
> see comment 94 and comment 98. The pref to change behavior is
> browser.link.open_newwindow.restrict
> 0 means no restrictions. 1 means only if no options
> 

Thanks.  Problem is, that link still opens in a new window, even with
browser.link.open_newwindow.restrict set to 0.  I double and triple checked for
spelling errors.  To try out that link directly, go to www.espn.com , and click
on any of the Gamecast links in the left sidebar.

Comment 154

13 years ago
Dan, I filed bug 262977 and bug 262978 for issues with focusing and links from
external applications. Both are quite confusing for users.
(They are filed as UNCO because I can't get a recent build to test with. Please
somebody on current nightly confirm them)
mmortal03:

I was having your problem. I did a dig through the source and the pref has been
changed to browser.link.open_newwindow.restriction. It can have the following
values:

0: no restrictions - divert everything
1: don't divert window.open at all
2: don't divert window.open with features

Comment 156

13 years ago
Great progress on this bug.  I see that checkins have been made in the Aviary,
but I was wondering about the possibility of this functionality in Seamonkey.  

Could anyone let me know if Seamonkey implementation is planned?  Thanks.

Comment 157

13 years ago
(In reply to comment #151)
> This type of URL is still popping up a new window with "new tab" enabled.
> 
> <a href="#"
>
onClick="window.open('http://sports.espn.go.com/nfl/gamecast/index','Popup','width=780,height=540,scrollbars=no,noresize');
> return false">GameCast</a>

The ESPN URL is now 404 but when I tried it earlier it didn't open a new window
for me.  However I am still having problems with the PubMed link I listed
earlier -- checked the source and it does look like it is using window.open to
open new windows. I have both prefs set to 0 as type 'integer' in about:config.
Am I doing something wrong?
Has this been checked into the 0.10 branch ("latest-0.9")?  I see a preference
for opening links in new window or new tab (force links that open in new
windows...) but that only applies to links inside Firefox -- the control for
links from external applications doesn't seem to be landed.
(Assignee)

Comment 159

13 years ago
There was a time when bugzilla pages were attuned more toward getting the bug
fixed than a question and answer forum. I miss those days.

Comment 154: thanks.
Comment 155: sigh.
Comment 156: sure.
Comment 157: this bug is still under construction.
             window.open is unaffected in builds before Oct 4.
Comment 158: see comment 93, comment 134, comment 142.
Comment 159: release the bugspam!
Dan, are technical comments on the patches ok?

In particular:

1)  The nsGlobalWindow.cpp changes only check for the name "_blank"; they should
    also check for "_new" (see the actual targeting code in docshell, eg).
2)  These same changes break window.open() into a previously opened window or
    frame, as far as I can tell, for the case when 
    |containerPref ==   nsIBrowserWindow::OPEN_NEWTAB| (it'll now load in a new
    tab, even if a frame/tab/window with that name already exists).

The second issue is likely to bite on a lot of sites if users set that pref...

On a non-code tangent, I hope this'll be landing on trunk someday too? ;)

Comment 161

13 years ago
The links in my feedlist on bloglines.com don't work anymore if I set Firefox to
open links in new tabs. Instead of opening the news items in the other frame,
Firefox opens a blank tab. 

This is the code which is used:

function doLoadf(index,subid,siteid) 
{
  parent.basefrm.location.href = '/myblogs_display?sub='+subid+'&site='+siteid;
  subNoUnread(index,subid,siteid);
}

...
 aux = insDoc( cFolder, 
          jitem = gLnk("S", "  The Burning Edge  ", 
               "javascript:doLoadf(6,1595305,9394)", "title=\"  The Burning Edge
 \"", "", "9394" ))
...

If browser.link.open_newwindow.restriction is set to 1 the problem doesn't exist. 

Updated

13 years ago
Whiteboard: l10n changes landed on aviary branch → [have patch] need reviews for linux and mac
Comment on attachment 160692 [details] [diff] [review]
linux-only native hookup of external URL control (Aviary branch)

The logic looks good to me.
Attachment #160692 - Flags: review?(blizzard) → review+
(Assignee)

Comment 163

13 years ago
Created attachment 161419 [details] [diff] [review]
don't divert window.open to a new tab if the named window exists

Boris: Thanks for catching that. Here's a patch for you to review :)

By the way can I just say that window targeting by name is so very, very messed
up all throughout Mozilla. The appearance of it mostly working is dependent on
the way all the different broken subroutines support each other into a mostly
functional whole. See bug 103638.

Anyway, this patch will fix comment 160 and probably comment 161, which sounds
like the same thing, as well. Gert-Paul, please try your problem with this
patch (wait for it to land if you must) and if your problem still persists,
start a new bug, attach a usable testcase, and assign it to me.
(Assignee)

Updated

13 years ago
Attachment #161419 - Flags: superreview?(jst)
Attachment #161419 - Flags: review?(bzbarsky)
Comment on attachment 161419 [details] [diff] [review]
don't divert window.open to a new tab if the named window exists

> window targeting by name is so very, very messed up

Agreed.  We need some itility functions (possibly on windowwatcher?  Or
something?) to deal with it, at the least -- the copy/paste of code between
here, docshell, and windowwatcher is incredibly uncool.

In fact, it'd be nice to see such utility functions land on trunk along with
this patch, if you're up to it...  If not, please file a followup bug on that
specific issue and cc me.

>Index: dom/src/base/nsGlobalWindow.cpp
>+  nsAutoString nameString(aName);
...
>+  if (nameString.EqualsIgnoreCase("_top") ||
>+      nameString.EqualsIgnoreCase("_self") ||
>+      nameString.EqualsIgnoreCase("_content") ||
>+      nameString.EqualsIgnoreCase("_parent") ||
>+      nameString.Equals(NS_LITERAL_STRING("_main")))

This is ok for branch, but for trunk this should use LowerCaseEqualsLiteral and
EqualsLiteral respectively, both just called on aName.

>-  if (!aExtraArgument) { // divert only if no extra argument
>+  if (divertOpen) { // no such named window

Any reason to push the !aExtraArgument check down below the QI here?  It looks
like it just makes us do an unnecessary QI sometimes, since the QI result is
effectively ignored if aExtraArgument.

r=bzbarsky with that.

Eagerly awaiting trunk patches for this stuff, too.  ;)
Attachment #161419 - Flags: review?(bzbarsky) → review+
Created attachment 161474 [details]
Desktop clutter :)

For your enjoyment: the current state of things :)
 
Am glad this subject is being taken care of.

Comment 166

13 years ago
I just noticed this isn't quite working 100% yet, and I thought it was.

Not sure if this one applies here or not:

Go to http://boston.redsox.mlb.com/NASApp/mlb/index.jsp?c_id=bos

and click on game day button (around 9 o'clock) and a new window pops open,
though all my other ones open in new tabs.

Comment 167

13 years ago
I think we need to create a sticky for the option
browser.link.open_newwindow.restriction.  :)

Sasquatch Bigfoot, please try this option, and set it to 0. I had the same
problem as you did, and this is the solution, as discussed earlier in this thread.

Comment 168

13 years ago
I think we need to create a sticky for the option
browser.link.open_newwindow.restriction.  :)

Sasquatch Bigfoot, please try this option, and set it to 0. I had the same
problem as you did, and this is the solution, as discussed earlier in this thread.

Might it be better to have 0 as the default for this option?  It seems that when
people choose to have all windows open in a a new tab, they MEAN all windows. 
This is confusing when some windows don't follow this.
Comment on attachment 161419 [details] [diff] [review]
don't divert window.open to a new tab if the named window exists

sr=jst
Attachment #161419 - Flags: superreview?(jst) → superreview+

Comment 170

13 years ago
(In reply to comment #168)
> ... It seems that when
> people choose to have all windows open in a a new tab, they MEAN all windows. 
> This is confusing when some windows don't follow this.

This is what it does mean, and it IS confusing.

Comment 171

13 years ago
*** Bug 263643 has been marked as a duplicate of this bug. ***

Comment 172

13 years ago
Using this enhancement has amplified Bug 131456 .  Now that we have a Single
Window Mode, it is necessary that closing tabs release memory in the same way
that closing windows does.  I am finding firefox.exe using up a lot of memory
after a day of keeping the same window open and just opening new tabs (which
shouldn't be a problem, but is).
Comment on attachment 160667 [details] [diff] [review]
mac-only native hookup of external URL control (Aviary branch)

rs=me for the branches but it would be nice to have the nsIBrowserDOMWindow
object stored as an attribute of the nsIChromeWindow.
Attachment #160667 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #160667 - Flags: superreview+
Attachment #160692 - Flags: superreview?(bryner) → superreview+
(Assignee)

Updated

13 years ago
Keywords: helpwanted → fixed-aviary1.0
Whiteboard: [have patch] need reviews for linux and mac
Target Milestone: Firefox1.0beta → Firefox1.0

Comment 174

13 years ago
I think bug #263844 is a regression from this?
A popup opened in a tab using java script is not allowed to window.close(); itself.
(Assignee)

Comment 175

13 years ago
No. That's a very old bug.
Dan, how so?  Popups opened via window.open() can close themselves fine here (on
trunk, at least).  So if they can't do it after being forced into a tab, the
problem is in the code forcing them into a tab, no?  Or at least in the way
window.close() is handled for tabs (which would indeed be a very old bug, but
we're exposing it, all of a sudden.  Prior to this checkin, the only way to
expose it was to manually open tabs in a popup window).
(Assignee)

Comment 177

13 years ago
Yes, it's just that it's more exposed now. window.close() has never, ever worked
on windows opened into a tab. This discussion should be moved to that other bug.
Dan, until this checkin there _weren't_ any such windows, is the point.
at the end of this page (comments part):
http://www.ynet.co.il/articles/0,7340,L-2989051,00.html

the window.open heght & width features unexpectedly affect the current window
(not as other window features). However the following testcase _doesn't_ have
this behavior:

----------------
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">

<html>
 <head>
   <title>testcase</title>
   <meta http-equiv="Content-Type" content="text/html; urf-8">
 </head>
 <body>
    <a href="javascript:void(0);"
      onclick="window.open('about:blank', 'test',
'toolbar=no,location=no,directories=

no,status=yes,menubar=no,scrollbars=auto,movable=yes,resizable=no,width=430,height=417')

;">
      Click me
    </a>
 </body>
</html>
----------------
(Assignee)

Comment 180

13 years ago
Boris:
>>window.close() has never, ever worked on windows opened into a tab.
>Dan, until this checkin there _weren't_ any such windows, is the point.

I don't see the distinction between tabbed windows introduced by this
enhancement, and middle-click tabbed windows.

Asaf: 
If that was a bug report, please write it up so that I can understand what to do
and what to expect, and put it in a new bug.
danm, the difference is that script never opened middle-click tabs, so it can't
reasonably expect to close them. But since script is now opening tabs through
redirected window.open, it has every expectation that it will be able to close
those windows.

Comment 182

13 years ago
Actually, bz, those types of tabs/windows _have_ existed before this patch.
Being an exclusive tab-browser person, I've seen it hundreds of times, but as
you said yourself in #176.
(Assignee)

Comment 183

13 years ago
>danm, the difference is that script never opened middle-click tabs, so it can't
>reasonably expect to close them. But since script is now opening tabs through
>redirected window.open, it has every expectation that it will be able to close
>those windows.

Nor can script open left-click tabs in current nightlies. The decision of
whether to tab a new window is, as always, up to the program and the user's prefs.

Firefox 0.8: User left-clicks <a target="_blank">: new window can be closed from
script. User middle-clicks same link: new window (tab) cannot be closed from
script. Now can we please take this to the relevant bug? Feel free to + it too
if you believe this old bug's recent increased visibility makes it more important.

Comment 184

13 years ago
After reading the comments, I can't tell if this bug will be fixed in Mozilla or
only in Firefox.  There are indeed related Mozilla bugs (e.g. bug 231984).  

Three of the four open bugs this one blocks are "Browser" and not "Firefox".  If
this enhancement is not going to be implemented in the Mozilla browser, how does
it block other bugs for the Mozilla browser?  
I've been using this feature on linux fc2 for several days, and it works nicely
so far (eg, links from thunderbird open in new/same tab in firefox, ditto for
links in gaim).

Comment 186

13 years ago
(In reply to comment #107)
> (In reply to comment #89)
> ...
> > 3) An option to force Firefox to remain on the background would be nice, that
> > way you could just click all links (in an email, for example) and then switch to
> > Firefox instead of clicking a link, switching the focus back to the email client
> > after Firefox steals it, click another link etc.
> 
> Didn't it used to work this way? Somehow I remember that being the case. I would
> also like to see this.

It was working here this way with 2004100505 (all external links were opened in
the background *g* ). Unfortunatelly in 2004101414 the behaviour changed. Did
anything change between these two builds? 
BTW: Is there a new bug regarding 3)? Or is there now an option to open links
from external apps in the background?

Comment 187

13 years ago
Clicking on a link in Thunderbird does not bring it up at all (it does nothing)
in Firefox from today (10/18/2004). 

The same version Thunderbird with Bangbang023's and the "official" 10/14 build
opens the link fine, so it is NOT a Thunderbird issue. This is with the Bangbang
or 10/14 "official" build already open. With today's "official build" already
open, it does nothing. It also won't open anything if no browser open, where it
used to open the default browser before.

Comment 188

13 years ago
*** Bug 255504 has been marked as a duplicate of this bug. ***
*** Bug 266078 has been marked as a duplicate of this bug. ***
Bug 246274 is now showing as closed, but the terminology in the preferences
panel remains very confusing and the preference panel UI does seem to be under
discussion in this bug (comment 56), so while not wishing to draw the ire seen
in comment 143, I *think* that this is relevant to this bug track. If I'm wrong,
and I should be reopening 246274, I'm sure someone will let me know :)

There was a proposal for what seems like a sensible compromise (bug 246274
comment 16) of "switch to". That would make the prefs UI look like:

Tabbed Browsing
(...)
  [ ] Switch to new tabs opened from links
  [ ] Switch to new tabs opened from bookmarks or history
(...)

But to me, even that is confusing. I propose that the confusion lies in the fact
that the preference is an on/off toggle, but the statement contains two verbs
(switch and open) which makes it harder to parse. By reversing the preference so
that it describes a single action, it becomes much easier to understand: 

Tabbed Browsing
(...)
  [ ] New tabs from links open in background
  [ ] New tabs from bookmarks or history open in background
(...)
I, uh, forgot to make it explicit, but I guess the proposal for the terminology
change also means reversing the effect of the checkbox. I suppose the strings
could be "... open in foreground", but that seemed a little less intuitive to
me. YMMV.

Comment 192

13 years ago
The new version of the prefs is much clearer. 

Comment 193

13 years ago
#190 makes a lot more sense to me. Is that in Aviary yet? 
(Assignee)

Comment 194

13 years ago
My monthly drive-by mass bug diligence:

Comment 184: What looks like an organizational observation is probably a request
to adapt the patch to the trunk and the Suite. I suppose this is a new
statement; after all this bug was opened specifically against Phoenix and
previous comments look forward only to a trunk adaptation.
  Sure, as far as I know the Suite wants this capability as well. It's not
ready. We don't have all the bugs worked out yet and frankly I'm a little tired
of Mozilla hacking right now.

Comment 186: Bug 263553.

Comment 187: I assume this problem has since magically evaporated.

Comment 190: Mike that argument has been going for months and it seems this bug
was where it took place. Still I consider it an ancillary issue and contentious
enough that it would be better served in a dedicated bug. For full flavour you
should bring comments 6, 54, 57, and 139, as well as similar comments from bugs
75138 and 121969 with you. By the way I prefer the current wording to the
proposal's. Discussion in a development forum or the new bug, please.

Comment 193: Eh?
There's an interesting issue, now on Linux.

It works pretty well when launching firefox with an argument set to a full url,
including protocol: stuff, but not when it is not included.

i.e. launching firefox www.google.com doesn't follow these settings while
firefox http://www.google.com does.

Comment 196

13 years ago
What is the official position on a new option to distinguish between regular
links and window.open calls when opening windows in a new tab?  A simple link in
a page is very different than a window.open call with specific size and layout
properties meant to be viewed in a specific format.

Should a new bug be opened?
(Assignee)

Updated

13 years ago
Blocks: 103843
For the record, this fix introduced bug 267249, once this lands on the trunk,
we'll need to land that fix on the trunk as well...
Blocks: 267249

Comment 198

13 years ago
Still blocking Aviary? Need to change Target Milestone: to 1.1 and add a
blocking 1.1 field.

Comment 199

13 years ago
(In reply to comment #168)
> I think we need to create a sticky for the option
> browser.link.open_newwindow.restriction.  :)
> 
> Sasquatch Bigfoot, please try this option, and set it to 0. I had the same
> problem as you did, and this is the solution, as discussed earlier in this thread.
> 
> Might it be better to have 0 as the default for this option?  It seems that when
> people choose to have all windows open in a a new tab, they MEAN all windows. 
> This is confusing when some windows don't follow this.


I don't know if this was missed or not, but I wanted to reiterate that I second
the option to make 0 the default.

Updated

13 years ago
Blocks: 263956

Comment 200

13 years ago
I highly recommend that all who are interested in this look at how the Netscape
people have handled this in thier Firefox-derived browser.

Comment 201

13 years ago
*** Bug 272560 has been marked as a duplicate of this bug. ***
Keywords: aviary-landing
*** Bug 272638 has been marked as a duplicate of this bug. ***

Comment 203

13 years ago
(In reply to comment #200)
> I highly recommend that all who are interested in this look at how the Netscape
> people have handled this in thier Firefox-derived browser.

Agreed. The Netscape Preview makes tab options simple and easy to understand.
Why take up so much room with radio buttons when a pulldown menu does the same
thing?
(In reply to comment #203)
>
> Agreed. The Netscape Preview makes tab options simple and easy to understand.
> Why take up so much room with radio buttons when a pulldown menu does the same
> thing?

Sounds similar to what I did for TBP. Any screenshots?

Comment 205

13 years ago
(In reply to comment #204)
> Sounds similar to what I did for TBP. Any screenshots?

The specific Tab Browsing pane:
http://gemal.dk/misc/nsb09.png

Other screenshots of the new Netscape Browser:
http://gemal.dk/blog/2004/11/30/netscape_browser_screenshots/
Comment on attachment 160667 [details] [diff] [review]
mac-only native hookup of external URL control (Aviary branch)

File->New Window on mac when no window is open, was broken for some time on the
aviary branch.

Considering the points this patch touced, and that the regression isn't repro'
on latest trunk builds, I wonder if this one caused it.

Please double-check before landing it on the trunk.
this checkin changed nsIDOMChromeWindow.idl but did not change its uuid. this
will break binary compat with C++ code using this interface. please update its uuid.

Comment 208

13 years ago
> this checkin changed nsIDOMChromeWindow.idl but did not change its uuid. this
> will break binary compat with C++ code using this interface. please update its
uuid.

where did these interface changes land?  please please don't forget to change
the UUID of interfaces when they change.  it's the only thing that gives
extension authors and embedders a fighting chance at using non-frozen interfaces.
Did nsIDOMChromeWindow.idl's UUID not change on the aviary branch?  Or did it
change there but not on the trunk?

/be

Comment 210

13 years ago
This seems to be the right bug to comment, bonsai shows me the checkin "single
window mode aviary branch merge. bug 172962, 263689, 263844, 263960
r=bryner,jst,peterv" to mozilla/ docshell/ base/ nsDocShell.cpp

The problem with that checkin is that it seems to have killed the preference
browser.block.target_new_window that is widely used by Seamonkey addicts,
although it is still referenced in a few locations in the source. At least that
pref does no longer have any effect on target="_blank" links...
(In reply to comment #210)
> The problem with that checkin is that it seems to have killed the preference
> browser.block.target_new_window that is widely used by Seamonkey addicts,
> although it is still referenced in a few locations in the source. At least that
> pref does no longer have any effect on target="_blank" links...

This is not a bug. The pref is meaningless now and is replaced with the prefs
under the browser.link pref branch.
If it's meaningless, references to it should be removed from the tree and
relevant documentation should be updated accordingly...
(Assignee)

Comment 213

13 years ago
Ed et.al. (comment 200, 203-205): I think "Netscape's" UI is better, myself. Why
not file an enhancement bug requesting a review of Firefox's UI? Such a concern
will not be addressed in this bug, which will be closed "fixed" soon.

Brendan, Darin (comment 208-209): The situation with nsIDOMChromeWindow's UUID
is all taken care of. Thanks to Christian for pointing out the problem.

Boris, Bradley, Peter (comment 210-212): browser.block.target_new_window was
completely excised from the trunk by the migration of this patch. Errr except
for one definition, completely unused, in embedding/minimo/all.js. That too
could be removed. Firefox 1.0 collected instances of this complaint for several
months as people adjusted, and now we'll see a similar wave of surprise pass
over the trunk.

Comment 214

13 years ago
Sorry for the confusion. I looked up "browser.block." using lxr and the files
displayed there obviously haven't been updated in in the last 2 days. At least
that's what I see, the files on my disk and from bonsai blame are OK.

I hope that someone makes sure that this is an item for the 1.8a6 release notes
as many Seamonkey users seem to have browser.block.target_new_window=true in
user.js.
Dan, mind updating http://www.mozilla.org/unix/customizing.html to reflect reality?

Comment 216

13 years ago
(In reply to comment #212)
> If it's meaningless, references to it should be removed from the tree and
> relevant documentation should be updated accordingly...

I think the documentation is key here to keep track of things. 

Comment 217

13 years ago
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a6) Gecko/20041208 Firefox/1.0+

Links from other applications now correctly open a new tab instead than a new
window, but they got focus even when "select new tabs opened from links" is
unchecked.
(Assignee)

Comment 218

13 years ago
Single Window Mode is implemented in the Firefox 1.0 branch and on the trunk. On
the trunk I think there are problems with embedded or native apps like Camino,
and I expect there to be issues with external links in the Suite. Surprisingly
I've noticed no such bugs being filed. When such bugs do spring up, we'll deal
with them there. I'm sticking a fork in this one.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Comment 219

13 years ago
Oh. And comment 217, see bug 262537. And the Suite needs a UI. I'll leave that
too to another bug.

Comment 220

13 years ago
(In reply to comment #219)
> Oh. And comment 217, see bug 262537. And the Suite needs a UI. I'll leave that
> too to another bug.

I think that "other" bug might be 161466 .

Updated

13 years ago
Keywords: aviary-landing
Can someone please explain to me where the patch to nsIDOMChromeWindow.idl to add 

attribute nsIBrowserDOMWindow browserDOMWindow;

came from?

See:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsIDOMChromeWindow.idl&branch=&root=/cvsroot&subdir=mozilla/dom/public/idl/base&command=DIFF_FRAMESET&rev1=1.5&rev2=1.6

I don't see it in any of the patches in this bug.

Comment 222

13 years ago
*** Bug 274834 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Blocks: 274859

Updated

13 years ago
No longer blocks: 274859

Comment 223

13 years ago
What and when was the final fix on this bug? I seem to have lost something in
the translation here. Was it based off a 10/13 attachment? Thanks. Or should
this be a  "WONTFIX"? Thanks again.

Comment 224

13 years ago
For an education in flexibility and clarity of language, check out Tools >
Options > Tab Browsing in the second version of Netscape's prototype browser
based on Firefox.
(In reply to comment #218)
> Single Window Mode is implemented in the Firefox 1.0 branch and on the trunk. On
> the trunk I think there are problems with embedded or native apps like Camino,
> and I expect there to be issues with external links in the Suite. Surprisingly
> I've noticed no such bugs being filed. When such bugs do spring up, we'll deal
> with them there. I'm sticking a fork in this one.

FYI, for Camino the bug is bug 274143, but it's not so much a bug as a general
reminder and RFE; the old "pseudo-SWM" prefs, or a subset thereof that were
implemented, were never accessible via the UI.

Comment 226

13 years ago
(In reply to comment #223)
> What and when was the final fix on this bug? I seem to have lost something in
> the translation here. Was it based off a 10/13 attachment? Thanks. Or should
> this be a  "WONTFIX"? Thanks again.

Anyone?
it was marked fixed with comment 218

Updated

12 years ago
Depends on: 255123

Comment 228

12 years ago
(In reply to comment #168)
> I think we need to create a sticky for the option
> browser.link.open_newwindow.restriction.  :)
> 
> Sasquatch Bigfoot, please try this option, and set it to 0. I had the same
> problem as you did, and this is the solution, as discussed earlier in this thread.
> 
> Might it be better to have 0 as the default for this option?  It seems that when
> people choose to have all windows open in a a new tab, they MEAN all windows. 
> This is confusing when some windows don't follow this.


Was there ever a bug made up for this option?




(In reply to comment #194)
...
> Comment 190: Mike that argument has been going for months and it seems this bug
> was where it took place. Still I consider it an ancillary issue and contentious
> enough that it would be better served in a dedicated bug. For full flavour you
> should bring comments 6, 54, 57, and 139, as well as similar comments from bugs
> 75138 and 121969 with you.... Discussion in a development forum or the new
bug, please.

Did we ever get a bug number for this?

Comment 229

12 years ago
(In reply to comment #195)
> There's an interesting issue, now on Linux.

Same issue on Windows (XP), with Firefox 1.0.3 (French language)

If (gui) option is set to open external urls in new/current tab
(not on new window) -- without using the tabbrowser extension:

1. Using "firefox http://www.google.com" works as expected (new/current tab)
   ok

2. Using "firefox goole.com" open a new windows (not new/current tab)
   Looks like I discover the same BUG.

Hope this helps

(In reply to comment #228)

> (In reply to comment #194)
> > 75138 and 121969 with you.... Discussion in a development forum or the new
> bug, please.
> 
> Did we ever get a bug number for this?

The new labels for these preferences ("Select new tabs opened from ...") seem to
work pretty well, so I left it alone and didn't bother opening one, no.

Comment 231

12 years ago
*** Bug 105547 has been marked as a duplicate of this bug. ***

Comment 232

12 years ago
*** Bug 121969 has been marked as a duplicate of this bug. ***

Comment 233

12 years ago
*** Bug 121969 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Blocks: 163993

Updated

12 years ago
Blocks: 121969

Updated

12 years ago
No longer blocks: 163993

Updated

12 years ago
Blocks: 128632

Comment 234

10 years ago
I vote for: open links in last unpopulated, (untitled) or blank tab.

Comment 235

10 years ago
This is 2 years old, and has been implimented and resolved.

Status:  RESOLVED FIXED  
Severity:  enhancement  
Target Milestone:  Firefox1.0

I don't mean to be rude or anything, but what you're asking for seems to be more like a new request rather then an old.

It's a good idea in theory, we could just stop opening 5 thousand blank tabs all the time, but face it we're too lazy for that.

It would only be good for pages pointing to about:none, as a large amount of web pages are too lazy for <titles> also.

Comment 236

9 years ago
ok

Updated

8 years ago
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1?

Updated

8 years ago
Flags: blocking-firefox3.6?

Comment 237

8 years ago
Created attachment 377306 [details]
screenshots

Comment 238

8 years ago
Comment on attachment 377306 [details]
screenshots

(In reply to comment #237)
> Created an attachment (id=377306) [details]
> screenshots

Don't spam bugzilla with junk pictures that have absolutely nothing to do with anything.
Attachment #377306 - Attachment is obsolete: true
Flags: blocking-firefox3.6?

Comment 239

7 years ago
https://bugzilla.mozilla.org/show_bug.cgi?id=280661

Comment 240

7 years ago
https://bugzilla.mozilla.org/show_bug.cgi?id=280661

Updated

2 years ago
Flags: blocking-aviary1.0PR-
Flags: blocking-aviary1.0+
You need to log in before you can comment on or make changes to this bug.