Closed Bug 1747909 Opened 9 months ago Closed 9 months ago

Auto labels use white text over default GUI background until configured

Categories

(Thunderbird :: Message Reader UI, defect)

Thunderbird 91
Desktop
All
defect

Tracking

(thunderbird_esr91+ verified, thunderbird96+ verified, thunderbird97 affected)

VERIFIED FIXED
97 Branch
Tracking Status
thunderbird_esr91 + verified
thunderbird96 + verified
thunderbird97 --- affected

People

(Reporter: lea.gris, Assigned: Paenglab)

References

Details

Attachments

(3 files)

Attached image unreadable.png

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:95.0) Gecko/20100101 Firefox/95.0

Steps to reproduce:

When X-Mozilla-Keys: key words are displayed in message header bar, these renders white text over light grey background in Linux default system colour theme.

To reproduce:
In Thunderbird Linux Ubuntu default theme using system colours:

  1. Subscribe to RSS feed https://stackoverflow.com/feeds/tag?tagnames=bash&sort=newest
    When viewing RSS entries/posts, the labels in header like:
    X-Mozilla-Keys: linux bash oracle
    Keywords: linux, bash, oracle
    these are displayed as unreadable white text over light grey background.

Actual results:

Keywords/Keys displayed as unreadable white text over light grey background.

See attached screenshot portion.

Expected results:

Use readable contract like the default text colour.

When a label has not been configured with a colour, the rendering of the text is considered with the background is RGB #000000 black and use a white colour for the text, but the background of the label is rendered the default GUI background tint which is usually light grey.

This is indeed a bug affecting all automatically created labels without a user configured colour.

This makes automatically created labels, totally illegible. (IMAPv4 or ATOM XML RSS tags or labels).

Component: Theme → Message Reader UI
OS: Unspecified → All
Hardware: Unspecified → Desktop
Summary: X-Mozilla-Keys display unreadable white text over light grey background → Auto labels use white text over default GUI background until configured

Generated HTML code for an automatically-generated label

<label xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
  value="database"
  class="tagvalue"
  style="color: white; background-color: ;"
  aria-label="Étiquettes: database"
/>

Generated inline style-sheet

color: white;
background-color: ;

As can be seen, newly generated labels without a specifically configured colour, defines no background-color, but still programmatically sets a white text colour.

Could be fixed with:

A proper behaviour change would be to not set the CSS color when background-color is not set either.

So that when label colour is not defined, it renders with the default GUI's text and background colours.

This fixes the issue by checking in isColorContrastEnough() if there is a background colour set. If not the default text colour (black) is used.

Assignee: nobody → richard.marti
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #9257172 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9257172 [details] [diff] [review]
1747909-tag-black-text.patch

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

::: mail/modules/TagUtils.jsm
@@ +108,5 @@
>  function isColorContrastEnough(aColor) {
> +  // Is a color set? If not, return "true" to use the default color.
> +  if (!aColor) {
> +    return true;
> +  }

This function is a bit funny, returning a Color() and not boolean. If anything it should return null for false. 
It's always checked for a falsy value though, so not clear to me how it fixes the bug.

But all caller of isColorContrastEnough() seems to check for boolean, or not? See https://searchfox.org/comm-central/search?q=isColorContrastEnough&case=false&regexp=false&path=

(In reply to Richard Marti (:Paenglab) from comment #5)

But all caller of isColorContrastEnough() seems to check for boolean, or not? See https://searchfox.org/comm-central/search?q=isColorContrastEnough&case=false&regexp=false&path=

The function is not funny at all, it always returns a boolean.

The other return point is this:

   return new Color(cR, cG, cB).isContrastRatioAcceptable(
     new Color(0, 0, 0),
     "AAA"
   );

It creates a new Color object instance from the R,G,B components of the label colour (which due to the 0 padding, makes it black when the background-color is not defined).
From this Color object instance, it calls the isContrastRatioAcceptable() method with a black Color object. The methods returns a boolean, so does the isColorContrastEnough method as well.

Comment on attachment 9257172 [details] [diff] [review]
1747909-tag-black-text.patch

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

Oh I see now. Makes sense! r=mkmelin
Attachment #9257172 - Flags: review?(mkmelin+mozilla) → review+
Target Milestone: --- → 97 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/b221272a0b63
Use in Tags black text when no background color is set. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED

Comment on attachment 9257172 [details] [diff] [review]
1747909-tag-black-text.patch

[Approval Request Comment]
User impact if declined: unreadable RSS auto tags
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Attachment #9257172 - Flags: approval-comm-esr91?
Attachment #9257172 - Flags: approval-comm-beta?

Comment on attachment 9257172 [details] [diff] [review]
1747909-tag-black-text.patch

[Triage Comment]
Approved for beta
Approved for esr91

Attachment #9257172 - Flags: approval-comm-esr91?
Attachment #9257172 - Flags: approval-comm-esr91+
Attachment #9257172 - Flags: approval-comm-beta?
Attachment #9257172 - Flags: approval-comm-beta+

This screenshot from Thunderbird Daily 97.0a1 (2022-01-04) (64-bit) just confirms the issue has been resolved with the intended result.

Attachment #9257759 - Flags: feedback+
Duplicate of this bug: 1745702
You need to log in before you can comment on or make changes to this bug.