Middle-clicking autocomplete entries opens the URI in the same tab instead of a new one

VERIFIED FIXED in Firefox 3 alpha6

Status

()

Firefox
Location Bar
--
enhancement
VERIFIED FIXED
12 years ago
8 years ago

People

(Reporter: Daniel Cater, Assigned: Simon Bünzli)

Tracking

Trunk
Firefox 3 alpha6
Points:
---
Bug Flags:
blocking-firefox3 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
Latest-trunk

When typing a URI in the location bar that has already been visited, a dropdown
list for autocompletion appears. Activating them loads the URI. Since
middle-clicking is associated with new tabs in most of the browser (home button,
links, View Image, back button, forward button etc.) it would be great if you
could middle-click an autocomplete entry and have it load up in a new tab.

Reprodicble: Always

Steps to Reproduce:

1. Ctrl + L.
2. Start typing the beginning of a URL you have previously visited (past the
http://www.) - Autocomplete list should appear.
3. Move the mouse over one of the entries.
4. Middle-click it.

Actual Results:

URL loaded in the same tab.

Expected Results:

URL should have loaded in a new tab.

Note: After typing a URL, Alt + Enter will open it in a new tab. The tab it was
typed in still holds the URL in the location bar.

Comment 1

12 years ago
I agree with you...

BTW: you shouldn't mention more than one bug in a single bug report, and the bug
you mentioned at the end of you report is already here: bug 227826
(Reporter)

Comment 2

12 years ago
I just mentioned it so that anyone who intends on making a patch might take it
into account if they want to. Also to demonstarte that Alt + Enter performs a
similar behaviour to what this bug requires, so that that code might be looked
at when trying to fix this bug. Thanks for the bug number.

I've also filed bug 295501 on middle-clicking the Go button and noted this bug
and bug 227826 in it.
Yes, my idea is coming to life. MUHAHAHAHA.

Thanks for filing it for me, DJCater.

I had it jotted down in notebook since last Fall. But I thought it was already
recommended so rather than making a dupe and wiping the shell from my face after
somebody would add me as a "Duplicate of Bug ******", I thought I'd save myself
the embarrassment. Any way sorry for chiming in like that.

Carry on. :D

Comment 4

12 years ago
Duplicate of Core bug 164008?
(Reporter)

Comment 5

12 years ago
Possibly, but that bug mentions that 'nothing' happens when middle-clicking,
whereas this one states that the URL is loaded in the current tab.

If you dupe this bug to that one be sure to update the summary to include the
word 'autocomplete' and move it to Firefox -> Location Bar and Autocomplete as
it's not a bug with Core -> Tabbed Browser.

Comment 6

12 years ago
(In reply to comment #5)
> Possibly, but that bug mentions that 'nothing' happens when middle-clicking,
> whereas this one states that the URL is loaded in the current tab.

But its duplicates bug 257601 and bug 277700 do mention 'loading in the current
tab'.
 
> If you dupe this bug to that one be sure to update the summary to include the
> word 'autocomplete' and move it to Firefox -> Location Bar and Autocomplete as
> it's not a bug with Core -> Tabbed Browser.
 
I don't have the priviliges to change those things, somebody else would have to
do that.
See:
<method name="onPopupClick">
http://lxr.mozilla.org/seamonkey/source/toolkit/content/widgets/autocomplete.xml#697
The handleenter function can be found here:
http://lxr.mozilla.org/seamonkey/source/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp#257
Eventually that fires an ontextentered function, then this is fired:
http://lxr.mozilla.org/seamonkey/source/toolkit/content/widgets/autocomplete.xml#204
BrowserLoadURL:
http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser.xul#209
You can find that function here:
http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser.js#1699
Created attachment 192813 [details] [diff] [review]
patch

Ok, this makes it work.
I would think that a dispatchevent of the enter key also would (have to) work,
but it doesn't seem like it.
Attachment #192813 - Flags: review?(mconnor)

Updated

12 years ago
Status: NEW → ASSIGNED

Updated

12 years ago
Assignee: nobody → martijn.martijn
Status: ASSIGNED → NEW
*** Bug 257601 has been marked as a duplicate of this bug. ***
*** Bug 277700 has been marked as a duplicate of this bug. ***
Comment on attachment 192813 [details] [diff] [review]
patch

The event is just passed to the "textentered" observer, (in this case, handleURLBarCommand) right? In that case, my patch in bug 279687 should fix this, by making BrowserLoadURL (called from handleURLBarCommand) notice middle clicks, without the need for a synthetic event.
Maybe, but iirc it does not work that way.
You're right, I looked into this a bit but forgot to comment. I'm not sure that relying on the fact that this.input reacts to an event with altKey=true is a good thing... that could theoretically change in the future. Another solution would be to set up an nsIObserver and have the autocomplete widget send out a notice for middle click, though that is a bit more involved... maybe this is "good enough", or maybe a solution like the one in comment 11 is possible.
You're welcome to try a different/better approach.
(Assignee)

Comment 15

12 years ago
What about overwriting onPopupClick for the address bar in a new subclassed binding and call openUILink from that one? This would also allow for shift+click and ctrl+click to work properly.

Until this is fixed, a patch in form of an extension is available at
http://forums.mozillazine.org/viewtopic.php?p=2117740#2117740
Yes, I guess that might also work, might be even better.
Please, post a patch?
(Assignee)

Comment 17

12 years ago
Created attachment 214245 [details] [diff] [review]
subclass the binding for #PopupAutoComplete

This patch changes the onPopupClick handler just for #urlbar's popup menu, where we can be sure that openUILink exists (which nicely handles middle-clicking and ctrl+/shift+clicking). Not sure whether there are any accessibility rules I'm violating here, though, because I close the popup through controller.handleEscape.
Attachment #214245 - Flags: review?(mconnor)
When I press escape while the urlbar autocomplete is open, the url bar gets cleared. That doesn't happen with your patch.
(Assignee)

Comment 19

12 years ago
(In reply to comment #18)
> When I press escape while the urlbar autocomplete is open, the url bar gets
> cleared. That doesn't happen with your patch.

Not sure I correctly understand you: This shouldn't happen without my patch either. Escape either closes the popup or clears the address bar, but not both at the same time.

What currently happens however is that if a link is not opened in the current tab, the address bar for that tab will display what you typed and not the actual URL. This can be fixed by calling controller.handleEscape a second time (or by calling handleURLBarRevert directly instead) before calling openUILink.
(In reply to comment #19)
> Not sure I correctly understand you: This shouldn't happen without my patch
> either. Escape either closes the popup or clears the address bar, but not both
> at the same time.
Huh? For me, it does both at the same time.
(Assignee)

Comment 21

12 years ago
Created attachment 214681 [details] [diff] [review]
subclass the binding for #PopupAutoComplete (take 2)

Un-bitrotted and updated to my comment #19.
Attachment #214245 - Attachment is obsolete: true
Attachment #214681 - Flags: review?(mconnor)
Attachment #214245 - Flags: review?(mconnor)
*** Bug 337860 has been marked as a duplicate of this bug. ***
*** Bug 354496 has been marked as a duplicate of this bug. ***
Blocks: 361735
Comment on attachment 192813 [details] [diff] [review]
patch

This isn't right per comment 13, and the second patch looks like more what we want.
Attachment #192813 - Flags: review?(mconnor) → review-
*** Bug 362341 has been marked as a duplicate of this bug. ***
(In reply to comment #24)
> (From update of attachment 192813 [details] [diff] [review] [edit])
> This isn't right per comment 13, and the second patch looks like more what we
> want.

I'm not sure I agree with comment 13. I mean it's talking about a theoretical situation that could change.
Simon's patch seems needlessly complicated to me.

(Assignee)

Comment 27

11 years ago
(In reply to comment #26)
> Simon's patch seems needlessly complicated to me.

My patch is slightly more complicated because it also allows Ctrl+clicking and Shift+clicking (for those among us with only two mouse buttons).

In addition to that, my patch is IMO also more correct by virtue of limiting itself to the address bar (middle-clicking doesn't necessarily equal Alt+Enter for _all_ autocomplete fields).
(In reply to comment #27)

> My patch is slightly more complicated because it also allows Ctrl+clicking and
> Shift+clicking (for those among us with only two mouse buttons).

Ah, ok.

> In addition to that, my patch is IMO also more correct by virtue of limiting
> itself to the address bar (middle-clicking doesn't necessarily equal Alt+Enter
> for _all_ autocomplete fields).

Hmm, is that more correct? I would think that is a disadvantage. 

(In reply to comment #28)
> Hmm, is that more correct? I would think that is a disadvantage. 

Never mind about this, I don't think there is something useful about middle-clicking on an autocomplete entry, anyway.
(Assignee)

Comment 30

11 years ago
Comment on attachment 214681 [details] [diff] [review]
subclass the binding for #PopupAutoComplete (take 2)

Gavin: That's the last of my bugs I'm moving review requests from Mike Connor to you and the first one to delay if you currently haven't got that many cycles left... anyway, TIA for the reviews.
Attachment #214681 - Flags: review?(mconnor) → review?(gavin.sharp)

Updated

10 years ago
Flags: blocking-firefox3?
Flags: blocking-firefox3? → blocking-firefox3+

Updated

10 years ago
Duplicate of this bug: 375690
Comment on attachment 214681 [details] [diff] [review]
subclass the binding for #PopupAutoComplete (take 2)

r=me, sorry for the long long delay!
Attachment #214681 - Flags: review?(gavin.sharp) → review+
(Assignee)

Updated

10 years ago
Whiteboard: [checkin needed]

Updated

10 years ago
Assignee: martijn.martijn → zeniko

Updated

10 years ago
Target Milestone: --- → Firefox 3 alpha6
Created attachment 267236 [details] [diff] [review]
As checked in

Checking in jar.mn;
/cvsroot/mozilla/browser/base/jar.mn,v  <--  jar.mn
new revision: 1.113; previous revision: 1.112
done
Checking in content/browser.css;
/cvsroot/mozilla/browser/base/content/browser.css,v  <--  browser.css
new revision: 1.25; previous revision: 1.24
done
Checking in content/urlbarBindings.xml;
/cvsroot/mozilla/browser/base/content/urlbarBindings.xml,v  <--  urlbarBindings.xml
new revision: 1.3; previous revision: 1.2
done
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
(Assignee)

Comment 34

10 years ago
Created attachment 267243 [details] [diff] [review]
encoding fix

Thanks Ryan, also for correcting my jar.mn omission.

Unfortunately somewhere in the process the patch's encoding got changed (latin-1 instead of UTF-8). Could you please correct that as well?
(Assignee)

Updated

10 years ago
Whiteboard: [checkin needed]
Whoops, thanks for catching that - checked in and bumped to rev 1.4.
Whiteboard: [checkin needed]
Flags: in-testsuite?
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3pre) Gecko/2008010705 Minefield/3.0b3pre ID:2008010705
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.