Closed Bug 1579364 Opened 7 months ago Closed 7 months ago

Tags: black text on red/green tag not correct

Categories

(Thunderbird :: Folder and Message Lists, defect)

defect
Not set

Tracking

(thunderbird_esr6869+ fixed, thunderbird70 fixed, thunderbird71 fixed)

RESOLVED FIXED
Thunderbird 71.0
Tracking Status
thunderbird_esr68 69+ fixed
thunderbird70 --- fixed
thunderbird71 --- fixed

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1534360 +++

I was just looking at bug 1578829, when I noticed the following:

TB 60:
tag red/green/blue/magenta(purpe) : white text
tag orange: black text

In TB 68 I see this:
tag blue/magenta(purpe) : all white text
tag red/green/orange: black text

Black on red/green is hard to read.

I'm pretty sure that after bug 1534360 we had it correctly, so only black on orange, but now it's changed again. Well, I'm wrong: I tried the Dailies or 2019-04-04 after bug 1534360 and 2019-04-05 after bug 1541829 and they have black on red/green.

I also have a tag of RGB (200,50,200) (purple) and the text is still shown as black.

I added debug to
https://searchfox.org/mozilla-central/rev/2c0a60b5b065ae0fdfcfae168141e82b834fb2d4/toolkit/modules/Color.jsm#74

  get useBrightText() {
    dump(`=== ${this.r} ${this.g} ${this.b} ${this.relativeLuminance}\n`);
    return this.relativeLuminance <= CONTRAST_BRIGHTTEXT_THRESHOLD;
  }

And I see:
Red tag === 255 0 0 0.2126
Green tag === 0 153 0 0.2278246557150657
Blue tag === 51 51 255 0.10291460242446715
My purple tag === 200 50 200 0.18730695228818234

The threshold is at approx. 0.179, so only for blue we use white text colour.

Richard, can you confirm that the current behaviour is wrong?

Mike, is this the expected behaviour or is there anything wrong in the calculation?

Or am I missing something? We fixed bug 1534360 which complained about the blue tag, but in TB 60 all text colours for red/green/blue/magenta were white. Now it's only white on blue/magenta.

Flags: needinfo?(richard.marti)
Flags: needinfo?(mdeboer)

Yes, I can confirm this. But note, in TB 60 the text colours aren't calculated but hard coded in tagColors.css.

Flags: needinfo?(richard.marti)

How about raising the threshold to 0.23 instead of 0.179. That white text would be used for red/green and "my" purple.

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

Yes, I can confirm this. But note, in TB 60 the text colours aren't calculated but hard coded in tagColors.css.

I'd like to first understand which part is hardcoded and which part isn't, before we dive into the calculations again. They were wrong before, indeed, but I'm quite sure they're correct now. The 0.179 threshold is derived from Math.sqrt(1.05 * 0.05) - 0.05, since we're always comparing contrast ratios of black vs. white text. So using a different constant here would essentially mean you're moving away from this boolean comparison towards different text colors.

If you want higher granularity to check different contrast types, you could use var useBrightText = !backgroundColor.isContrastRatioAcceptable(new Color(0, 0, 0), "AAA"); - Where "AA" might already be enough contrast for tags in TB.

Flags: needinfo?(mdeboer)

(In reply to Mike de Boer [:mikedeboer] from comment #3)

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

Yes, I can confirm this. But note, in TB 60 the text colours aren't calculated but hard coded in tagColors.css.

I'd like to first understand which part is hardcoded and which part isn't, before we dive into the calculations again.

In TB 68 and up we use only the calculation, no hardcoding.

Mike, thanks for the comment. As Richard, said, we use useBrightText() here:
https://searchfox.org/comm-central/rev/a2afcaf776f3bbff0fe6c2db4c4190942aadca12/mail/base/modules/TagUtils.jsm#107

isContrastRatioAcceptable() works better, I'll attach a patch.

This gives the backgrounds we are used to. Only black text on orange, the rest white text.

Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #9093265 - Flags: review?(richard.marti)
Comment on attachment 9093265 [details] [diff] [review]
1579364-tag-background.patch

That works, thanks.
Attachment #9093265 - Flags: review?(richard.marti) → review+

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/5f512492b2eb
Fix tag background by using isContrastRatioAcceptable() instead of useBrightText(). r=Paenglab

Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 71.0
Version: 66 → 68
Attachment #9093265 - Flags: approval-comm-esr68+
Attachment #9093265 - Flags: approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.