Closed Bug 1417693 Opened 2 years ago Closed 2 years ago

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

Categories

(Thunderbird :: General, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 59.0

People

(Reporter: bgrins, Assigned: Paenglab)

References

Details

Attachments

(2 files)

TB/IM are also affected here.
Flags: needinfo?(richard.marti)
Yes, I have it on the radar. But I leave the n-i to not forget it.
TB uses the binding only in Chat.

I don't build SM, so I only do the TB part.
Flags: needinfo?(richard.marti)
Attachment #8929767 - Flags: review?(florian)
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...
Attachment #8934282 - Flags: review?(frgrahl)
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Comment on attachment 8934282 [details] [diff] [review]
Stop using the binding in suite

Works for me. Thanks.
Attachment #8934282 - Flags: review?(frgrahl) → review+
Comment on attachment 8929767 [details] [diff] [review]
button-image-binding-TB.patch

Florian asked if I could take this; flagging myself.
Attachment #8929767 - Flags: review?(florian) → review?(nhnt11)
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?
(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 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 :-(.
Attachment #8929767 - Flags: review?(nhnt11) → review+
Thanks. There was no hurry until bug 1416524 lands.
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
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
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 59.0
You need to log in before you can comment on or make changes to this bug.