Closed
Bug 361735
Opened 15 years ago
Closed 14 years ago
middleclick on a search suggestion should open the result in a new tab
Categories
(Firefox :: Search, enhancement)
Firefox
Search
Tracking
()
VERIFIED
FIXED
Firefox 3 alpha8
People
(Reporter: Peter6, Assigned: zeniko)
References
Details
(Whiteboard: bugmorph)
Attachments
(1 file)
2.34 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
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•15 years ago
|
||
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
Assignee | ||
Comment 2•14 years ago
|
||
As the search box uses the same PopupAutoComplete as the urlbar, this should even have been fixed directly by bug 295498.
Assignee | ||
Comment 3•14 years ago
|
||
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 | ||
Comment 4•14 years ago
|
||
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?
Comment 5•14 years ago
|
||
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
Updated•14 years ago
|
Target Milestone: --- → Firefox 3 M8
Comment 6•14 years ago
|
||
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+
Assignee | ||
Comment 7•14 years ago
|
||
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
Comment 8•14 years ago
|
||
(In reply to comment #7) > Follow-up bug? Yes, please!
Assignee | ||
Comment 9•14 years ago
|
||
Alright, delaying the check-in, until bug 391385 has been considered.
Keywords: checkin-needed
Comment 10•14 years ago
|
||
I think we should land this patch now and back it out if we end up fixing bug 391385.
Comment 11•14 years ago
|
||
Oh, I hadn't noticed you posting a patch there. I'll take a look.
Assignee | ||
Comment 12•14 years ago
|
||
This issue has been fixed in bug 391385.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 13•14 years ago
|
||
Patch for bug 391385 was backed out
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 14•14 years ago
|
||
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
Comment 15•14 years ago
|
||
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: 14 years ago → 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Comment 16•14 years ago
|
||
had to back this out while investigating tree orange, and couldn't check back in, because the tree was closed.
Comment 17•14 years ago
|
||
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: 14 years ago → 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Updated•14 years ago
|
Flags: in-litmus?
Comment 18•14 years ago
|
||
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.
Description
•