395642 - make clear button in Applications prefpane be "x" button inside search field

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P4)

Description

[Myk Melez [:myk] [@mykmelez]]

faaborg's review in bug 377784, comment 45 says that the clear button in the Applications prefpane should become an "x" button inside the search field on the right-hand side.

This bug is specific to that search field, but mconnor further notes in bug 377784, comment 46 that "We need the search textbox widget to do all of this instead of continuously reinventing this wheel," so maybe this bug should depend on another bug that implements this functionality generically.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Requesting wanted-1.9 for this Applications prefpane polish fix.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Not blocking, so lower priority.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Dao still plans to get to 388811 in the Fx3 timeframe, after which it'll be trivial to fix this bug.

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 332438 [details] [diff] [review]
patch

This looks perfect, but for some reason it isn't working.  A bit of preliminary investigation reveals that the command handler isn't getting called once I stop typing in the field, but it's not clear why.  No errors show up in the console.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Myk, it looks like something messed-up in your profile? I tried the tryserver build from Dao and it works perfectly for me. The x button is visible after I have pressed the first key. 

Comment 7

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #6)
> Myk, it looks like something messed-up in your profile? I tried the tryserver
> build from Dao and it works perfectly for me. The x button is visible after I
> have pressed the first key. 

I applied the patch, but I haven't pulled for a few days.  I bet I don't have the code that implements the type="search" textbox.  D'oh!  Fixing...

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 332438 [details] [diff] [review]
patch

(In reply to comment #7)
> I applied the patch, but I haven't pulled for a few days.  I bet I don't have
> the code that implements the type="search" textbox.  D'oh!  Fixing...

Yup, that was the problem.  Sorry for the confusion.  This patch looks great.  r=myk

Comment 9

[Dão Gottwald [:dao]]

http://hg.mozilla.org/index.cgi/mozilla-central/rev/df933c713700

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080808120213 Minefield/3.1a2pre ID:20080808120213

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080808031528 Minefield/3.1a2pre ID:20080808031528