Closed Bug 1559601 Opened 5 years ago Closed 5 years ago

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

Categories

(Calendar :: Calendar Frontend, defect)

Product:
Calendar
Component:
Calendar Frontend
Version:
Lightning 68
Platform:
Unspecified
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: ssitter, Assigned: Paenglab)

References

(Regression)

Details

(Keywords: access, regression)

Attachments

(3 files, 1 obsolete file)

2019-06-15-unifinder-tasktab.png
(deleted), image/png
Details
1559601-brighttext-to-toolbar-class.patch
(deleted), patch
Paenglab
: review+
Details | Diff | Splinter Review
1559601-brighttext-to-toolbar-class-beta.patch
(deleted), patch
Paenglab
: review+
Fallen
: approval-calendar-beta+
Details | Diff | Splinter Review
Attached image 2019-06-15-unifinder-tasktab.png (deleted) —

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.

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?

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(geoff)
Keywords: regression
Regressed by: 1535265

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

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

Flags: needinfo?(mkmelin+mozilla)
Attached patch 1559601-brighttext-to-toolbar-class.patch (obsolete) (deleted) — — Splinter Review

Magnus, you meant something like this?

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

Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Flags: needinfo?(geoff)
Attachment #9072757 - Flags: review?(geoff)
Attachment #9072757 - Flags: feedback?(mkmelin+mozilla)
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.
Attachment #9072757 - Flags: review?(geoff) → review+
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);
Attachment #9072757 - Flags: feedback?(mkmelin+mozilla) → feedback+

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

Attachment #9072757 - Attachment is obsolete: true
Attachment #9072767 - Flags: review+
Keywords: checkin-needed

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

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

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 7.1

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

Attachment #9072841 - Flags: review+
Attachment #9072841 - Flags: approval-calendar-beta?(philipp)

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.

Target Milestone: 7.1 → 7.0
Attachment #9072841 - Flags: approval-calendar-beta?(philipp) → approval-calendar-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: