Closed Bug 361735 Opened 13 years ago Closed 12 years ago

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

Categories

(Firefox :: Search, enhancement)

enhancement
Not set

Tracking

()

VERIFIED FIXED
Firefox 3 alpha8

People

(Reporter: Peter6, Assigned: zeniko)

References

Details

(Whiteboard: bugmorph)

Attachments

(1 file)

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)
We could use the same technique as the second patch in bug 295498 (probably even reuse the same binding).
Depends on: 295498
Hardware: PC → All
As the search box uses the same PopupAutoComplete as the urlbar, this should even have been fixed directly by bug 295498.
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.
Assignee: nobody → zeniko
Status: NEW → ASSIGNED
Attachment #267312 - Flags: review?(gavin.sharp)
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?
Flags: blocking-firefox3?
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!
Flags: blocking-firefox3? → blocking-firefox3+
Whiteboard: bugmorph
Target Milestone: --- → Firefox 3 M8
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?
Attachment #267312 - Flags: review?(gavin.sharp) → review+
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?
Keywords: checkin-needed
(In reply to comment #7)
> Follow-up bug?

Yes, please!
Blocks: 391385
Alright, delaying the check-in, until bug 391385 has been considered.
Keywords: checkin-needed
I think we should land this patch now and back it out if we end up fixing bug  	391385.
Oh, I hadn't noticed you posting a patch there. I'll take a look.
This issue has been fixed in bug 391385.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Patch for bug 391385 was backed out
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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).
Keywords: checkin-needed
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
Status: REOPENED → RESOLVED
Closed: 13 years ago12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
had to back this out while investigating tree orange, and couldn't check back in, because the tree was closed.
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
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
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Flags: in-litmus?
Litmus Triage Team: An owner is needed for this Litmus best case.
(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
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.