1417693 - Either remove usages of the 'button-image' binding, or move it to comm-central

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Brian Grinstead [:bgrins]]

Looks like there are a few usages of the binding which is being planned to be removed in Bug 1416524: https://searchfox.org/comm-central/search?q=.xml%23button-image&path=.

It's a pretty simple binding: https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/toolkit/content/widgets/button.xml#379-384

Comment 1

[Jorg K (CEST = GMT+2)]

TB/IM are also affected here.

Comment 2

[Richard Marti (:Paenglab)]

Yes, I have it on the radar. But I leave the n-i to not forget it.

Comment 3

[Richard Marti (:Paenglab)]

Attachment: button-image-binding-TB.patch

TB uses the binding only in Chat.

I don't build SM, so I only do the TB part.

Comment 4

[Stefan [:stefanh]]

Attachment: Stop using the binding in suite

Note that this is a shared file, so I have to override all 3 button.css. I can’t do it the way Richard does since the SM button is in a xul file. Instead I have to use descendant selectors to override the .button-* rules in button.css...

Comment 5

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8934282 [details] [diff] [review]
Stop using the binding in suite

Works for me. Thanks.

Comment 6

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8929767 [details] [diff] [review]
button-image-binding-TB.patch

Florian asked if I could take this; flagging myself.

Comment 7

[Florian Quèze [:florian]]

Comment on attachment 8929767 [details] [diff] [review]
button-image-binding-TB.patch

Review of attachment 8929767 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me on Mac, but I'm puzzled about the platform differences.

::: mail/themes/osx/mail/chat.css
@@ +75,5 @@
>  }
>  
> +.startChatBubble > .button-box > .button-icon,
> +.closeConversationButton > .button-box > .button-icon {
> +  margin-inline-start: 0;

Why do we need a margin-inline-start on Mac, margin-inline-end on Linux, and nothing on Windows?

Comment 8

[Richard Marti (:Paenglab)]

(In reply to Florian Quèze [:florian] from comment #7)
> Comment on attachment 8929767 [details] [diff] [review]
> button-image-binding-TB.patch
> 
> Review of attachment 8929767 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me on Mac, but I'm puzzled about the platform differences.
> 
> ::: mail/themes/osx/mail/chat.css
> @@ +75,5 @@
> >  }
> >  
> > +.startChatBubble > .button-box > .button-icon,
> > +.closeConversationButton > .button-box > .button-icon {
> > +  margin-inline-start: 0;
> 
> Why do we need a margin-inline-start on Mac, margin-inline-end on Linux, and
> nothing on Windows?

On Mac, to remove this margin: https://dxr.mozilla.org/comm-central/source/mozilla/toolkit/themes/osx/global/button.css#41, on Linux, this: https://dxr.mozilla.org/comm-central/source/mozilla/toolkit/themes/linux/global/button.css#26, and on Windows there is no such rule.

Comment 9

[Florian Quèze [:florian]]

Comment on attachment 8929767 [details] [diff] [review]
button-image-binding-TB.patch

(In reply to Richard Marti (:Paenglab) from comment #8)

Thanks! And sorry for the review delay :-(.

Comment 10

[Richard Marti (:Paenglab)]

Thanks. There was no hurry until bug 1416524 lands.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/340a97792676
Remove the use of the button-image binding in TB. r=florian
https://hg.mozilla.org/comm-central/rev/492ec0687aad
Stop using the 'button-image' binding in suite/. r=frg