Closed Bug 1710214 Opened 4 years ago Closed 3 years ago

Toolbarbuttons with light themes that don't define border colours have no border on hover

Categories

(Thunderbird :: Theme, defect)

defect

Tracking

(thunderbird_esr78 unaffected, thunderbird89 fixed)

RESOLVED FIXED
90 Branch
Tracking Status
thunderbird_esr78 --- unaffected
thunderbird89 --- fixed

People

(Reporter: Paenglab, Assigned: Paenglab)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

After bug 1691297 the toolbarbutton-1 don't have a visible border with light themes that don't define a border colour like the "Thunderbird Suave" theme.

This comes from the background-clip: padding-box; we have defined. Until bug 1691297 we overwrite this on hover with using background: .... Now we have only set the background-color and the background-clip is still used. Removing it fixes this by always using border-box.

I also changed on menulist the background: to background-color.

Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9220944 - Flags: review?(alessandro)

We could potentially get this in b4 with bug 1691297

Comment on attachment 9220944 [details] [diff] [review] 1710214-no-toolbarbutton-1-padding-box.patch Review of attachment 9220944 [details] [diff] [review]: ----------------------------------------------------------------- This seems good, thanks. Why we had the `background-clip: padding-box;` attribute? What was it used for? In general I'm all for being attribute specific when updated background by using only the color, but are you sure that with this change we're not gonna have any regressions if some elements use a background-image attribute?
Attachment #9220944 - Flags: review?(alessandro) → review+

(In reply to Alessandro Castellani [:aleca] from comment #3)

Comment on attachment 9220944 [details] [diff] [review]
1710214-no-toolbarbutton-1-padding-box.patch

Review of attachment 9220944 [details] [diff] [review]:

This seems good, thanks.
Why we had the background-clip: padding-box; attribute? What was it used
for?

No more sure but I think this was when we used gradients to not paint into the border.

In general I'm all for being attribute specific when updated background by
using only the color, but are you sure that with this change we're not gonna
have any regressions if some elements use a background-image attribute?

When there was a rule that used background-image it wouldn't have applied because of the background: rules we used for the hover state.

Target Milestone: --- → 90 Branch

Comment on attachment 9220944 [details] [diff] [review]
1710214-no-toolbarbutton-1-padding-box.patch

[Approval Request Comment]
Regression caused by (bug #): bug 1691297
User impact if declined: not correct drawn toolbar buttons with light themes
Testing completed (on c-c, etc.): soon on c-c
Risk to taking this patch (and alternatives if risky): low

Attachment #9220944 - Flags: approval-comm-beta?

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/e37008cded84
Remove the padding-box from the toolbarbutton-1. r=aleca

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED

Comment on attachment 9220944 [details] [diff] [review]
1710214-no-toolbarbutton-1-padding-box.patch

[Triage Comment]
Approved for beta

Attachment #9220944 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: