Closed Bug 1743146 Opened 3 years ago Closed 3 years ago

Port bug 1718846: Draw our own titlebar buttons when using lightweight themes

Categories

(Thunderbird :: Upstream Synchronization, task)

Product:
Thunderbird
Component:
Upstream Synchronization
Platform:
Unspecified
Linux
Type:
task

Tracking

(thunderbird_esr91 unaffected)

Status:
RESOLVED FIXED
Milestone:
99 Branch
Tracking Flags:
Tracking Status
thunderbird_esr91 --- unaffected

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

Attachments

(3 files, 2 obsolete files)

Screenshot from 2021-11-26 12-17-43.png
(deleted), image/png
Details
dark-theme-light-TB.png
(deleted), image/png
Details
Bug 1743146 - Port bug 1718846: Draw our own titlebar buttons when using lightweight themes. r=aleca
(deleted), text/x-phabricator-request
Details
No description provided.
Attached patch 1743146-theme-titlebar-buttons.patch (obsolete) (deleted) — — Splinter Review

Port of https://hg.mozilla.org/mozilla-central/rev/0d5589e4ee6585207a89410bb3e9b9412c7a82aa

Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9252654 - Flags: review?(alessandro)
Attached patch 1743146-theme-titlebar-buttons.patch (obsolete) (deleted) — — Splinter Review

I also added bug 1432090 with the button positioning.
https://hg.mozilla.org/mozilla-central/rev/50f07fce131a6a67ef8a2feedd3f334a9eda1fa1

Attachment #9252654 - Attachment is obsolete: true
Attachment #9252654 - Flags: review?(alessandro)
Attachment #9252665 - Flags: review?(alessandro)
Comment on attachment 9252665 [details] [diff] [review] 1743146-theme-titlebar-buttons.patch Review of attachment 9252665 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for taking care of this, but I don't think I agree with the approach chosen by m-c. Using non native titlebar buttons when using a theme looks very out of place, and I'm sure many of our Linux users will not like this. Did you try using this discarded approach? https://phabricator.services.mozilla.com/D129393 I don't think we had any report about this issue, but nonetheless I think the correct approach would be to detect the contrast of those elements and enforce dark or light coloring accordingly.
Attachment #9252665 - Flags: review?(alessandro) → review-

A touch of Windows UI in Linux....what could possibly go wrong :P

We should also not include bug 1432090 into this patch as it needs a bit more refinement and further testing from various Linux users using different GTK DEs.
Can you open a separate bug for it?

Attached image dark-theme-light-TB.png (deleted) —

This is how it looks with Breeze-dark system theme and light TB theme. With the patch the caption buttons are dark.

Attachment #9252665 - Attachment is obsolete: true
Attachment #9252654 - Attachment is obsolete: false

Although the buttons are very attractive, they are still very visibly different from rest of UI, and I can't find anything to match it. For a long time I avoided chrome specifically because its CSD did things in its own ways, and when they finally get it right, firefox starts misbehaving... It seems the the browsers seem to alternate between design trends for some reason

I get the rationale of rendering custom buttons as it simplifies a lot of things, but are they necessary when user is on 'System theme-auto'? In that case buttons would not have bad contrast provided gtk theme is properly applied. In all other cases, custom buttons can be used. Currently, I am seeing buttons even with system auto theme.

Leo, this is a TB bug. If you want comment about Firefox you should do this in bug 1718846.

Attachment #9252654 - Attachment is obsolete: true
status-thunderbird_esr91: --- → unaffected
Keywords: checkin-needed-tb
Target Milestone: --- → 99 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/c844a2197148
Port bug 1718846: Draw our own titlebar buttons when using lightweight themes. r=aleca

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: