Closed Bug 506626 Opened 11 years ago Closed 11 years ago

the qute theme uses a older style search icon

Categories

(Thunderbird :: Message Reader UI, defect)

x86
Windows Vista
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b4

People

(Reporter: andreasn, Assigned: andreasn)

References

Details

Attachments

(2 files, 4 obsolete files)

The qute theme currently makes use of a old style search icon. Would be nice to make use of the same icon as Firefox instead.
Attached patch patch using the global icon (obsolete) — Splinter Review
Here is some css using the global icon.
Assignee: nobody → nisses.mail
Status: NEW → ASSIGNED
Have only tested this on Vista, so someone needs to confirm it works on XP as well.
Blocks: 488060
Attachment #390792 - Flags: ui-review?(clarkbw)
Comment on attachment 390792 [details] [diff] [review]
patch using the global icon

looks good to me on Vista.

philor, can you review this? I didn't see Firefox doing anything XP specific for the search icon.

Though I did notice that firefox is handling the rtl for this and we should probably be doing the same.

.search-go-button:-moz-locale-dir(rtl) {
   -moz-transform: scaleX(-1);
 }
Attachment #390792 - Flags: ui-review?(clarkbw)
Attachment #390792 - Flags: ui-review+
Attachment #390792 - Flags: review?(philringnalda)
Blocks: 484166
Comment on attachment 390792 [details] [diff] [review]
patch using the global icon

Yes on all counts, except the r+ one ;)

It looks just fine in ltr XP, but we do need to do rtl, since otherwise it's a pretty awful dropmarker stuck to the bottom of the handle. Not with :-moz-locale-dir since that's 1.9.2-only, but since bug 484166 used chromedir for gnomestripe, there must be a chromedir attribute within range.

Since this appears to make Search-bar.png unused, we should also stop packaging it, and remove the image file.
Attachment #390792 - Flags: review?(philringnalda) → review-
Attached patch updated patch (obsolete) — Splinter Review
adds rtl and removes Search-bar.png from the tree and jar.nm file
Attachment #390792 - Attachment is obsolete: true
Attachment #393643 - Attachment is obsolete: true
Hmm, patch doesn't seem to be the correct one...

Also, for Windows when you remove a file you need to add it somewhere into the removed-files file.  This makes sure that the file gets removed for users that are updating.

mail/installer/removed-files.in
Attached patch _this_ is the right patch (obsolete) — Splinter Review
My filesystem is playing tricks on me, kept uploading the file from the wrong folder...
Sorry for that.
Attachment #393657 - Attachment is obsolete: true
Attachment #393662 - Flags: review?(mkmelin+mozilla)
(In reply to comment #7)
> Hmm, patch doesn't seem to be the correct one...
> 
> Also, for Windows when you remove a file you need to add it somewhere into the
> removed-files file.  This makes sure that the file gets removed for users that
> are updating.
> 
> mail/installer/removed-files.in

This is not correct for icon type files that are being jar'd up so just ignore it and lets assume that I need more coffee. :)
to make it work on 1.9.1 instead of only trunk
Attachment #393662 - Attachment is obsolete: true
Attachment #393670 - Flags: review?(mkmelin+mozilla)
Attachment #393662 - Flags: review?(mkmelin+mozilla)
Comment on attachment 393670 [details] [diff] [review]
make use of chromedir instead of -moz-locale-dir

I'll let philor finish off this review as he's already done most of it. Looks fine to me (untested).
Attachment #393670 - Flags: review?(mkmelin+mozilla) → review?(philringnalda)
Just so I don't forget by the time I get to reviewing: the selector needs to be "#quick-search-button[chromedir="rtl"] .quick-search-button-image" since one of the reasons chromedir needed to die was that you had to stick it on every element you wanted to use as a selector, and for reasons I've forgotten since we have one on the button, and one on the clear-button, but not on the button-image.
Oh, the reason why is that I needed it there for Pinstripe, since Mac rtl is weird in many ways including not flipping the magnifying glass, and then I failed to properly review the gnomestripe fix in bug 484166. Precursor patch to add one to the quick-search-button-image and make your patch work once I get back to that computer. (And, I'm *so* going to love landing bug 511682.)
We'll rope you into this bug one way or another ;)

Sets the stage for making andreasn's patch work, and lets gnomestripe work without having to march all the way up to the top of the DOM every time style gets resolved on a .quick-search-button-image.
Attachment #395677 - Flags: review?(mkmelin+mozilla)
Comment on attachment 395677 [details] [diff] [review]
Prequel [checked in]

Sure, r=mkmelin
Attachment #395677 - Flags: review?(mkmelin+mozilla) → review+
Comment on attachment 395677 [details] [diff] [review]
Prequel [checked in]

http://hg.mozilla.org/comm-central/rev/39cf09d5e38b
Attachment #395677 - Attachment description: Prequel → Prequel [checked in]
Attachment #393670 - Flags: review?(philringnalda) → review+
http://hg.mozilla.org/comm-central/rev/610cd9957818
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b4
You need to log in before you can comment on or make changes to this bug.