1702788 - Adapt TB to work with browser.proton.enabled enabled properly

Status: RESOLVED FIXED

(Thunderbird :: Theme, task)

Description

[Richard Marti (:Paenglab)]

Fx is planning to enable browser.proton.enabled soon. We should begin to adapt TB to follow (but not the tabs appearance).

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1702788-proton-fixes.patch

This is my first shot. There are a lot of changes where I unified more code. But also a lot of @supports -moz-bool-pref("browser.proton.enabled") switches to enable Proton code and disable non-Proton code.

Comment 2

[Alessandro Castellani [:aleca]]

Comment on attachment 9213343 [details] [diff] [review]
1702788-proton-fixes.patch

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

Wow, thank you so much for going through this massive endeavor, the UI already looks 10 times better with these changes when proton is enabled.
Just a few small changes before approving it.
Great work!

::: calendar/base/themes/common/today-pane.css
@@ +288,1 @@
>  #show-completed-checkbox > .checkbox-check {

I think we should properly indent these selectors inside the @support brackes, even if it means a bigger diff, but at least is accurate.

::: calendar/base/themes/common/widgets/calendar-widgets.css
@@ +36,5 @@
>    border: none;
>    width: 10px;
>    height: 10px;
> +  color: inherit;
> +  list-style-image: url(chrome://global/skin/icons/twisty-expanded.svg);

This should be background-image otherwise it would conflict with the CSS declaration coming from checkbox.css
Also it needs a background-size: contain;

::: mail/base/content/aboutAddonsExtra.css
@@ +12,5 @@
>    }
>  }
>  
> +@supports -moz-bool-pref("browser.proton.enabled") {
> +@media (prefers-color-scheme: dark) {

Indent properly also here, and anywhere else we have this new support query.

::: mail/themes/windows/mail/compose/messengercompose.css
@@ +134,3 @@
>  #msgIdentity::part(dropmarker) {
>    display: none;
>  }

You added this in the shared messengercompose.css, can this be removed from the Windows variation?

Comment 3

[Richard Marti (:Paenglab)]

Attachment: 1702788-proton-fixes.patch

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

Comment on attachment 9213343 [details] [diff] [review]
1702788-proton-fixes.patch

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

::: calendar/base/themes/common/today-pane.css
@@ +288,1 @@

#show-completed-checkbox > .checkbox-check {

I think we should properly indent these selectors inside the @support
brackes, even if it means a bigger diff, but at least is accurate.

It's by intent to not indent. When Proton stays, all non-Proton code will be removed and the @support lines will be removed. When we indent now, this means we have to un-indent it again and this removal would be the blame bug of the lines inbetween the @supports instead of the correct one that last touched it.

::: calendar/base/themes/common/widgets/calendar-widgets.css
@@ +36,5 @@

border: none;
width: 10px;
height: 10px;

• color: inherit;
• list-style-image: url(chrome://global/skin/icons/twisty-expanded.svg);

This should be background-image otherwise it would conflict with the CSS
declaration coming from checkbox.css
Also it needs a background-size: contain;

fixed it

::: mail/base/content/aboutAddonsExtra.css
@@ +12,5 @@

}
}

+@supports -moz-bool-pref("browser.proton.enabled") {
+@media (prefers-color-scheme: dark) {

Indent properly also here, and anywhere else we have this new support query.

See above

::: mail/themes/windows/mail/compose/messengercompose.css
@@ +134,3 @@

#msgIdentity::part(dropmarker) {
display: none;
}

You added this in the shared messengercompose.css, can this be removed from
the Windows variation?

The shared is for Proton and this one is for non-Proton. Complicated, I know.

Comment 4

[Richard Marti (:Paenglab)]

Attachment: 1702788-proton-fixes.patch

Same patch but with 3px border-radius for the buttons etc. including the items in the panels.

Comment 5

[Alessandro Castellani [:aleca]]

Comment on attachment 9213609 [details] [diff] [review]
1702788-proton-fixes.patch

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

Great work!

Comment 6

[Richard Marti (:Paenglab)]

Thanks!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/639d7085bcd2
Adapt TB to work with browser.proton.enabled enabled properly. r=aleca

Comment 8

[Magnus Melin [:mkmelin]]

I suspect this is causing TEST-UNEXPECTED-FAIL | comm/mail/test/static/browser_parsable_css.js | Got error message for chrome://messenger/skin/themeableDialog.css: Unexpected token ‘:’ in media list.
https://treeherder.mozilla.org/logviewer?job_id=335536692&repo=comm-central&lineNumber=5473

Comment 9

[Richard Marti (:Paenglab)]

(In reply to Magnus Melin [:mkmelin] from comment #8)

I suspect this is causing TEST-UNEXPECTED-FAIL | comm/mail/test/static/browser_parsable_css.js | Got error message for chrome://messenger/skin/themeableDialog.css: Unexpected token ‘:’ in media list.
https://treeherder.mozilla.org/logviewer?job_id=335536692&repo=comm-central&lineNumber=5473

I'll fix this in bug 1703268.