482717 - delete button don't use systems delete icon

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, enhancement)

Description

[Andreas Nilsson (:andreasn)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.7) Gecko/2009030422 Ubuntu/8.10 (intrepid) Firefox/3.0.7 Ubiquity/0.1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b4pre) Gecko/20090310 Shredder/3.0b3pre

It would be nice if Thunderbird picked up the delete icon from the system.

Reproducible: Always

Comment 1

[Andreas Nilsson (:andreasn)]

Attachment: patch to fix the issue

Comment 2

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

Attachment: same patch, using hg against trunk

Comment 3

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

it looks slightly off center for me, like it's aligned left instead of in the center of the button

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Seems like analogous patches for the non-linux themes could also be worthwhile.

Comment 5

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

Attachment: updated patch removing alignment issue

here's an updated patch that removes the alignment issue.  the global/skin/button.css was adding -moz-margin-end: 2px; and causing this misalignment.

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 367311 [details] [diff] [review]
updated patch removing alignment issue

For better efficiency, you should be able to use

.hdrTrashButton[trash="true"] > .button-box > .button-icon 

... in stead?

For the "better fix wanted" also showing in the patch, how about this?
.msgHeaderView-button[anonid="hdrTrashButton"] {

(Also to be used for the new lines of course.)

Comment 7

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

Attachment: removes trash="true" selector, improves child selector

(In reply to comment #6)
> (From update of attachment 367311 [details] [diff] [review])
> For better efficiency, you should be able to use
> 
> .hdrTrashButton[trash="true"] > .button-box > .button-icon 

yes, didn't mean to get lazy. :)

> For the "better fix wanted" also showing in the patch, how about this?
> .msgHeaderView-button[anonid="hdrTrashButton"] {

good point, I didn't know you could use anonid attr selectors but it makes sense.

> (Also to be used for the new lines of course.)

Changed from trash=true to the anonid selector.  I also removed the trash="true" from the XBL and made the appropriate changes in (1) other necessary files.

http://mxr.mozilla.org/comm-central/search?string=trash=%22true%22

Comment 8

[Magnus Melin [:mkmelin]]

Comment on attachment 367689 [details] [diff] [review]
removes trash="true" selector, improves child selector

Actually, seems to me that code comment was bogus and we don't have to do even that. 

At least this seems to work fine for me

.hdrTrashButton {
  -moz-box-orient: vertical;
  list-style-image: url("moz-icon://stock/gtk-delete?size=menu");
}

.hdrTrashButton > .button-box > .button-icon {
  -moz-margin-end: 0px;
}

Comment 9

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

Attachment: removed anonid selector

yeah, that does work.  should have checked if it was correct from the beginning :)

Here's the updated patch

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 368068 [details] [diff] [review]
removed anonid selector

r=mkmelin

Is the "!important" really needed for -moz-box-orient though? My test was without and pistripe doesn't have one. 

So please remove if appropriate.

Comment 11

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

Attachment: final patch, removes the !important attr

(In reply to comment #10)
> (From update of attachment 368068 [details] [diff] [review])
> r=mkmelin
> 
> Is the "!important" really needed for -moz-box-orient though? My test was
> without and pistripe doesn't have one. 
> 
> So please remove if appropriate.

right, tested this out and it isn't needed.

will mark this for checkin

carrying forward the r=mkmelin

Comment 12

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/96138c622010