279687 - Should be able to open new tab by middle-clicking "Go" button

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, enhancement, P5)

Description

[Corey Farwell]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 StumbleUpon/1.999
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 StumbleUpon/1.999

You should be able to open a new tab and going to the URL on the address bar and
pressing on the go button with the middle key on the mouse.

Reproducible: Always

Steps to Reproduce:
1. type in url
2. press middle key on go button
3. nothing opens

Actual Results:  
nothing opens

Expected Results:  
opened a new webpage thats in the address bar in a new tab

Comment 1

[Jo Hermans]

related: bug 263942

Comment 2

[Nickolay_Ponomarev]

couldn't find duplicates, confirming

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch

Comment 4

[Jaime Mitchell (use [e-mail] for email)]

*** Bug 295501 has been marked as a duplicate of this bug. ***

Comment 5

[Daniel C]

Thanks Jaime, me being stupid again.

Related bugs include 295498 and 227826.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 183688 [details] [diff] [review]
Patch

This is rotten.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 202932 [details] [diff] [review]
patch


>-  if (gBrowser.localName == "tabbrowser" &&
>-      aTriggeringEvent && 'altKey' in aTriggeringEvent &&
>-      aTriggeringEvent.altKey) {
>+  var newTab = aTriggeringEvent && (aTriggeringEvent.altKey ||
>+                                    aTriggeringEvent.button == 1)

this is wrong (keeps our bogus Alt-Click for new tab on the Go button, which is pretty inconsistent).  This also doesn't properly support shift modifiers like other UI bits.  I think the best thing to do is implement an onclick handler for the Go button along the lines of the one implemented for the home button, using whereToOpenLink to determine behaviour, and leave this function as-is for the url bar.

Just to be clear, consistent and correct behaviour would be:

"tab" == open foreground
"tabshifted" == open background
"window" == open in new window
"current" == open in current
"save" == save the URL

>-  }
>-  else  
>+  } else {
>     loadURI(url, null, aPostData);
>+  }

ugh.  That is all.

Comment 9

[Phil Ringnalda (:philor)]

*** Bug 333269 has been marked as a duplicate of this bug. ***

Comment 10

[Brandon]

In 2.0a2 it appears that the middle-click event is being captured and opens a new tab. Unfortunately, the new tab is load the currently opened page rather than the URL in the address bar.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch v2

This does two things:
1) Makes on the Go button use openUILink, by changing it to a click handler (middle click doesn't trigger oncommand) and ignoring right clicks (button==2).
2) Makes openUILink and openUILink in not treat "tabshifted" as "tab". This means that a lot of existing callers, like the context menu's "View Image" and the Back/Forward buttons and a bunch of others properly obey the Shift key for "new tab" operations (Ctrl+click or middle click). I don't really know why that wasn't the case before, and I don't really know if that is desired or expected behavior.

Example: middle click on back used to always use either a background tab or a foreground tab, depending on browser.tabs.loadInBackground. With this patch, it uses browser.tabs.loadInBackground, unless the shift key is held down when you middle click or ctrl click, in which case it does the opposite.

Comment 12

[Mike Connor [:mconnor]]

Comment on attachment 223567 [details] [diff] [review]
patch v2

r+ui-r+a=me

Comment 13

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/base/content/browser.xul 	1.268.2.41
mozilla/browser/base/content/utilityOverlay.js 	1.32.2.6
mozilla/browser/base/content/utilityOverlay.js 	1.36
mozilla/browser/base/content/browser.xul 	1.305

Comment 14

[Brandon]

I'm still having one issue with this new fix.  The new fix still does not work for keywords.  So if I type a keyword into the url and then middle-click go, I get www.keyword.com rather than what I expect.

Also completely ignore comment #10, I was unaware at the time that TabMix had implemented their own action for middle-clicking go to be duplicate tab.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #14)
> I'm still having one issue with this new fix.  The new fix still does not work
> for keywords.

That's bug 339710.