405892 - Finish polish of the search field

Status: VERIFIED FIXED

(Toolkit :: Downloads API, enhancement, P4)

Description

[Madhava Enros [:madhava]]

[actually nominating for wanted]

As shown in this mockup (attached to bug 397655) :
https://bugzilla.mozilla.org/attachment.cgi?id=288692

We should add the magnifying glass icon (appropriate per platform) that we use in all search fields in Firefox.

Comment 1

[Mike Connor [:mconnor]]

We need a global searchbar style for every search widget, and to just use that where appropriate.

Comment 2

[Ed Lee :Mardak]

Would it be bad to just reuse chrome://browser/skin/Search.png and related styles?

Is there an issue of download manager being in toolkit?

madhava: Is the endcap supposed to be round on os x (basically copy what the search engine bar looks like)?

If so, Dão, the search box can't be just a plain textbox with an image tacked on at the end? (Even if not, what about rtl issues?)

Comment 3

[Dão Gottwald [:dao]]

(In reply to comment #2)
> Would it be bad to just reuse chrome://browser/skin/Search.png and related
> styles?
>
> Is there an issue of download manager being in toolkit?

Yes. What we can do is move the image to /toolkit and update the references in /browser.

> If so, Dão, the search box can't be just a plain textbox with an image tacked
> on at the end? (Even if not, what about rtl issues?)

We should just use the upcoming search textbox widget.

Comment 4

[Madhava Enros [:madhava]]

Yeah, it ought to match OS X style on mac, so we should use the search textbox widget.

Comment 5

[Ed Lee :Mardak]

So OS X has a search icon, rounded search box, text "Search...", and it's right aligned in the window.

Should other platforms get some/all of this treatment?

Assuming we won't be able to use the search textbox widget..

Comment 6

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

On OSX round=search, yes, and it should get that view. I'm sad we won't be able to use the search textbox widget, though. A boo hoo hoo.

Comment 7

[Shawn Wilsher :sdwilsh]

Are we not getting the textbox widget anymore?

Comment 8

[Dão Gottwald [:dao]]

Attachment: patch

this builds upon the patch in bug 388811

Comment 9

[Shawn Wilsher :sdwilsh]

Do we need to remove any styles for this?  Do our unit tests still pass?

Comment 10

[Dão Gottwald [:dao]]

pinstripe should be cleaned up. The tests should pass since the search box comes from a timed textbox, although I haven't yet looked at your tests in detail.

Comment 11

[Dão Gottwald [:dao]]

Attachment: patch, including pinstripe cleanup

Comment 12

[Ed Lee :Mardak]

Shawn is probably referring to this testcase in particular:

http://mxr.mozilla.org/seamonkey/source/toolkit/mozapps/downloads/tests/browser/browser_bug_416303.js

It makes sure hitting esc in the input clears the search and moves focus to the list. Hitting esc from the list closes the window.

Comment 13

[Dão Gottwald [:dao]]

Yes, I knew about these tests theoretically. The second test script that interacts with the search field is browser_multiword_search.js. Both will pass if bug 388811 gets fixed as expected.

Comment 14

[Shawn Wilsher :sdwilsh]

Comment on attachment 308938 [details] [diff] [review]
patch, including pinstripe cleanup

r=sdwilsh, but you need to cleanup winstripe and gnomestripe as well.

Comment 15

[Dão Gottwald [:dao]]

winstripe and gnomestripe don't seem to provide a special styling for the search field. Am I missing something?

Comment 16

[Shawn Wilsher :sdwilsh]

Comment on attachment 308938 [details] [diff] [review]
patch, including pinstripe cleanup

hehe, I thought they did, but upon further investigation I see I was wrong.  r=sdwilsh as the patch stands.

Comment 17

[Damon Sicore (:damons)]

Comment on attachment 308938 [details] [diff] [review]
patch, including pinstripe cleanup

a1.9+=damons

Comment 18

[Dão Gottwald [:dao]]

please don't check this in before bug 388811.

Comment 19

[Dão Gottwald [:dao]]

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

Comment 20

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

Mmh why the magnifying icon is still on the left side? Each search field shows this icon on the right side. Is this intended or shall I file a new bug about that issue?

Comment 21

[Dão Gottwald [:dao]]

(In reply to comment #20)
> Mmh why the magnifying icon is still on the left side?

That is taken over from pinstripe's styling for the search fields in the bookmarks and history sidebar.

Comment 22

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

(In reply to comment #21)
> That is taken over from pinstripe's styling for the search fields in the
> bookmarks and history sidebar.

I filed bug 449465.

Comment 23

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

Dao, when hitting ESC within the web search field the content doesn't get cleared like it happens for all the other search textbox fields. Is this instance different? If its not intended we should reopen the bug and fix the remaining issue.

Comment 24

[Dão Gottwald [:dao]]

Which web search field?

Comment 25

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

"Tools | Web Search" or simply the one near the location bar e.g the Google search.

Comment 26

[Dão Gottwald [:dao]]

So which bug would you want to reopen? This one is about the download manager.

FYI: The searchbar has its own binding. I don't think we want to use the new search textbox widget from toolkit there, since you could only clear the textbox but not the results.

Comment 27

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

Thanks for the enlightenment.

Verified this replacement 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 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080808120213 Minefield/3.1a2pre ID:20080808120213