tree-listbox should have ARIA role of "tree" for addressbook and account manager
Categories
(Thunderbird :: General, defect)
Tracking
(thunderbird_esr102 fixed, thunderbird102 affected)
People
(Reporter: henry-x, Assigned: henry-x)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
text/x-phabricator-request
|
wsmwk
:
approval-comm-beta-
wsmwk
:
approval-comm-esr102+
|
Details |
(deleted),
text/x-phabricator-request
|
wsmwk
:
approval-comm-beta-
wsmwk
:
approval-comm-esr102+
|
Details |
The addressbook tree and account manager trees should have role="tree"
.
Rather than wait for Bug 1752532, we can modify the existing tree-listbox class to use the correct roles for 102
Assignee | ||
Comment 1•2 years ago
|
||
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/2b88e08e67f3
Use ARIA role of "tree" for the addressbook and account manager trees. r=aleca,darktrojan
Updated•2 years ago
|
Comment 3•2 years ago
|
||
Somehow you've broken browser_calendarList.js. Failure log.
From a quick glance, it seems like the selectedIndex
is 0 when it should be 1 after this delete and the wrong calendar gets deleted.
Assignee | ||
Comment 4•2 years ago
|
||
(In reply to Geoff Lankow (:darktrojan) from comment #3)
Somehow you've broken browser_calendarList.js. Failure log.
I tracked it down to the added whitespace here https://hg.mozilla.org/comm-central/rev/2b88e08e67f3#l1.14
This extra whitespace was acting as a MutationRecord.previousSibling
here https://searchfox.org/comm-central/rev/232c80b88b0dcd6c874ff0d754215df78e4f020e/mail/base/content/widgets/tree-listbox.js#292
That code is too fragile. I'll try and strengthen it rather than just remove the whitespace.
I also noticed that the current implementation tries to select the previous index https://searchfox.org/comm-central/rev/232c80b88b0dcd6c874ff0d754215df78e4f020e/mail/base/content/widgets/tree-listbox.js#275 but the convention is to select the same index. I can fix that here as well.
Also, looking through the code, this line is setting the selectedIndex
when a calendar is about to be removed https://searchfox.org/comm-central/rev/232c80b88b0dcd6c874ff0d754215df78e4f020e/calendar/base/content/calendar-management.js#345 rather than just letting the widget handle this. @darktrojan Do you know why it is changing the selection?
Comment 5•2 years ago
|
||
Probably because that code is much older than the widget it now controls. If it's getting in the way, change it.
Comment 6•2 years ago
|
||
I'm going to remove the offending white space to clear the failing test.
Assignee | ||
Comment 8•2 years ago
|
||
The convention from xul:tree and xul:listbox is to select the next item that was not deleted. We also simplify the logic by using selectedRow (an Element) rather than selectedIndex.
Assignee | ||
Updated•2 years ago
|
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/73636ef4138a
Refactor tree-listbox selection code when selected item is removed. r=darktrojan
Assignee | ||
Comment 10•2 years ago
|
||
Comment on attachment 9282195 [details]
Bug 1775245 - Use ARIA role of "tree" for the addressbook and account manager trees. r=aleca,darktrojan
[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Wrong role announced by screen reader, and confusing presence of listboxes within listboxes.
Testing completed (on c-c, etc.): Tested on Daily with Orca
Risk to taking this patch (and alternatives if risky): Medium. This does some refactoring.
NOTE: Also want revision 5f7e28323e23 (see comment 6)
Assignee | ||
Comment 11•2 years ago
|
||
Comment on attachment 9284730 [details]
Bug 1775245 - Refactor tree-listbox selection code when selected item is removed. r=darktrojan
[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Previous code was fragile and would break under certain conditions (e.g. whitespace, or removing two items). Plus, the behaviour when deleting an item is not consistent with what is planned in bug 1752532
Testing completed (on c-c, etc.): Passed try-comm-central tests.
Risk to taking this patch (and alternatives if risky): Medium. Required refactoring.
Comment 12•2 years ago
|
||
Comment on attachment 9282195 [details]
Bug 1775245 - Use ARIA role of "tree" for the addressbook and account manager trees. r=aleca,darktrojan
[Triage Comment]
Next week is the merge, making beta 104. So an uplift to beta won't be happening.
Comment 13•2 years ago
|
||
Comment on attachment 9284730 [details]
Bug 1775245 - Refactor tree-listbox selection code when selected item is removed. r=darktrojan
[Triage Comment]
Assignee | ||
Comment 14•2 years ago
|
||
Comment on attachment 9282195 [details]
Bug 1775245 - Use ARIA role of "tree" for the addressbook and account manager trees. r=aleca,darktrojan
[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Wrong role announced by screen reader, and confusing presence of listboxes within listboxes.
Testing completed (on c-c, etc.): Tested with Orca
Risk to taking this patch (and alternatives if risky): Medium. This does some refactoring.
NOTE: Also want revision 5f7e28323e23 (see comment 6)
Assignee | ||
Comment 15•2 years ago
|
||
Comment on attachment 9284730 [details]
Bug 1775245 - Refactor tree-listbox selection code when selected item is removed. r=darktrojan
[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Previous code was fragile and would break under certain conditions (e.g. whitespace, or removing two items). Plus, the behaviour when deleting an item is not consistent with what is planned in bug 1752532
Testing completed (on c-c, etc.): Passed try-comm-central tests.
Risk to taking this patch (and alternatives if risky): Medium. Required refactoring.
Comment 16•2 years ago
|
||
Comment on attachment 9284730 [details]
Bug 1775245 - Refactor tree-listbox selection code when selected item is removed. r=darktrojan
[Triage Comment]
Approved for esr102
extra smoke testing of 102.1.1 would be good for address book
Comment 17•2 years ago
|
||
Comment on attachment 9282195 [details]
Bug 1775245 - Use ARIA role of "tree" for the addressbook and account manager trees. r=aleca,darktrojan
[Triage Comment]
Approved for esr102
Comment 19•2 years ago
|
||
bugherder uplift |
Comment 20•2 years ago
|
||
(In reply to Wayne Mery (:wsmwk) from comment #16)
extra smoke testing of 102.1.1 would be good for address book
I actually did smoketesting of 102.1.1 two days after this needinfo request, tested AB, and saw a bug, see my email message to beta testers dated 2022-08-05, 21:26 CEST:
SUCCESS
- menus
- setup gmx-imap, connect Carddav-AB, caldav-cal
- setup gmail-imap, get messages,
- read and reply/reply-all,
- open attachments (incl. PDF), save attachments, drag attachments out to OS folder,
- compose with attachments, drag attachments into composition, including inline image
- Calendar: CALDAV event creation and edit,
- Address Book: CardDAV card creation and edit
After adding a birthday 29-Feb without year to a CardDAV card, it disappeared from card display and on next edit, not found.
This works for me now on 102.1.2 (64-bit), Win10 - we've fixed birthdays without year in Bug 1779100 - Re-allow entering a birthday or special date without year (avoid ux-interruption, also for migrated contacts w/o year).
Today I've tested the behaviour of deleting contacts on 102.1.2 (64-bit), Win10 and as a result, filed
Bug 1785314 - After deleting a contact, first contact in list gets selected and search box is focused
(although maybe this bug was only affecting deleting address books?)
Description
•