Closed Bug 586234 Opened 10 years ago Closed 9 years ago

When middle clicking links in popups, open the new tab in a full browser window

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 4.0b7
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: alice0775, Assigned: dao)

References

()

Details

(Keywords: regression)

Attachments

(2 files, 3 obsolete files)

Attached file testcase
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre ID:20100810040309
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre ID:20100810040309

In Popup window,
The behavior of the tabstrip is different from Firefox 3.6.x.

When I open in new tab(middle-click and/or Ctrl+click link nor Alt+Enter in LocationBar) on popup window, tab strip does not appears.

I think the tabstrip should be appears and switching of tab must be possible.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with NEW Profile
2. Open testcase
3. Click link
4. Middle-click and/or Ctrl+click link nor Alt+Enter in LocationBar

Actual Results:
 Tabstrip does not appears though the new tab is opened.

Expected Results:
 Tabstrip ahould be appear as same as Firefox3.6.x.


Steps to Reproduce in real world:
1. Start Minefield with NEW Profile +  Flash Player
2. Open URL ( http://www.einslive.de/ )
3. Click "WEBRADIO" at the top o page to open popup
4. Middle-click a link(HTML-Version, etc.) at bottom of popup window
   OR KeyPress Alt+Enter in the LocationBar

Actual Results:
 Tabstrip does not appears though the new tab is opened.
 You can confirm when click X button of window control.

Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/528a9b4f475d
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100317 Minefield/3.7a4pre ID:20100317002903
Fails:
http://hg.mozilla.org/mozilla-central/rev/11798edae90d
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100317 Minefield/3.7a4pre ID:20100317003949
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=528a9b4f475d&tochange=11798edae90d
Candidate Bug:
Bug 347930 - Tab strip should be a toolbar instead
This is a big loss of functionality. Asking for blocking2.0.
blocking2.0: --- → ?
OS: Windows 7 → All
Hardware: x86 → All
Target Milestone: Firefox 4.0 → ---
Duplicate of this bug: 584670
Duplicate of this bug: 589539
We should actually disable the function to force a link from a popup into a new tab, instead. I don't think showing a tabstrip is a great UX, either. Team UX: agree?

Not sure that this blocks, to be honest. Not great, but this should not be expected functionality.
blocking2.0: ? → -
Keywords: polish, uiwanted
Duplicate of this bug: 589029
(In reply to comment #4)
> We should actually disable the function to force a link from a popup into a new
> tab, instead. I don't think showing a tabstrip is a great UX, either. Team UX:
> agree?

If I understand this correctly: Are you asking what should happen if you cmd-click or middle-click a link that results in a pop-up? It should just work as a normal pop-up in my opinion, but my understanding of the description of the problem might be wrong.

(in several other locations we let middle-click/cmd-click fall back to normal click if we can't do what's being asked for, or if the middle-click has undefined behavior (e.g. mid-clicking the back button)
(In reply to comment #6)
> If I understand this correctly: Are you asking what should happen if you
> cmd-click or middle-click a link that results in a pop-up?

No - the question is what should happen when someone is *in* a popup that has no tab bar (e.g. the one opened by clicking the link in the testcase), and then requests a new tab using e.g. middle click.
(In reply to comment #7)
> No - the question is what should happen when someone is *in* a popup that has
> no tab bar (e.g. the one opened by clicking the link in the testcase), and then
> requests a new tab using e.g. middle click.

Ah, thanks. Yeah, showing a tabstrip in the tiny pop-up window isn't going to be helpful. I'd redirect it to the underlying window if possible — if not, maybe disable it like beltzner suggests.
Assignee: nobody → dao
Keywords: uiwanted
(In reply to comment #7)
> (In reply to comment #6)
> > If I understand this correctly: Are you asking what should happen if you
> > cmd-click or middle-click a link that results in a pop-up?
> 
> No - the question is what should happen when someone is *in* a popup that has
> no tab bar (e.g. the one opened by clicking the link in the testcase), and then
> requests a new tab using e.g. middle click.

This may sound dumb,but wouldn't the behavior of Firefox 3.6 be adequate,i.e. the requested new tab opens in the popup,alongside the existing tab and each of these tabs (in the popup) can be closed independently from the others,or the popup as a whole can be closed like a normal window.
(In reply to comment #9)
> This may sound dumb,but wouldn't the behavior of Firefox 3.6 be adequate,i.e.
> the requested new tab opens in the popup,alongside the existing tab and each of
> these tabs (in the popup) can be closed independently from the others,or the
> popup as a whole can be closed like a normal window.

No, it's a fair question. The problem is that these windows are often weird (since they can disable the toolbar, turn off resizing, etc).

Ideally, we would reset these properties if another tab was trying to open in that window, so you got a normal window from there on. If we can't do that, it's better to redirect the request to the underlying window.
Attached patch patch (obsolete) — Splinter Review
Writing this was an interesting journey, although I repeatedly regretted taking this bug...

This gets rid of some our few openNewTabWith and openNewWindowWith callers, which I believe is a win. openNewTabWith in particular doesn't look like it belongs in utilityOverlay.js at all, as it breaks when called from a non-browser window. I'll look into cleaning this up in a followup.

Another side effect of this patch is that it kills the exotic "force feed preview page" feature, which seems to be neither advertised nor discoverable nor useful.
Attachment #469865 - Flags: review?(gavin.sharp)
Attached patch patch v2 (obsolete) — Splinter Review
discovered a bug upon reviewing my own code
Attachment #469865 - Attachment is obsolete: true
Attachment #469869 - Flags: review?(gavin.sharp)
Attachment #469865 - Flags: review?(gavin.sharp)
(In reply to comment #12)
> Created attachment 469869 [details] [diff] [review]
> patch v2
> 
> discovered a bug upon reviewing my own code

something wrong in the patch.
it will load a link in current tab when right click on a link.
Attached patch patch v3 (obsolete) — Splinter Review
(In reply to comment #13)
> something wrong in the patch.
> it will load a link in current tab when right click on a link.

Thanks, this should fix it.

I also restored handleLinkClick's return value since mak thinks it's useful.
Attachment #469869 - Attachment is obsolete: true
Attachment #469955 - Flags: review?(gavin.sharp)
Attachment #469869 - Flags: review?(gavin.sharp)
Component: Tabbed Browser → General
QA Contact: tabbed.browser → general
Duplicate of this bug: 593610
As you can read in the description of the above duplicate bug, this only happens when "toolbar" is "no" (window.open, third argument is a string containing "toolbar=no".
Duplicate of this bug: 578013
(In reply to comment #10)
> (In reply to comment #9)
> > This may sound dumb,but wouldn't the behavior of Firefox 3.6 be adequate,i.e.
> > the requested new tab opens in the popup,alongside the existing tab and each of
> > these tabs (in the popup) can be closed independently from the others,or the
> > popup as a whole can be closed like a normal window.
> 
> No, it's a fair question. The problem is that these windows are often weird
> (since they can disable the toolbar, turn off resizing, etc).
> 
> Ideally, we would reset these properties if another tab was trying to open in
> that window, so you got a normal window from there on. If we can't do that,
> it's better to redirect the request to the underlying window.

Actually, I would expect the window to stay as it was (i.e. non-resizable, no toolbars). But maybe it's just me...

I think it would be pretty neat if middle-clicks opened new tabs in parent window by default, but it should be possible to drag a tab to a pop-up window and have it there explicitly.

By the way, what will happen if the parent window is closed?
(In reply to comment #18)
> By the way, what will happen if the parent window is closed?

A new window will be opened.
Also, I think this should block. It would be okay if middle click did nothing in popups, but the way this currently fails doesn't seem acceptable...
blocking2.0: - → ?
Sneaky. Block a blocker with it, eh Dave? This now blocks due to the dataloss case thanks to the new EM, ew.
blocking2.0: ? → betaN+
Bug 593687 may need a separate fix, though one that follows the one here.
Attached patch patch v3Splinter Review
updated to tip
Attachment #469955 - Attachment is obsolete: true
Attachment #483721 - Flags: review?(gavin.sharp)
Attachment #469955 - Flags: review?(gavin.sharp)
Keywords: polish
Comment on attachment 483721 [details] [diff] [review]
patch v3

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js

>         let tab = win.gBrowser.loadOneTab(aURI ? aURI.spec : "about:blank", {
>                                           referrerURI: referrer,
>+                                          relatedToCurrent: !needToFocusWin,
>                                           fromExternal: isExternal,
>                                           inBackground: loadInBackground});

Why this change? I don't see why tabs opened from this path would be "related to current" (or why we'd want them to have that behavior), regardless of which window (current or other) they're opening in... r=me with it removed.
Attachment #483721 - Flags: review?(gavin.sharp) → review+
We need some additional tests for openUILink, and contentAreaClick/handleLinkClick in general. Looks like bug 549340 includes some.
I tried the test in bug 549340, it times out early in waitForFocus...
Blocks: 606678
http://hg.mozilla.org/mozilla-central/rev/ec6b12294371
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Summary: When I open in new tab(middle-click and/or Ctrl+click link nor Alt+Enter in LocationBar) on popup window, tab strip does not appears though the new tab is opened. → When middle clicking links in popups, open the new tab in a full browser window
Target Milestone: --- → Firefox 4.0b8
Blocks: 606691
Flags: in-testsuite?
As a followup, maybe we can move "open in sidebar" from PlacesUIUtils.openNodeIn to whereToOpenLink and openLinkIn? We may add a new value "sidebar" for where.
Target Milestone: Firefox 4.0b8 → Firefox 4.0b7
Alice, are all cases fixed for you in the latest nightly build?
(In reply to comment #29)
> Alice, are all cases fixed for you in the latest nightly build?
No, not fixed
Alt+Enter in LocationBar makes hidden tab
(In reply to comment #30)
> No, not fixed
> Alt+Enter in LocationBar makes hidden tab

I filed bug 610203 for that.
(In reply to comment #30)
> (In reply to comment #29)
> > Alice, are all cases fixed for you in the latest nightly build?
> No, not fixed

I tried the testcase attached to this bug with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101107 Firefox/4.0b8pre ID:20101107030911 and the same version on Windows.

Doing a right click on the link in the popup and open it as new tab still doesn't open the tab in the browser window but opens it as a hidden tab inside the popup. so I wonder what has been really fixed by the attached patch.
(In reply to comment #32)
> Doing a right click on the link in the popup and open it as new tab still
> doesn't open the tab in the browser window but opens it as a hidden tab inside
> the popup.

bug 606678
I see (where == "current") returns from handleLinkClick. Shouldn't it be (event.button == 0 && !event.ctrlKey && !event.shiftKey &&
!event.altKey && !event.metaKey) at the very beginning?
As expressed in various bug reports (such as 626365) the old behavior (as in FF3.6.x) is actually preferred by many users. As a UX professional I quite disagree that the tab strip in the pop-up amounts to bad UX, especially when considering the current behavior of opening the new tab in a window that does not have focus or may no longer exist. There are several cases where the FF3.6.x behavior makes perfect sense and is indeed and without doubt the expected behavior.
I find the reasons given here rather weak and it seems as it was more a decision by developers to drop functionality so that other issues did not need to get fixed. IMHO it entirely goes against end-user preferences.

Unfortunately, any new bugs in this regard get closed as duplicates or invalid. Please advise on how to proceed to get this removed or remodeled to become configurable. I would be fine with keeping it the way it is as default, but have an option in about:config to change back to old behavior.
Blocks: 561482
No longer blocks: 561482
Blocks: 561482
You need to log in before you can comment on or make changes to this bug.