Closed Bug 377551 Opened 16 years ago Closed 16 years ago

Pages opened in new tabs should skip chrome-less windows (like popups)


(Firefox :: Tabbed Browser, defect, P4)




Firefox 3 beta3


(Reporter: warp-bugzilla, Assigned: florian)





(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en) AppleWebKit/419 (KHTML, like Gecko) Safari/419.3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20070309 Firefox/

The "New pages should be opened in new tabs" should be more intelligent: If more than one window is open, it should not open the new tab in the current/last opened window if it's a window without browser chrome (e.g. popups without menu bars etc.) but instead open the page in one of the windows which have full browser chrome.

Reproducible: Always

Steps to Reproduce:
I don't think it's actually a dupe even if it's related. Bug 243893 is clearly a bug reporting concerning the tab bar which was resolved and fixed, but the issue I'm talking here about is the general usability issue caused by the behavior of re-using popup windows.

The behavior should be fixed in my opinion. I think Mike Beltzer talked in Bug 243893 about it in comment 26 - this bug report is the same as use case 2 in his comment, but I cannot re-open the bug and this gets reported once a week by users of our site which uses popup windows for legitimate reasons: People can open chat windows in new popups on demand (and they do not want to have the browser chrome there) but they expect the target="_blank" links in the chat windows to be opened in a true browser window and I do not want to have to abuse JavaScript just to force a browser to open a simple link in a true browser window with chrome.

If you won't fix the usability issue, simply tell me that and I will communicate this to our users, but handling this report as a dupe is not enough - this issue simply was not really discussed in 243893.
Confirming as a valid RFE and updating summary a bit.
Ever confirmed: true
Hardware: PC → All
Summary: New pages should be opened in new tabs should skip popups → Pages opened in new tabs should skip chrome-less windows (like popups)
Version: unspecified → Trunk
See bug 391778 for an example of where Firefox's (default!) behavior completely breaks a site.  A small popup on Sony's site tries to open a large (unsized) popup, but users end up with an additional tab in a tiny, non-resizable window.

Combined with the change between Firefox 1.0 and Firefox 2.0 to make "New pages should be opened in: a new tab" the default, this is a serious usability regression for some sites.
Severity: enhancement → major
Flags: blocking-firefox3?
Thank you gentlemen and scholars!  Happy I helped the Mozilla-Firefox cause. (I guess I'll have to make a PayPal contribution, now)
We need to fix the remaining cases of this, I thought I had this fixed, but for Mac at least there's issues.
Assignee: nobody → mconnor
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: --- → Firefox 3 M9
moving out bugs that don't need to block b1
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Attached patch work in progress (obsolete) — Splinter Review
* The openDialog part doesn't work yet.

* I'm not sure the window.toolbar.visible test is acceptable to ensure we are not on a chrome-less window.
Attached patch patch v2 (obsolete) — Splinter Review
Assignee: mconnor → f.qu
Attachment #286637 - Attachment is obsolete: true
Attachment #286733 - Flags: review?(mano)
Priority: -- → P5
Priority: P5 → P4
Target Milestone: Firefox 3 Mx → Firefox 3 M11
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Comment on attachment 286733 [details] [diff] [review]
patch v2

This patch no loner applies.

Index: browser/base/content/browser.js
RCS file: /cvsroot/mozilla/browser/base/content/browser.js,v
retrieving revision 1.877
diff -u -U 8 -r1.877 browser.js
--- browser/base/content/browser.js	25 Oct 2007 23:02:20 -0000	1.877
+++ browser/base/content/browser.js	30 Oct 2007 20:27:27 -0000
@@ -3960,20 +3960,44 @@
     var url = aURI ? aURI.spec : "about:blank";
     switch(aWhere) {
       case Ci.nsIBrowserDOMWindow.OPEN_NEWWINDOW :
         newWindow = openDialog(getBrowserURL(), "_blank", "all,dialog=no", url);
       case Ci.nsIBrowserDOMWindow.OPEN_NEWTAB :
         var loadInBackground = gPrefService.getBoolPref("browser.tabs.loadDivertedInBackground");
-        var newTab = gBrowser.loadOneTab("about:blank", null, null, null, loadInBackground, false);
-        newWindow = gBrowser.getBrowserForTab(newTab).docShell
-                            .QueryInterface(Ci.nsIInterfaceRequestor)
-                            .getInterface(Ci.nsIDOMWindow);
+        if (window.toolbar.visible) {


+          // open a new tab in the same window.
+          var newTab = gBrowser.loadOneTab("about:blank", null, null, null, loadInBackground, false);
+          newWindow = gBrowser.getBrowserForTab(newTab).docShell
+                              .QueryInterface(Ci.nsIInterfaceRequestor)
+                              .getInterface(Ci.nsIDOMWindow);
+        }
+        else {
+          // we are in a chrome-less window, look for another window we can use.
+          var wm = Components.classes[";1"]
+                             .getService(Components.interfaces.nsIWindowMediator);
+          var windows = wm.getZOrderDOMWindowEnumerator("navigator:browser", true);

I think getZOrderDOMWindowEnumerator is still broken on linux.
Attachment #286733 - Flags: review?(mano) → review-
(In reply to comment #12)
> I think getZOrderDOMWindowEnumerator is still broken on linux.

Indeed. See bug 302281 comment 5 and bug 156333.
Whiteboard: [needs new patch]
Attached patch patch v3Splinter Review
This duplicates the ugly code of getMostRecentBrowserWindow from nsBrowserContentHandler.js
Attachment #286733 - Attachment is obsolete: true
Attachment #293953 - Flags: review?(mano)
Whiteboard: [needs new patch] → [needs review mano]
I think it's worth checking the most-common case above the #ifdef block in which |window| just fits the criteria, and only go through the most-recent-loop in all other edge cases....
Attachment #293953 - Flags: review?(mano) → review-
Comment on attachment 293953 [details] [diff] [review]
patch v3

+    if (!window.document.documentElement.getAttribute("chromehidden"))
+      return window;

Somehow i missed that, r=mano
Attachment #293953 - Flags: review- → review+
Severity: major → normal
Whiteboard: [needs review mano]
Attachment #293953 - Flags: approval1.9?
Attachment #293953 - Flags: approval1.9? → approval1.9+
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.926; previous revision: 1.925
Closed: 16 years ago
Resolution: --- → FIXED
I'm pretty sure this has busted target="_blank" links and links from external applications, but not middle-clicks, for me.  They now open up new tabs that remain empty, and then open a new window, too, and the content loads in the new window.
I spoke too soon - it was just a conflict with an add-on. (server switcher 0.4, if you care to know).
Depends on: 439054
Did seems to have caused bug 439054 which ruins the experience on several intranet applications we developed. 
I see more than one issue (with bug 439054): 

* Some like to have _blank targets opened in a new popup
* Some like to have _blank targets opened in a new full-size window, not necessarily a popup
* Some like to have _blank targets opened in the same frame (popup/browser window), but preferably as a tab.

Tab-browsing is something that adds to pop-ups and non-pop-ups, and has to be handled independent of opening a new document, may it be a link or a js-inited window. 
Would it be possible to add a new switch to the method to specify some of this wishes? Tabbed browsing is wide-spread nowadays, why not add control for it?
Patrick, see bug 105409.
I understand that adding new features is a hairy thing and needs a lot of consideration.

The resolution to this bug changes the user experience very heavily. It changed the behaviour on the client/user side. The old behaviour was useful like is the new one. But it is hardcoded, and neither the coder nor the user can change it. I understand that adding an option for the coder would be problematic. I don't see much danger in allowing a coder to hint for opening in the same context, though. But at least the user should be able to choose the behaviour.

I really would suggest to have an option to toggle between the new and the old behaviour. I'd suggest to add an option likewise to 'When I open a link in a new tab, switch to it immediately' in the Tabs Options page.

The preference could be extended to a 4th value, enabling the new behaviour.

See (description is not valid any more with this bug resolution!)
After reviewing some bugs and thinking more about tab and popup-behavior...

This resolution disables tabs in popups completely. This is a bad regression.
Moreover there's no need to configure this:
* Opening external links in a full-chrome window is a good thing. Full-screen popups that look like full-chrome windows are bad. Opening a link in such a window in disguise should not be fostered.
* A user wanting a new window instead of a tab (comment 6) needs no special precautions: The context menu has "Open link in new window". No need for "Open link in a tab in a full-chrome window".
* Having tabs in popups and showing a tab bar is a good thing to keep context for web apps. See bug 439054 comment 11.

I think the resolution to this bug should be changed to only affect external links. These get focus after loading, so they do not get lost if they end up in a different window. Other links should not be diverted.
This behavior messed with some software we developped. Please, come back to Firefox 2 behavior. ;)
You need to log in before you can comment on or make changes to this bug.