Daniel C Reporter
	Description • 19 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.

	Michael Kristensen
	Comment 1 • 19 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

	Daniel C Reporter
	Comment 2 • 19 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.

	Noah (NoahSUMO/SolidSnake) [:Noah]
	Comment 3 • 19 years ago

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

	zug_treno
	Comment 4 • 19 years ago

Duplicate of Core bug 164008?

	Daniel C Reporter
	Comment 5 • 19 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.

	zug_treno
	Comment 6 • 19 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.

	Martijn Wargers (dead)
	Comment 7 • 19 years ago

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

	Martijn Wargers (dead)
	Comment 8 • 19 years ago

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.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 9 • 19 years ago

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

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 10 • 19 years ago

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

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 11 • 19 years ago

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.

	Martijn Wargers (dead)
	Comment 12 • 19 years ago

Maybe, but iirc it does not work that way.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 13 • 19 years ago

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.

	Martijn Wargers (dead)
	Comment 14 • 19 years ago

You're welcome to try a different/better approach.

	Simon Bünzli Assignee
	Comment 15 • 19 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

	Martijn Wargers (dead)
	Comment 16 • 19 years ago

Yes, I guess that might also work, might be even better.
Please, post a patch?

	Simon Bünzli Assignee
	Comment 17 • 19 years ago

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.

	Martijn Wargers (dead)
	Comment 18 • 19 years ago

When I press escape while the urlbar autocomplete is open, the url bar gets cleared. That doesn't happen with your patch.

	Simon Bünzli Assignee
	Comment 19 • 19 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.

	Martijn Wargers (dead)
	Comment 20 • 19 years ago

(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.

	Simon Bünzli Assignee
	Comment 21 • 19 years ago

Un-bitrotted and updated to my comment #19.

	Phil Ringnalda (:philor)
	Comment 22 • 18 years ago

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

	Ryan Flint [:rflint] (ping via IRC for reviews)
	Comment 23 • 18 years ago

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

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 24 • 18 years ago

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.

	Ria Klaassen (not reading all bugmail)
	Comment 25 • 18 years ago

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

	Martijn Wargers (dead)
	Comment 26 • 18 years ago

(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.

	Simon Bünzli Assignee
	Comment 27 • 18 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).

	Martijn Wargers (dead)
	Comment 28 • 18 years ago

(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. 

	Martijn Wargers (dead)
	Comment 29 • 18 years ago

(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.

	Simon Bünzli Assignee
	Comment 30 • 18 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.

	Mike Connor [:mconnor]
	Comment 32 • 17 years ago

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

r=me, sorry for the long long delay!

	Ryan Flint [:rflint] (ping via IRC for reviews)
	Comment 33 • 17 years ago

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

	Simon Bünzli Assignee
	Comment 34 • 17 years ago

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?

	Ryan Flint [:rflint] (ping via IRC for reviews)
	Comment 35 • 17 years ago

Whoops, thanks for catching that - checked in and bumped to rev 1.4.

	Carsten Book [:Tomcat]
	Comment 36 • 17 years ago

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3pre) Gecko/2008010705 Minefield/3.0b3pre ID:2008010705