Closed Bug 431002 Opened 16 years ago Closed 16 years ago

Search fields should use search-close instead of the close button for tabs

Categories

(Firefox :: Theme, defect)

Product:
Firefox
Component:
Theme
Platform:
All
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 3.1a1

People

(Reporter: faaborg, Assigned: ehsan.akhgari)

References

Details

(Whiteboard: [has patch] [has review] [RC2-])

Attachments

(1 file, 2 obsolete files)

Patch (v1.1)
2.79 KB, patch
dao
: review+
mtschrep
: approval1.9-
samuel.sidler+old
: approval1.9.0.4-
Details | Diff | Splinter Review 
Search fields, like in the library window and the addons manager window should use Search-close.png and Search-close-aero.png in /themes/winstripe/browser/
Attached patch Patch (v1) (obsolete) — — Splinter Review 
This patch uses the new icon for the search box in Library.  I didn't implement the add-ons manager part, because it's part of the toolkit, while this icon is in browser/, and I'm not sure making toolkit dependent on the browser is the right thing to do here.
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #317957 - Flags: review?(gavin.sharp)
Whiteboard: [has patch] [needs review gavin] [needs attention from UX team, see comment 1]
I can add a search close to toolkit as well, perhaps we should have both references that to reduce redundancy?
Yeah, we should put it in toolkit instead of browser.
Comment on attachment 317957 [details] [diff] [review]
Patch (v1)

OK, waiting on the name and location of the new icon in order to create a new patch.  Will the dimensions be the same as the current icon (chrome://browser/skin/Search-close.png)?
Attachment #317957 - Attachment is obsolete: true
Attachment #317957 - Flags: review?(gavin.sharp)
Whiteboard: [has patch] [needs review gavin] [needs attention from UX team, see comment 1] → [needs status update]
Ping?
Files will be:

/toolkit/themes/winstripe/global/icons/Search-close-aero.png
/toolkit/themes/winstripe/global/icons/Search-close.png

will land with bug 430908
>Will the dimensions be the same as the current icon

yep
Whiteboard: [needs status update]
Depends on: 431285
Attached patch Patch (v1.1) — — Splinter Review 
Updated patch to address the add-ons manager dialog as well.  Please note that together with this patch, the following should be CVS-removed:

/browser/themes/winstripe/browser/Search-close-aero.png
/browser/themes/winstripe/browser/Search-close.png

Also, the close icon from the following files can be removed:

/toolkit/themes/winstripe/mozapps/extensions/searchIcons-aero.png
/toolkit/themes/winstripe/mozapps/extensions/searchIcons.png
Attachment #318322 - Flags: review?(gavin.sharp)
Whiteboard: [has patch] [needs review gavin]
Attached file Search close files (obsolete) — 
Here are the search close files to land at the same time as the patch in case they are not in yet for some reason.
Blocks: 431887
Attachment #318322 - Flags: review?(gavin.sharp) → review?(dao)
Whiteboard: [has patch] [needs review gavin] → [has patch] [needs review dao]
Attachment #318322 - Flags: review?(dao) → review+
Is this bug present also on Linux and MAC?
Comment on attachment 318322 [details] [diff] [review]
Patch (v1.1)

Requesting approval on this CSS-only change to make the close button in search boxes look better.
Attachment #318322 - Flags: approval1.9?
Comment on attachment 318325 [details]
Search close files

These icons need to land in this bug.
Attachment #318325 - Flags: approval1.9?
Whiteboard: [has patch] [needs review dao] → [has patch] [has review] [needs approval]
(In reply to comment #10)
> Is this bug present also on Linux and MAC?

I'm not sure.  If it is, can you file a new bug?  This bug has been filed for Windows...
Comment on attachment 318322 [details] [diff] [review]
Patch (v1.1)

we are closed down for showstoppers. We can get this in 3.0.1 if needed.
Attachment #318322 - Flags: approval1.9? → approval1.9-
Comment on attachment 318325 [details]
Search close files

we are closed down for showstoppers. We can get this in 3.0.1 if needed.
Attachment #318325 - Flags: approval1.9? → approval1.9-
Flags: wanted1.9.0.x?
Whiteboard: [has patch] [has review] [needs approval] → [has patch] [has review] [needs approval][RC2?]
Whiteboard: [has patch] [has review] [needs approval][RC2?] → [has patch] [has review] [needs approval][RC2-]
This can be pushed on mozilla-central now.
Keywords: checkin-needed
Whiteboard: [has patch] [has review] [needs approval][RC2-] → [has patch] [has review] [RC2-]
Comment on attachment 318322 [details] [diff] [review]
Patch (v1.1)

This theme-only change would be nice to have on 3.0.1.
Attachment #318322 - Flags: approval1.9.0.1?
Attachment #318325 - Flags: approval1.9.0.1?
Attachment #318322 - Flags: approval1.9.0.1? → approval1.9.0.2?
Attachment #318325 - Flags: approval1.9.0.1? → approval1.9.0.2?
http://hg.mozilla.org/index.cgi/mozilla-central/rev/7d85b7265c81
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1a1
Comment on attachment 318325 [details]
Search close files

these files are already in the tree
Attachment #318325 - Attachment is obsolete: true
Attachment #318325 - Flags: approval1.9.0.2?
Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a1pre) Gecko/2008071303 Minefield/3.1a1pre ID:2008071303
Status: RESOLVED → VERIFIED
Comment on attachment 318322 [details] [diff] [review]
Patch (v1.1)

Moving approval request out to 1.9.0.3.
Attachment #318322 - Flags: approval1.9.0.2? → approval1.9.0.3?
Doesn't really meet the "wanted" criteria (security, stability, regression from maintenance release).
Flags: wanted1.9.0.x?
Attachment #318322 - Flags: approval1.9.0.4? → approval1.9.0.4-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: