Convert message header XUL custom elements to HTML custom elements
Categories
(Thunderbird :: Message Reader UI, task, P1)
Tracking
(thunderbird_esr91 wontfix)
Tracking | Status | |
---|---|---|
thunderbird_esr91 | --- | wontfix |
People
(Reporter: aleca, Assigned: aleca)
References
Details
Attachments
(9 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
Those custom elements have a very convoluted structure and they create a bunch of problems when resizing the window.
Let's clean them up and convert them into simpler HTML elements.
Wasn't something like this already done here in bug 1589861 comment #32 and later backed out? Bug 1589861 comment #58 said: "realistically much of this is going to be very similar in the end".
Assignee | ||
Comment 2•3 years ago
|
||
Similar but done differently and a bit better.
That patch was backed out because it created a bit of a mess with the grid structure of the message header.
This bug will only exclusively focus on those custom elements, without changing the HTML structure of the message header, or the labels.
Assignee | ||
Comment 3•3 years ago
|
||
Assignee | ||
Comment 4•3 years ago
|
||
I started working on this and decided to upload an early WIP patch without going to forward before gathering some feedback.
I'm focusing on the rows that need to handle multiple addresses as well as the more button.
The initial idea is to have this type of structure:
div
ul
li > address
li > address
...
button.more
Does this make sense semantically?
Does using the <address>
element makes sense to wrap the email address?
Any suggestions or recommendations?
Assignee | ||
Updated•3 years ago
|
Comment 5•3 years ago
|
||
I would use <ol> instead of <ul> since it's (loosely) ordered addresses.
You can use <address>, but there might not be a need for it. If we can get by with simply using the link, then the extra box wouldn't add any value.
Comment 6•3 years ago
|
||
I think the proposal would work well for a static document, but these elements require some interactivity. My initial thought is we want:
- Some sort of list.
- Each address has some interactive controls, which require focus.
- Ideally we could combine the addresses into a single widget that controls the focus, so each address isn't a focus stop.
- We might want to allow for multi-selection (e.g. to address https://thunderbird.topicbox.com/groups/ux/Tfec8f7aebbb89bdc).
- We might want to combine "To", "Cc", "Reply-To", etc into a single widget.
Therefore, if the list was fully expanded, I think the "Grouped lists" widget I'm planning in bug 1752532 comment 9 might work quite well. Specifically, it would look like
<ul is="grouped-list" role=listbox">
<li role="none">
<span id="toHeading">To</span>
<ul role="group" aria-labeledby="toHeading">
<li role="option">first@server</li>
<li role="option">second@server</li>
</ul>
</li>
<li role="none">
<span id="ccHeading">CC</span>
<ul role="group" aria-labeledby="ccHeading">
<li role="option">third@server</li>
</ul>
</li>
<li role="none">
<span id="replyToHeading">Reply-To</span>
<ul role="group" aria-labeledby="replyToHeading">
<li role="option">reply@server</li>
</ul>
</li>
</ul>
However, I'm not sure how well this would work in the "collapsed" state shown here https://bug1556261.bmoattachments.org/attachment.cgi?id=9243994 We could try putting the "3 Cc", "1 Reply to" and "more" buttons as list "option"s in no group at the end and see how that runs on a screen reader.
This would mean that bug 1752532, which would provide the API to construct such a structure, would be blocking. But we could work together to try and synchronise what API you would need for this widget.
Regarding using a wrapping <address>
. I don't think it is quite appropriate. It seems to be more limited in scope than its name might imply: it is meant to be linked to an article
or body
https://html.spec.whatwg.org/multipage/sections.html#the-address-element
Assignee | ||
Comment 7•3 years ago
|
||
Thanks both for the valuable feedback.
(In reply to Henry Wilkes [:henry] from comment #6)
However, I'm not sure how well this would work in the "collapsed" state shown here https://bug1556261.bmoattachments.org/attachment.cgi?id=9243994 We could try putting the "3 Cc", "1 Reply to" and "more" buttons as list "option"s in no group at the end and see how that runs on a screen reader.
I'm gonna temporarily defer this change (collapsing all recipients row into 1) for later since it needs more work to guarantee proper a11y.
So we can start with a simpler structure for now and improve it later.
This would mean that bug 1752532, which would provide the API to construct such a structure, would be blocking. But we could work together to try and synchronise what API you would need for this widget.
I would love to use the new widget from that bug, so definitely that's the final objective.
For now, I think I'll implement a simpler structure to move things forward, and later we can adapt it to the new widget in order to not being a blocker.
This is the structure I will use for each recipient row:
<div>
<span id="toHeading">To</span>
<ul aria-labeledby="toHeading">
<li>first@server</li>
<li>second@server</li>
</ul>
</div>
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Pushed by alessandro@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/f4fb3a75992a
Replace XUL mail-multi-emailheaderfield, mail-headerfield, and mail-newsgroup with HTML custom elements. r=mkmelin,henry
Assignee | ||
Comment 9•3 years ago
|
||
- Improves the hover state of the address book indicator
- Use a consistent outline on :focus-visible
- Remove outline when :hover and :focus-visible for all elemenets
Comment 10•3 years ago
|
||
Comment 11•3 years ago
|
||
Assignee | ||
Comment 12•3 years ago
|
||
Comment 13•3 years ago
|
||
Reminder that we need to prevent focus being lost when the recipient list is refreshed. In particular when the MORE button is pressed. https://phabricator.services.mozilla.com/D144358#inline-803010
@aleca Have you already written a patch for this? Otherwise I could put one together.
Another thing I noticed is that we're setting an aria-label
on the header-recipient
list items using the fullAddress
https://hg.mozilla.org/comm-central/rev/f4fb3a75992a#l10.242 However, the fullAddress
is not necessarily the displayed text and it is missing the "(not) in address book" information. I think your previous approach of setting aria-label
on the <ol>
element worked better and saved reading out the field each time for every item in the list. I imagine the reason you changed it was because this was the old behaviour and one of the tests was testing for it. However, we now have the added context of a "list" which we did not have before.
So Basically, in version 91 each Tab stop for a list of two recipienst would read as
- "To: My Contact <my.contact@server.org>",
- "Address is in the addressbook",
- "To: second@server.org",
- "Address is not in the addressbook".
Note that in 91, an address in the addressbook was still shown as the full address, rather than the contact name, so the accessible name matched the displayed text.
Currently, each Tab stop reads as
- "List with 2 items. To: My Contact <my.contact@server.org>. One of two.",
- "To: second@server.org. Two of two".
And with aria-label
only on the <ol>
, each Tab stop would instead reads as
- "To. List with 2 items. My Contact. In the address book. One of two.",
- "second@server.org. Not in the addressbook. Two of two".
The latter correspond to what a we display visually, and takes the semantics into account.
If you want, I can put together a patch for this as well.
Assignee | ||
Comment 14•3 years ago
|
||
(In reply to Henry Wilkes [:henry] from comment #13)
Reminder that we need to prevent focus being lost when the recipient list is refreshed. In particular when the MORE button is pressed. https://phabricator.services.mozilla.com/D144358#inline-803010
@aleca Have you already written a patch for this? Otherwise I could put one together.
I haven't written anything for that front.
If you have the bandwidth, it would be great.
Another thing I noticed is that we're setting an
aria-label
on theheader-recipient
list items using thefullAddress
https://hg.mozilla.org/comm-central/rev/f4fb3a75992a#l10.242 However, thefullAddress
is not necessarily the displayed text and it is missing the "(not) in address book" information. I think your previous approach of settingaria-label
on the<ol>
element worked better and saved reading out the field each time for every item in the list. I imagine the reason you changed it was because this was the old behaviour and one of the tests was testing for it. However, we now have the added context of a "list" which we did not have before.
I changed that to respect the "accessible" strings we were expecting from the tests.
That was a temporary step, indeed we need to improve the whole area and now we have the flexibility to do so (another patch to convert the remaining XUL headers is coming).
So Basically, in version 91 each Tab stop for a list of two recipienst would read as
- "To: My Contact <my.contact@server.org>",
- "Address is in the addressbook",
- "To: second@server.org",
- "Address is not in the addressbook".
Note that in 91, an address in the addressbook was still shown as the full address, rather than the contact name, so the accessible name matched the displayed text.
Currently, each Tab stop reads as
- "List with 2 items. To: My Contact <my.contact@server.org>. One of two.",
- "To: second@server.org. Two of two".
And with
aria-label
only on the<ol>
, each Tab stop would instead reads as
- "To. List with 2 items. My Contact. In the address book. One of two.",
- "second@server.org. Not in the addressbook. Two of two".
The latter correspond to what a we display visually, and takes the semantics into account.
If you want, I can put together a patch for this as well.
That would be awesome!
Assignee | ||
Comment 15•3 years ago
|
||
Henry, if you implement the persistent tab focus, keep in mind that we need to make those toolbar buttons accessible in the focus ring.
The initial idea is:
- First focus lands on the first button
- Use arrows or Home/End to move between buttons
- Tab moves the focus to the adjacent element (not button)
Very similar to the spaces toolbar implementation.
Also, something that needs a lot of improvement is how to handle the focus on the first place.
- Should the focus first reach the whole header to we can feed screen readers a summarized overview of sender + title + actions?
- What's the correct tab cycle? Sender buttons > recipients, or Buttons > sender > recipients
- Do we want to allow moving the focus to the address book indicator icon of a recipient? The whole recipient is focused via Tab but pressing arrow left/right will move the focus between the text content and the AB indicator?
Of course all of this doesn't need to happen all at once, and we can postpone some of this more fine tuned improvements after 102, considering that the message header as always been in this pretty horrible state for years.
Updated•3 years ago
|
Comment 16•3 years ago
|
||
(In reply to Alessandro Castellani [:aleca] from comment #15)
Henry, if you implement the persistent tab focus, keep in mind that we need to make those toolbar buttons accessible in the focus ring.
The initial idea is:
- First focus lands on the first button
- Use arrows or Home/End to move between buttons
- Tab moves the focus to the adjacent element (not button)
Very similar to the spaces toolbar implementation.
I'm not working on this in my upcoming patch, I'm just going to stop the focus being lost when you click the MORE button.
But this is what should happen in the future.
Also, something that needs a lot of improvement is how to handle the focus on the first place.
- Should the focus first reach the whole header to we can feed screen readers a summarized overview of sender + title + actions?
I don't think so. Just like that the screen reader will not just read out all the content of the focused element, it will only read its accessible name, and reading out the entire content would be bad anyway.
- What's the correct tab cycle? Sender buttons > recipients, or Buttons > sender > recipients
I have been thinking in the back of my mind about what this area would look like semantically. This header seems to have developed from a mix of information shoved in places where there was a gap, so it is not easily dissected. But currently, I'm thinking the structure, in DOM order, should be
- The button toolbar.
- A listbox widget that contains all the emails grouped under "From", "To", "Cc", etc. See comment 6. Maybe use a grid if we want to separate the name from the addressbook icon, but this might be over complicating it.
- A vertical grid widget with a heading and value cell on each row to collect all the other header fields. Maybe include the time, tags and the encryption "button", but I'm unsure about this.
- The subject as a heading element that labels the document. I'll have to think about this last one some more, partly because a focusable heading might be odd.
- Do we want to allow moving the focus to the address book indicator icon of a recipient? The whole recipient is focused via Tab but pressing arrow left/right will move the focus between the text content and the AB indicator?
No, I don't think it should receive focus. The addressbook indicator's click behaviour is redundant because it essentially just activates the first item in the popup menu that appears if you click anywhere else on the recipient. I personally think it should just be a non-interactive icon with the title "In the addressbook" and "Not in the addressbook", it just doesn't seem particularly useful as a distinct interactive part which wouldn't necessarily require more usage than anything else in the popup.
Comment 17•3 years ago
|
||
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 18•3 years ago
|
||
All sounds good from comment 16, I agree entirely.
We can tackle this after 102 and have a little session to define scope and implementation strategy with another developer so we can divide and conquer this area.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 19•3 years ago
|
||
Try run with the addition of tests to cover the customization options: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=863d13d09b2053c13da223a7e049dc9d5c61889f
Assignee | ||
Comment 20•3 years ago
|
||
Comment 21•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/be1c2780ec01
Fix hover sate of address book indicator and other CSS improvements in message header. r=henry,Paenglab
Assignee | ||
Comment 22•3 years ago
|
||
Waiting for the try run to be good before marking this for check-in:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=d2307b5ff33554d4012dfd1c8a09ff06d82dc248
Assignee | ||
Updated•3 years ago
|
Comment 23•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/b0b210928557
Add tests for message header customization. r=darktrojan
https://hg.mozilla.org/comm-central/rev/be7bfd24a7ad
Preserve focus when activating the MORE recipients button. r=aleca
https://hg.mozilla.org/comm-central/rev/14b42dad7e2b
Replace tags XUL custom elements with HTML in the message header. r=mkmelin
Assignee | ||
Comment 24•3 years ago
|
||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Comment 25•3 years ago
|
||
We can use the semantic structure to simply the screen reader output and make it correspond to the visually displayed text.
Comment 26•3 years ago
|
||
Depends on D147298
Assignee | ||
Updated•3 years ago
|
Comment 27•2 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/5b2ca5b3d489
Change accessible naming for message header lists. r=aleca
https://hg.mozilla.org/comm-central/rev/f63f329f0a8e
Patch up the read name for simple message headings (for Orca). r=aleca
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Comment 28•2 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/f55176d77e75
Replace website and message ids XUL custom elements with HTML in the message header. r=darktrojan
Updated•2 years ago
|
Assignee | ||
Comment 29•2 years ago
|
||
There's a strange inconsistency in the "From" field when an email comes from a forum or mailing list, and that mailing list is saved in the address book.
On 91, the header reads "Alessandro from Thunderbird Team <team@thuunderbird.net>"
On 102, the header reads "Thunderbird Team"
I fixed this in the upcoming patch on bug 1556261.
Assignee | ||
Comment 30•2 years ago
|
||
I found the "problem" which is not really a problem.
In 91 when adding a contact to the Address Book we weren't populating the display name of the card right away, but only saving the email.
In my patches I added the display name autodiscovery and if we have one I save it in the ABCard right away.
The display name for the team mailing list is "Thunderbird Team", since that's what topicbox pushes.
Deleting the "Display Name" of the saved mailing list makes the full address appear.
In my upcoming patch of bug 1556261 I'm adding an option to allow users to always show the "Full address (name + email) for the sender". That will take care of it and give the option to the user to control the outcome.
Description
•