Closed Bug 482717 Opened 15 years ago Closed 15 years ago

delete button don't use systems delete icon

Categories

(Thunderbird :: Message Reader UI, enhancement)

All
Linux
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b3

People

(Reporter: andreasn, Assigned: clarkbw)

References

Details

(Whiteboard: [tango])

Attachments

(1 file, 5 obsolete files)

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
Whiteboard: Tango
Attached patch patch to fix the issue (obsolete) — Splinter Review
Assignee: nobody → clarkbw
Attachment #366813 - Attachment is obsolete: true
Status: UNCONFIRMED → NEW
Ever confirmed: true
it looks slightly off center for me, like it's aligned left instead of in the center of the button
Blocks: 415415
Seems like analogous patches for the non-linux themes could also be worthwhile.
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.
Attachment #366816 - Attachment is obsolete: true
Attachment #367311 - Flags: review?(mkmelin+mozilla)
Severity: normal → enhancement
Hardware: x86 → All
Target Milestone: --- → Thunderbird 3.0b3
Version: unspecified → Trunk
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.)
(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
Attachment #367311 - Attachment is obsolete: true
Attachment #367689 - Flags: review?(mkmelin+mozilla)
Attachment #367311 - Flags: review?(mkmelin+mozilla)
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;
}
Attached patch removed anonid selector (obsolete) — Splinter Review
yeah, that does work.  should have checked if it was correct from the beginning :)

Here's the updated patch
Attachment #367689 - Attachment is obsolete: true
Attachment #368068 - Flags: review?(mkmelin+mozilla)
Attachment #367689 - Flags: review?(mkmelin+mozilla)
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.
Attachment #368068 - Flags: review?(mkmelin+mozilla) → review+
(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
Attachment #368068 - Attachment is obsolete: true
Attachment #368328 - Flags: review+
Status: NEW → ASSIGNED
Keywords: checkin-needed
Whiteboard: Tango → [tango][wating for checkin]
http://hg.mozilla.org/comm-central/rev/96138c622010
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [tango][wating for checkin] → [tango]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: