1559601 - Dropdown button text in unifinder and task tab not readable due to color issues

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Stefan Sitter [:ssitter]]

Attachment: 2019-06-15-unifinder-tasktab.png

Thunderbird 68.0b1 (20190613171710) on Xubuntu 19.04

Thunderbird default theme is activated. Lightnings preference "optimize colors for accessibility" is enabled. System theme is set to Adwaita Dark.

Error:
The dropdown button text in unifinder is barely readable due to color issues.
The dropdown button text in task tab is barely readable due to color issues.

Comment 1

[Richard Marti (:Paenglab)]

This is because these aren't toolbars any more since bug 1535265. On dark system themes an attribute is set on the toolbars with this function: https://searchfox.org/comm-central/rev/d2e573a84da86590d926e36e49905d8fc56c6204/mail/base/content/toolbarIconColor.js#71

Magnus or Geoff, would it be possible to extend this function to apply to the "no more" toolbars?

Comment 2

[Richard Marti (:Paenglab)]

Or we could add the brighttext attribute to the window instead the toolbars and adapt the CSS code this.

Comment 3

[Magnus Melin [:mkmelin]]

Should be easy to extend I think. Can you just add the needed selector (with a comma) to https://searchfox.org/comm-central/rev/d2e573a84da86590d926e36e49905d8fc56c6204/mail/base/content/toolbarIconColor.js#71

Comment 4

[Richard Marti (:Paenglab)]

Attachment: 1559601-brighttext-to-toolbar-class.patch

Magnus, you meant something like this?

I'm asking Geoff for review because it has calendar code in it.

Comment 5

[Geoff Lankow (:darktrojan)]

Comment on attachment 9072757 [details] [diff] [review]
1559601-brighttext-to-toolbar-class.patch

Nice. This is a big improvement, especially removing that double background on menu-button buttons.

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 9072757 [details] [diff] [review]
1559601-brighttext-to-toolbar-class.patch

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

::: mail/base/content/toolbarIconColor.js
@@ +70,5 @@
> +
> +    let toolbarSelector = "toolbox > toolbar:not([collapsed=true])";
> +    if (AppConstants.platform == "macosx") {
> +      toolbarSelector += ":not([type=menubar])";
> +    }

Instead of doing the getElementsByClassName later, I think here you can just do

toolbarSelector += ", .toolbar";

@@ +74,5 @@
> +    }
> +
> +    for (let toolbar of document.querySelectorAll(toolbarSelector)) {
> +      checkTextColor(toolbar);
> +    }

This could just be

document.querySelectorAll(toolbarSelector).forEach(checkTextColor);

Comment 7

[Richard Marti (:Paenglab)]

Attachment: 1559601-brighttext-to-toolbar-class.patch

Implemented Magnus's proposal with adding the toolbarSelector += ", .toolbar";. This makes it much simpler.

Comment 8

[Richard Marti (:Paenglab)]

For beta I have to create a different patch because the messageheader toolbar is still customizable.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/252ff46d7f01
Apply the 'brighttext' attribute to the <hbox> toolbars. r=darktrojan

Comment 10

[Richard Marti (:Paenglab)]

Attachment: 1559601-brighttext-to-toolbar-class-beta.patch

This is the patch for beta. Removed the change for the messageheader toolbar.

Comment 11

[Jorg K (CEST = GMT+2)]

TB 68 beta 2 / Cal 7.0:
https://hg.mozilla.org/releases/comm-beta/rev/acc642013a950e299a3e1dac8f84c4d48026d126

Richard, please check that it's working as expected on the beta build.