516384 - [Vista] New toolbar button icon improvements

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect, P3)

Description

[Thomas Stache]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3
Build Identifier: 

The new icon for the "File" toolbar button in the default Aero theme introduced in bug 506011 looks clipped at the bottom, as if a pixel is missing.

Additionally, the disabled "Tags" icon is nearly invisible, because it blends into the toolbar background perfectly.

Screenshot coming up...

Reproducible: Always

Comment 1

[Thomas Stache]

Attachment: Disabled "Tags" icon almost invisible

Comment 2

[Thomas Stache]

Attachment: "File" icon looks clipped at bottom border

Comment 3

[Thomas Stache]

3. issue:
both mentioned icons (Tag + File) shift depending on the state, which becomes obvious when selecting/deselecting a message in the message list.

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

adding andreas to take a look

Comment 5

[Andreas Nilsson (:andreasn)]

Attachment: patch to fix issues 1, 2 and 3

Haven't tried this out on my Vista system yet, as it's currently behaving a bit weird. Will see if a huge recompile fixes that. If any other Vista users want to take a look at the patch in the mean time, that would be great!

Comment 6

[Thomas Stache]

The attachment (which should be marked as patch) only seems to change the image, not the CSS. Looking at
http://mxr.mozilla.org/comm-central/source/mail/themes/qute/mail/icons/mail-toolbar-aero.png
I can't see how the image could cause the shifting icons depending on enabled/disabled state.

Comment 7

[Thomas Stache]

By the way Andreas, did you also correct the funny archive icon under the Back-Forward icons in the hovered state? ;)

Comment 8

[Andreas Nilsson (:andreasn)]

(In reply to comment #7)
> By the way Andreas, did you also correct the funny archive icon under the
> Back-Forward icons in the hovered state? ;)

Yep, just noticed that while I was editing the file. Not sure how that managed to slip in. :/
Fixed in the patch as well.

Comment 9

[Thomas Stache]

I just fired up my Windows XP, and can confirm that issue 3) -- the shifting icons -- happens there as well -> a CSS issue.

Comment 10

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 400746 [details] [diff] [review]
patch to fix issues 1, 2 and 3

the tag icon continues to be hard to see.  I wonder if it just has to do with the light colors it uses.  Otherwise things seem fine.

Comment 11

[Andreas Nilsson (:andreasn)]

Attachment: updated patch

the tag icon should be more visible when disabled now, but it still jumps around. The css that calls the image looks correct, so I suspect it might have something with the dropdown thing to do.

Comment 12

[Phil Ringnalda (:philor)]

Tag jumps around sounds really familiar, like there must be a bug from shortly before 2.0 shipped where mscott and I talked about it without getting anywhere.

Comment 13

[Andreas Nilsson (:andreasn)]

if the issue with the jumping tag icon happens on _every_ system, I think we can address it by moving the pixmap slightly on hover. So, is there anyone that don't have this issue?

Comment 14

[Thomas Stache]

Attachment: illustration of issue 3), split buttons shifting

(In reply to comment #13)
> if the issue with the jumping tag icon happens on _every_ system, I think we
> can address it by moving the pixmap slightly on hover. So, is there anyone that
> don't have this issue?

No, that wouldn't work to fix issue 3). What I see is:
if you have "File" and "Tag" buttons side-by-side (both are split buttons) and you select/deselect a message the dropdown marker of the split buttons move, making the whole toolbar-button wider, pushing other toolbar contents to the right.

Maybe the text-beside-icons toolbar mode made this more obvious, as now the text moves the same way.

Comment 15

[Thomas Stache]

Having said that, this issue should be split off from this bug, as it's not icon-related. (and most likely there is already a bug for it)

Comment 16

[Phil Ringnalda (:philor)]

Indeed - one that goes by the name bug 397838.

Comment 17

[Andreas Nilsson (:andreasn)]

as 397838 is fixed, can this go in?

Comment 18

[Thomas Stache]

looks like the patch has no review requests.

Comment 19

[Phil Ringnalda (:philor)]

Did you want to perk up the tag icon in mail-toolbar-small-aero.png, too? I'm just looking at the bare images right now, so I can't tell whether or not you ought to want to.

Comment 20

[Phil Ringnalda (:philor)]

Comment on attachment 400993 [details] [diff] [review]
updated patch

But now that I've finally remembered to look, we're fine with the small disabled tag, and even though not much of it shows, I think we really do want to get rid of that archive-under-back :)

Comment 21

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 400993 [details] [diff] [review]
updated patch

looks good, though i think we want to fix that archive-under-back

Comment 22

[Phil Ringnalda (:philor)]

This patch *does* fix it, that was why I wanted it so much :)

Comment 23

[Bryan Clark (DevTools PM) [:clarkbw]]

Ah ha!  It appears that you need to compile the patch in for it to work.  I'll have to remember that for next time!

Comment 24

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/a77204848e3e
http://hg.mozilla.org/releases/comm-1.9.1/rev/f423550f3fd1