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

RESOLVED FIXED in Firefox 4.0b7

Status

()

Firefox
General
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: Alice0775 White, Assigned: dao)

Tracking

({regression})

Trunk
Firefox 4.0b7
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

7 years ago
Created attachment 464766 [details]
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
(Assignee)

Updated

7 years ago
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)

Updated

7 years ago
Assignee: nobody → dao
Keywords: uiwanted

Comment 9

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

Comment 11

7 years ago
Created attachment 469865 [details] [diff] [review]
patch

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)
(Assignee)

Comment 12

7 years ago
Created attachment 469869 [details] [diff] [review]
patch v2

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)
(Reporter)

Comment 13

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

Comment 14

7 years ago
Created attachment 469955 [details] [diff] [review]
patch v3

(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)
(Assignee)

Updated

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

Comment 18

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

Comment 19

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

A new window will be opened.
(Assignee)

Comment 20

7 years ago
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: - → ?
Blocks: 593687
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+
(Assignee)

Comment 22

7 years ago
Bug 593687 may need a separate fix, though one that follows the one here.
(Assignee)

Comment 23

7 years ago
Created attachment 483721 [details] [diff] [review]
patch v3

updated to tip
Attachment #469955 - Attachment is obsolete: true
Attachment #483721 - Flags: review?(gavin.sharp)
Attachment #469955 - Flags: review?(gavin.sharp)
(Assignee)

Updated

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

Comment 26

7 years ago
I tried the test in bug 549340, it times out early in waitForFocus...
(Assignee)

Updated

7 years ago
Blocks: 606678
(Assignee)

Comment 27

7 years ago
http://hg.mozilla.org/mozilla-central/rev/ec6b12294371
Status: NEW → RESOLVED
Last Resolved: 7 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
(Reporter)

Updated

7 years ago
Blocks: 606691
Flags: in-testsuite?

Comment 28

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

Updated

7 years ago
Target Milestone: Firefox 4.0b8 → Firefox 4.0b7
Alice, are all cases fixed for you in the latest nightly build?
(Reporter)

Comment 30

7 years ago
(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.
Blocks: 610203
(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.
(Assignee)

Comment 33

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

Comment 34

7 years ago
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?

Comment 35

6 years ago
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.

Updated

5 years ago
Blocks: 561482
(Assignee)

Updated

5 years ago
No longer blocks: 561482

Updated

5 years ago
Blocks: 561482
You need to log in before you can comment on or make changes to this bug.