1751986 - Missing focus ring after Ctrl+click or Ctrl+Space* on selected contact in new Address Book

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect, P4)

Description

[Thomas D. (:thomas8)]

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

Seen in 98.0a1 (2022-01-25) (64-bit), Win10

In new AB, there's sometimes no focus ring for contact items which are focused, but not selected. So focus seemingly disappears to nowhere, but is actually sitting on a contact. Which is irritating and error-prone. This is also needed to fix bug 1751978.

*: Ctrl+Space doesn't work for selecting/deselecting a contact, that's bug 1751978, but would otherwise be another way to trigger this bug.

STR:

1. Open ("new") Address Book, and click back and forth on several ABs so that any selections and current index are gone (virgin list, important). Then click e.g. Personal AB.
2. Out of 3 subsequent contacts, A, B, C: click to select contact B
3. Hold Ctrl while clicking contact B again to deselect (on Windows), then move mouse away (bug: no focus ring remains)
4. press arrow down key (to find out where the invisible focus was)

Actual results:

• Contact B is correctly deselected after Ctrl+click (step 3)
• Contact B has focus (as shown in step 4), but it does not have a focus ring (this bug)
• Current focus is not visible anywhere (irritating and error-prone, this bug)
• Pressing arrow keys correctly starts out from invisibly focused item, so C is now focused and selected (ok)
  that's).

Expected results:

• after ctrl+click (or Ctrl+Space) on selected contact, the deselected contact should have a visible focus ring
• For the correct behaviour, try STR in contacts side bar of composition

Comment 1

[Thomas D. (:thomas8)]

[Fixed by related bug 1751294]
Another inconvenient variant of the same issue:

STR

• click back and forth on several ABs so that any selections are gone
• click into AB search field
• press Tab exactly twice
• try to spot where the focus is
• press arrow down

Actual

• no indication of focus whatsoever, but arrow keys reveal that focus is somewhere on top of the list

Expected

• if there's no selection, pressing tab into list must move focus ring on first contact (or perhaps on the contact which last had focus, as it does in contacts side bar although there are UI refresh glitches which require mouse moves to update the display).

Comment 3

[Thomas D. (:thomas8)]

This is still happening. Probably requires fiddling with current index for the first click on an item of a virgin list, similar to bug 1751294.
Note: You won't see this after using tab to go into the list. Clicking back and forth between ABs currently restores the virgin state which is needed here.

Comment 4

[Thomas D. (:thomas8)]

Mass-adding regression keyword to applicable AB bugs.

#20220530ABRegressionKeyword01

Comment 5

[Henry Wilkes (they/them) [:henry-x]]

Should be (partially) addressed by bug 1752532 because using a roving tab index will give the items actual :focus styling. EDIT: Does not apply to this bug.

Comment 6

[Henry Wilkes (they/them) [:henry-x]]

Is this actually a bug after bug 1751978 lands? Do you want the focus ring to be visible even when using only a mouse? If that is the case, then we should only need to change the :focus-visible selector to :focus instead.

Also, I don't think this is blocking bug 1751978, it seems adjacent.

Comment 7

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Henry Wilkes [:henry] from comment #6)

Is this actually a bug after bug 1751978 lands? Do you want the focus ring to be visible even when using only a mouse? If that is the case, then we should only need to change the :focus-visible selector to :focus instead.

I'm going to change this to use :focus instead. The treeview-listbox uses aria-activedecendent rather than a roving tab-index. I think you only switch from :focus:not(:focus-visible) to :focus-visible when the focus moves, but this doesn't happen when switching from clicking the list to using keyboard navigation because the focus remains on the treeview-listbox element, instead only aria-activedecentent changes. So to allow for this use case we'll just have to use :focus instead.

I might also adjust the focus-ring to remain visible when over selected items.

Comment 8

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1751986 - Use :focus instead of :focus-visible for treeview-listbox. r=aleca

We also make sure the focus outline is visible on selected items as well.

Depends on D149422

Comment 9

[Henry Wilkes (they/them) [:henry-x]]

Comment on attachment 9282178 [details]
Bug 1751986 - Use :focus instead of :focus-visible for treeview-listbox. r=aleca

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: No focus styling in the contacts list when switching from mouse to keyboard (unless you first tab the focus out and back).
Testing completed (on c-c, etc.): No
Risk to taking this patch (and alternatives if risky): Low. Just a CSS change.

Comment 10

[Wayne Mery (:wsmwk)]

Hasn't yet landed on nightly
To make it to beta we will want this to land on nightly before the merge, then eligble for esr102.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e1b1405403a2
Use :focus instead of :focus-visible for treeview-listbox. r=aleca

Comment 12

[Wayne Mery (:wsmwk)]

Comment on attachment 9282178 [details]
Bug 1751986 - Use :focus instead of :focus-visible for treeview-listbox. r=aleca

[Triage Comment]
Approved for esr102

Alex, when we get RC2 built on Monday can you test it, or delegate someone to test, that we don't have any regression from this?

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9282178 [details]
Bug 1751986 - Use :focus instead of :focus-visible for treeview-listbox. r=aleca

[Triage Comment]
not for beta, but for esr102

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 102.0:
https://hg.mozilla.org/releases/comm-esr102/rev/573d38d981d2

Comment 15

[Alessandro Castellani [:aleca]]

(In reply to Wayne Mery (:wsmwk) from comment #12)

Alex, when we get RC2 built on Monday can you test it, or delegate someone to test, that we don't have any regression from this?

Tested on 102. All looks and behaves correctly so far.

Comment 16

[Wayne Mery (:wsmwk)]

Tested on 102. All looks and behaves correctly so far.

Thanks for ensuring we're good