361735 - middleclick on a search suggestion should open the result in a new tab

Status: VERIFIED FIXED

(Firefox :: Search, enhancement)

Description

[Peter van der Woude [:Peter6]]

repro:
select google or yahoo as searchengine
type any word and wait for the suggestions to appear
middleclick on an item from the list

result:
opens is same tab

expected:
open in a new tab

(maybe do the same for alt select)

Comment 1

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

We could use the same technique as the second patch in bug 295498 (probably even reuse the same binding).

Comment 2

[Simon Bünzli]

As the search box uses the same PopupAutoComplete as the urlbar, this should even have been fixed directly by bug 295498.

Comment 3

[Simon Bünzli]

Attachment: distinguish between urlbar, search box and content forms

Turns out that although we handle the search box and even content text forms, we handle them badly.

This patch makes a distinction between these three cases and handles the first two in the same way (Ctrl-click => new tab, Shift-click => new window) while again ignoring the modifier in the third case.

Comment 4

[Simon Bünzli]

Either this bug gets fixed or we'll need a new bug for the regression introduced by bug 295498 (see comment #3 above). Gavin: can you get to this before beta 1?

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

So, it does open a new tab now, but it loads the suggestion label, not a query searching for the suggestion label. That kinda morphs this bug, but it also qualifies it as a blocker! Woot!

Comment 6

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

Comment on attachment 267312 [details] [diff] [review]
distinguish between urlbar, search box and content forms

r=me, but perhaps we should consider using a different popup for the urlbar, the search bar, and the tabbrowser (content)? I guess they're largely the same, so maybe that isn't a good idea - what do you think?

Comment 7

[Simon Bünzli]

From http://lxr.mozilla.org/mozilla/search?string=PopupAutoComplete it seems that there really isn't all that much to #PopupAutoComplete, so code-wise it should be quite simple to untangle the three cases and use the current binding for #urlbar, add a new binding to search.xml for the searchbar and use the default binding for the content area. Not sure whether there are any draw-backs, though, since I don't know why the one popup has been reused so far. Follow-up bug?

Comment 8

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

(In reply to comment #7)
> Follow-up bug?

Yes, please!

Comment 9

[Simon Bünzli]

Alright, delaying the check-in, until bug 391385 has been considered.

Comment 10

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

I think we should land this patch now and back it out if we end up fixing bug  	391385.

Comment 11

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

Oh, I hadn't noticed you posting a patch there. I'll take a look.

Comment 12

[Simon Bünzli]

This issue has been fixed in bug 391385.

Comment 13

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

Patch for bug 391385 was backed out

Comment 14

[Simon Bünzli]

Right, let's fix this issue here then (the patch is pretty obvious to undo once a better solution has been found in bug 391385 or similar).

Comment 15

[Nickolay_Ponomarev]

Checking in browser/base/content/urlbarBindings.xml;
/cvsroot/mozilla/browser/base/content/urlbarBindings.xml,v  <--  urlbarBindings.xml
new revision: 1.23; previous revision: 1.22
done

Comment 16

[Nickolay_Ponomarev]

had to back this out while investigating tree orange, and couldn't check back in, because the tree was closed.

Comment 17

[Nickolay_Ponomarev]

Re-landed:

Checking in browser/base/content/urlbarBindings.xml;
/cvsroot/mozilla/browser/base/content/urlbarBindings.xml,v  <--  urlbarBindings.xml
new revision: 1.25; previous revision: 1.24
done

Comment 18

[Marcia Knous [:marcia]]

Litmus Triage Team: An owner is needed for this Litmus best case.

Comment 19

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #18)
> Litmus Triage Team: An owner is needed for this Litmus best case.

Sure :-)

http://litmus.mozilla.org/show_test.cgi?id=4671

in-litmus+

Also, verified fixed using:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007091204 Minefield/3.0a8pre 

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a8pre) Gecko/2007091204 Minefield/3.0a8pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a8pre) Gecko/2007091204 Minefield/3.0a8pre