337720 - F4 or alt+up/down in the search box no longer opens dropdown

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

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

Bug 283273 seems to have regressed: neither F4 nor Alt+Up/Down seem to be opening the engine drop down when focus is in the search bar. Seems like search.xml's openSearch method no longer works. I think this is a regression from bug 335441, but I'm not sure.

I tested the 05-11 1.8 branch build on Windows.

Comment 1

[Mike Connor [:mconnor]]

Pam, this is the stuff I mentioned last week.

Comment 2

[Pam Greene]

Attachment: Fixes keyboard commands and search-go-button's anonid

This patch includes the fixes for bug 337343 (changing the anonid for the search-go-button), since they're interrelated.  My Windows build isn't working yet, so if someone could test this there I'd appreciate it.

Comment 3

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

Seems to work fine on Windows.

Looks like you forgot to remove:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/search/content/searchbarBindings.css&rev=1.3&mark=7
and http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/themes/winstripe/browser/browser.css&rev=1.33#60 The second one looks like it could fix bug 337174 if it was appropriately tweaked.

Comment 4

[Pam Greene]

Attachment: Patch addressing Gavin's comments, including fixing 337174

Yup, I missed the first one; thanks.

The second one, for bug 337174, I've had waiting for a Windows build to test it on; but if you're willing to do that, I'm more than happy to roll it into this patch too.

Comment 5

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

Best to keep it separate, I think, I just wanted to point it out.

Comment 6

[Pam Greene]

Attachment: Patch addressing Gavin's comments, but not fixing 337174

Fair enough.  I'll leave that entirely alone here, then, with the understanding that it'll be fixed in bug 337174.

Comment 7

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

Comment on attachment 221856 [details] [diff] [review]
Patch addressing Gavin's comments, but not fixing 337174

>Index: components/search/content/search.xml

>-  <binding id="searchbar-dropmarker"
>-           extends="chrome://browser/content/search/search.xml#searchbar-base">

Since you're removing this, you can now get rid of the searchbar-base binding, and move it's <resources> to the searchbar binding itself.

>-  list-style-image: url("chrome://browser/skin/search-arrow.gif");

Need to remember to CVS remove this. Also need to remove:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/themes/winstripe/browser/jar.mn&rev=1.23&mark=14

Comment 8

[Pam Greene]

Attachment: Patch addressing Gavin's comments

And a reminder to whoever checks this in to CVS remove browser/themes/winstripe/browser/search-arrow.gif as well.

Comment 9

[Phil Ringnalda (:philor)]

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

Comment 10

[Pam Greene]

Attachment: Fresh patch for checkin

Updated to resolve merge conflicts.

Comment 11

[Brian Ryner (not reading)]

checked in