Closed Bug 1569670 Opened 5 years ago Closed 5 years ago

low contrast listing chatroom participants

Categories

(Thunderbird :: Instant Messaging, enhancement, P2)

Product:
Thunderbird
Component:
Instant Messaging
Platform:
x86_64
Windows 10
Type:
enhancement

Tracking

(thunderbird_esr6868+ fixed, thunderbird69 fixed, thunderbird70 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 70.0
Tracking Flags:
Tracking Status
thunderbird_esr68 68+ fixed
thunderbird69 --- fixed
thunderbird70 --- fixed

People

(Reporter: vtol, Assigned: Paenglab)

Details

Attachments

(3 files, 1 obsolete file)

scrrenshot chatroom participants
30.57 KB, image/png
Details
Participant's list with active talker
14.30 KB, image/png
Details
1569670-specificity-inactive-participants.patch
1.50 KB, patch
Paenglab
: review+
jorgk-bmo
: approval-comm-beta+
jorgk-bmo
: approval-comm-esr68+
Details | Diff | Splinter Review
  • OS: W10 Pro x_64 v1903 b18362.267
  • TB: 69.0b1 (64-bit)
  • TB theme: dark || default with dark OS theme

The list of chatroom participants is almost illegible

Definitely seems non-ideal, but the participant names SHOULD be more legible once they actually speak. Are people who have spoken recently easier to read?

I'll attach a screenshot of how it looks on macOS light theme with some people who have talked recently.

Indeed, talking participants are coloured and easily distinguishable. However, if you want to find one that is not talkative at the time it is hard on the eyes, not to say strenuous and likely not in line with industry reommendations/standards.

https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-contrast.html

Alex / Richard, anything we can do here? We still want to be able to differentiate between "active" and "inactive" people, but the difference is quite hard to read.

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

probably broken by bug 1545398. The specificity of selector with the two classes was now not high enough to override the one from https://searchfox.org/comm-central/source/mail/components/im/themes/chat.css#738

Assignee: nobody → richard.marti
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(richard.marti)
Attachment #9081416 - Flags: review?(alessandro)
Comment on attachment 9081416 [details] [diff] [review]
1569670-specificity-inactive-participants.patch

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

It definitely looks way better.

If we want to be super picky, and I think we should, let's increase the opacity to 0.55,
in order to reach a color similar to #A2A2A5 and pass the AA accessibility test:
https://webaim.org/resources/contrastchecker/?fcolor=A2A2A5&bcolor=38383C

r+ after that.
Attachment #9081416 - Flags: review?(alessandro) → review+
Flags: needinfo?(alessandro)

Fixed the opacity.

Attachment #9081416 - Attachment is obsolete: true
Attachment #9081863 - Flags: review+
Comment on attachment 9081863 [details] [diff] [review]
1569670-specificity-inactive-participants.patch

Bug 1545398 landed on 68.
Attachment #9081863 - Flags: approval-comm-esr68?
Attachment #9081863 - Flags: approval-comm-beta?

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/574298993373
Increase the specificity of the themed inactive participants selector. r=aleca DONTBUILD

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

Prize question: How did I guess that you wanted this landed without being subscribed on the bug? And who set the target milestone to 69? Please don't do that.

Target Milestone: Thunderbird 69.0 → Thunderbird 70.0

(In reply to Jorg K (GMT+2) from comment #10)

And who set the target milestone to 69? Please don't do that.

:raised hand:

Reason being is perhaps ignorance about the code/patch development cycle - how it trickles down from Nightly to Beta.

Thus far my humble understanding would appear that someone would have to manually request an uplift for a patch that is tested in Nightly to Beta. If that request is not made the user having reported an issue for Beta will have to wait for the release cycle to complete in order for Nightly patches (that were reported for Beta) to morph into Beta - which could be weeks in worst case and thus leaving the Beta user in the lurch for as long.

Please correct me if that is the wrong assumption and that patches reported for Beta but being patched in Nighly get an automtic uplift after some time the patch been tested in Nightly. Or is someone assigned to track patch testing in Nightly whom then manually invokes the uplift?

Look at what happened with bug 1564443, being a case in point

(In reply to Jorg K (GMT+2) from comment #14)

Grrrr, this affects 69 and the developer/reviewer didn't request uplift :-( - Now we apparently have a beta where chat doesn't work. Hard to believe that slipped through testing.

The Target Milestone is the version at which the patch lands, in this case TB 70. It will be released in TB 70 beta and later TB 76 ESR if not uplifted.

To achieve uplift, the developer or reviewer should check whether the bug existed in previous versions. If that's not done, chances are that the person managing the code for releases doesn't notice, like it happened in bug 1564443. Usually that code manager watches everything that's going on, but he/she's not responsible if something gets missed that wasn't flagged.

Not only developers can request uplift, also interested third parties can, like the reporter.

As you said, there aren't any automatic uplifts, it's a manual process driven by the flags.

Having a wrong Target Milestone can be quite fatal since then we can't track in which version the bug was fixed. Certainly moving the Target Milestone backwards doesn't trigger any uplift. Besides, those tracking flags are not to be set by anyone else.

Thank you for the feedback/explanation, which of course is logical.

What is not clear to me, as being said third party, after which period of time that a patch is being tested in Nightly the uplift to Beta can be requested and whether that requires to open a new bug - considering like in this case the bug is being closed for the reported Beta whilst the patch is applied in Nightly?

Suppose I did not win the prize? ;)

The request to beta can be made at any time of course as a flag on the patch in the bug where it's landed as you can see above, not in a new bug like you tied in bug 1569604. The guy/girl managing the code will come to visit and uplift in due course.

tracking-thunderbird69: --- → ?

We're not tracking 69. Beta uplifts will happen in due course. Which part of "those tracking flags are not to be set by anyone else" is so hard to understand?

tracking-thunderbird69: ? → ---

Because it is comfusing for simple minds (being me)

  1. The request to beta can be made at any time of course as a flag on the patch in the bug where it's landed
  2. Not only developers can request uplift, also interested third parties can, like the reporter.
  3. those tracking flags are not to be set by anyone else
  4. The guy/girl managing the code will come to visit and uplift in due course.
  5. To achieve uplift, the developer or reviewer should check whether the bug existed in previous versions
  6. If that's not done, chances are that the person managing the code for releases doesn't notice

Sums up to

  1. you can request uplift but do not touch tracking flags/milestones
  2. if no tracking flag is set by whoever is elegible to set milestone/tracking flag the uplift maynot happen (bug 1564443) unless an observant third party issues an uplift request

From the outside that looks like a flawed process just waiting for issues like bug 1564443 to happen. There should be no need at all for a third party having to issues an uplift request, but that is just my five cents.

Let me explain how our "flawed process" has worked for the last couple of years:

We track patches in fixed bugs. If we have a patch that has landed, we can (naturally) backport it. Anyone can request that backport at any time, even before landing, but it seems practical to request it after review is granted. Corollary: If no one requests that uplift, it's likely forgotten.

For some important versions we track some important bugs that we may want to fix in that version. That's what the tracking flags are for. Unlike FF, we only track ESR numbers, so 10, 17, 24, 31, 38, 45, 52, 60, 68, 76. Setting tracking on TB 69 is pointless since no one will look at it.

Your summary 1-6 and 1. are correct, 2. doesn't seem to add any value. Again: Distinguish between uplift/backport of a patch and tracking of an unsolved bug.

(In reply to Jorg K (GMT+2) from comment #17)

For some important versions we track some important bugs that we may want to fix in that version. That's what the tracking flags are for. Unlike FF, we only track ESR numbers, so 10, 17, 24, 31, 38, 45, 52, 60, 68, 76.

Thanks for pointing that out - hardly known (tracking policy) to the uninitited (irgnorant) user

(In reply to Jorg K (GMT+2) from comment #17)

Corollary: If no one requests that uplift, it's likely forgotten.
Setting tracking on TB 69 is pointless since no one will look at it.

From a user's perspective, one that might not be able to contribute with code PR but is a at least inclined to support product development by reporting (potential) bugs (incl. the learning curve of do and do not), this makes it unattractive to deploy Beta and report discovered bugs. Better to stick either to Nightly (with the inherit risk of breakages/regressions being introduced) or wait for ESR.

I don't follow that. Beta is much more stable than Daily/Nightly. If you use beta and detect a bug that is fixed in Daily, you can request beta uplift and you'll get it in the next beta, assuming the approval is granted. Bugs in Daily are also not tracked, so why would using Daily bring any advantage?

The advantage is in the workflow

Daily

  • report potential bug
  • bug eventually gets patched / tested / released in the Daily branch
  • user gets patched code (rather immiditaely) with the next Daily build (and can check whether the reported bug is remedied)
  • Con - inherent risk to get caught up in breakages/regressions being introduced (and no way to rollback to a last known good build)

vs

Beta

  • report potential bug
  • bug eventually gets patched / tested / released in the Daily branch
  • user has to monitor whether the developer bothers with uplift to Beta (no tracking) and if not initiated by the developer the user has to request uplift to Beta, which then may or not may happen (at the discretion of the developer)
  • if the uplift happens the user has to wait for the next Beta build (weekly cycle?) until it can be checked whether the reported bug is remedied
  • if the uplift does not happen the user has to wait for Daily to morph into Beta, which could be weeks depending at which stage of the build cycle been reported / patched / tested / released
  • Pro - more stable code than Daily

vtol / Jorg -- I think the discussion about process is good to have, but can we move this to tb-planning or somewhere else? It is gumming up this particular bug! Thanks!

Attachment #9081863 - Flags: approval-comm-esr68?
Attachment #9081863 - Flags: approval-comm-esr68+
Attachment #9081863 - Flags: approval-comm-beta?
Attachment #9081863 - Flags: approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: