#47147 closed defect (bug) (fixed)
Status message not exposed to assistive technologies (in the Image Editor)
Reported by: | anevins | Owned by: | afercia |
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Media | Keywords: | has-screenshots wpcampus-report has-patch commit has-dev-note |
Focuses: | ui, accessibility | Cc: |
Description (last modified by )
Moved from the WPCampus accessibility report issues on GitHub, see: https://github.com/WordPress/gutenberg/issues/15293
- Severity:
- Medium
- Affected Populations:
- Blind
- Low-Vision
- Cognitively Impaired
- Platform(s):
- Windows - Screen Reader
- Mac - VoiceOver
- Android - TalkBack
- iOS - VoiceOver
- Components affected:
- Edit Media
Issue description
Within the Edit Media page, when activating the "Restore Image"
button, a message is shown above the image while the Restore button
itself disappears.
Since the button would have been focused at the time when activated by
keyboard, this causes the keyboard focus position to be lost and reset
to the top of the page.
The message itself is also not announced by screen readers, and may not
be visible to screen magnification users if it appears outside their
current view.
Issue Code
<div class="imgedit-panel-content wp-clearfix"> <div class="error"> <p>Cannot save image metadata.</p> </div> <div class="imgedit-menu wp-clearfix"> ... </div> </div> ... <div class="imgedit-panel-content wp-clearfix"> <div class="updated"> <p>Image restored successfully.</p> </div> <div class="imgedit-menu wp-clearfix"> ... </div> </div>
Remediation Guidance
Add tabindex="-1" to the message, and then programatically focus
it when it appears. This will maintain a logical focus position, and
ensure that all visual users will see it, while assistive technologies
announce it.
Adding the alert role will also help to convey to screen readers
that this is an alert-type message.
Recommended Code
<div class="imgedit-panel-content wp-clearfix"> <div class="error" tabindex="-1" role="alert"> <p>Cannot save image metadata.</p> </div> <div class="imgedit-menu wp-clearfix"> ... </div> </div> ... <div class="imgedit-panel-content wp-clearfix"> <div class="updated" tabindex="-1" role="alert"> <p>Image restored successfully.</p> </div> <div class="imgedit-menu wp-clearfix"> ... </div> </div>
Relevant standards
- 2.1.1 Keyboard (Level A)
Note: This issue may be a duplicate with other existing accessibility-related bugs in this project. This issue comes from the Gutenberg accessibility audit, performed by Tenon and funded by WP Campus. This issue is GUT-73 in Tenon's report
Attachments (4)
Change History (48)
This ticket was mentioned in Slack in #core-media by anevins. View the logs.
5 years ago
#6
@
5 years ago
@afercia it seems that we're talking about 2 things;
1) Moving the keyboard focus from the "Restore Image" button which disappears visually;
2) Putting alert roles or status on the notifications
If there's a way to tie into the functionality that hides the submit button, that would be easiest for the focus handling bit
jQuery('.submit').on('click', function() { $(this).hide(); $('.notification').attr('tabindex', '-1').focus(); });
#7
@
5 years ago
@anevins it's indeed two issues.
Currently, when pressing "Restore Image", focus is moved to the first focusable element within the image editor container which is... the "Scale image" help button:
Worth reminding the media editor is used also within the media modal:
Not ideal, can be certainly changed but the image editor code is pretty old and moving focus to the first focusable element seemed a good idea. In the case of notices appearing in the page, it should behave as suggested above. However, I'd tend to think we should favor a generalic, reusable, solution rather than ad-hoc implementations.
Relevant code is in src/js/_enqueues/lib/image-edit.js
.
The second issue are the status notices that appear on the page. Instead of using a role=alert, I'd recommend to use wp.a11y.speak()
which is the core implementation of ARIA live regions. Passing the assertive
parameter to speak()
would produce the equivalent announcement of an ARIA alert.
This ticket was mentioned in Slack in #core-media by anevins. View the logs.
5 years ago
This ticket was mentioned in Slack in #core-media by anevins. View the logs.
5 years ago
#10
@
5 years ago
- Keywords has-patch needs-testing added; needs-patch removed
@antpb I just uploaded a patch that will handle the core issue. Screen readers now announce all of the "status updates" that can come from editing a piece of media.
I didn't implement the focus aspect, as that collides with a focus set for [imgLoaded](https://github.com/WordPress/WordPress/blob/master/wp-admin/js/image-edit.js#L611)
Can you take a look and hopefully we can get this in before the beta 2 for 5.3?
This ticket was mentioned in Slack in #core-media by mike. View the logs.
5 years ago
#12
@
5 years ago
@ryanshoover Thanks so much for the patch!
This ticket came up in triage. Would you like to be listed as the owner?
#14
@
5 years ago
@mikeschroder Sure thing. I can own this. Doesn't look like I can assign myself :)
#16
@
5 years ago
@joemcgill @antpb Just double checking that the media team can look at this patch in time for 5.3 :)
#17
@
5 years ago
I think this looks great @ryanshoover ! Thank you so much! Patch applied clean and I'm running tests now. Will add commit
and get a second commiter to review after I test.
#19
@
5 years ago
- Keywords commit removed
Ah removing commit, we may need to factor in the suggestion here: https://core.trac.wordpress.org/ticket/47147#comment:7
"
Instead of using a role=alert, I'd recommend to use wp.a11y.speak() which is the core implementation of ARIA live regions. Passing the assertive parameter to speak() would produce the equivalent announcement of an ARIA alert.
"
This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.
5 years ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
5 years ago
This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.
5 years ago
#23
@
5 years ago
- Keywords early added
- Milestone changed from 5.3 to 5.4
Hi,
Thanks for the work done on this ticket.
Unfortunately, we are now at Beta 3 in the release cycle. I'm moving this ticket to 5.4 early
milestone.
Feel free to discuss it if you feel this ticket could land before 5.3 Release Candidate 1.
#24
@
5 years ago
Just a quick note that I think this is totally fine to land in 5.3 still, if someone has time to work on it.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
5 years ago
This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.
5 years ago
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
5 years ago
This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.
5 years ago
#29
@
5 years ago
- Milestone changed from 5.4 to 5.5
Howdy,
This ticket still needs a patch and 5.4 beta 1 is tomorrow. Let's move this ticket to 5.5.
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
5 years ago
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
5 years ago
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
5 years ago
#35
@
4 years ago
- Keywords needs-patch added; has-patch needs-refresh removed
Testing a bit the current approach, I'm not sure we can use the role="alert"
pattern. Due to the way the image editor works (which is a bit inconsistent and shows its age) the alert is sometimes announced and sometimes not.
For example, testing with Firefox and NVDA:
- the alert is announced when saving "normal" actions
- it is not announced when scaling or restoring an image or in any other case where the AJAX response rebuilds the whole image editor HTML
Screen readers are able to announce an alert when it's "injected" in the DOM. But when there's a large DOM update and the updated region contains an alert, it seems they're not able to understand what is going on. The DOM update is just too large. I think a more solid approach would be using wp.a11y.speak()
instead.
Moreover, as it's pretty common in the legacy WordPress codebase, most of the involved functions "do something" but then also return a value. For example:
wp_save_image()
processes the image saving and then returns an object with some image data and the error / success messagewp_restore_image
restores the image data and then then returns the error / success messagewp_image_editor()
generates the whole Image Editor markup, which is returned by te main AJAX actionimage-editor
Additionally, looking at this switch statement in the image-editor
AJAX action: https://github.com/WordPress/wordpress-develop/blob/25133cf3f1bc2ac3c2294a291396ebddb80b0e98/src/wp-admin/includes/ajax-actions.php#L2603-L2618
- when saving: the
wp_save_image()
message object is passed as JSON encoded response - when scaling: the
wp_save_image()
message object is not passed as JSON encoded response, instead it's passed towp_image_editor()
which returns a response containing the whole HTML blob - when restoring: the
wp_restore_image()
message object is not passed as JSON encoded response, instead it's passed towp_image_editor()
which returns a response containing the whole HTML blob
Basically the image-editor
AJAX action response is different depending on the cases. Sometimes it contains a message that could be used on the JS side by wp.a11y.speak()
. Sometimes it just returns the HTML blob.
In order to use wp.a11y.speak()
reliably, the response should always return an object that contains also a message. This would require some refactoring of the (pretty old) code responsible for the various responses. Will attempt a patch.
#36
@
4 years ago
- Summary changed from Status message not exposed to assistive technologies to Status message not exposed to assistive technologies (in the Image Editor)
I'm attaching an animated GIF to better illustrate the role="alert"
approach doesn't work (when restoring) even when setting focus on the alert (because of the too large DOM update). Tested with Firefox and NVDA. Focus on the NVDA speech viewer, where it's clear the alert isn't announced automatically.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
4 years ago
#39
follow-up:
↓ 41
@
4 years ago
- Keywords has-patch needs-dev-note added; needs-patch removed
47147.diff takes a broader approach and does the following:
- changes the response format of
wp_ajax_image_editor()
so that it is possible to get the messages and pass them towp_send_json_error()
andwp_send_json_success()
- in order to do so, it uses output buffering to split the HTML from the messages
- I'd like the part above to be reviewed by the Media component maintainers or alternatively from the release Core Tech, /Cc @azaozz @SergeyBiryukov
Additionally:
- uses
wp.a11y.speak()
to announce messages to assistive technology - announce existing messages for the
scale
andrestore
actions - uses wp-i18n and entirely removes
imageEditL10n
- improves focus management: focus is moved to the admin alert notices, if any, or to the first tabbable element within the Image editor
Hint: To test the accessibility part of the patch and see when focus is set to the admin alert notices, a temporary style for focus may help. You can add somewhere in some CSS used on the page something along these lines:
:focus { outline: 2px solid red !important; }
This ticket was mentioned in Slack in #core by afercia. View the logs.
4 years ago
#41
in reply to:
↑ 39
@
4 years ago
Replying to afercia:
47147.diff takes a broader approach and does the following:
- changes the response format of
wp_ajax_image_editor()
so that it is possible to get the messages and pass them towp_send_json_error()
andwp_send_json_success()
- in order to do so, it uses output buffering to split the HTML from the messages
Looks good to me.
#42
@
4 years ago
- Keywords commit added
From an accessibility perspective, and for history, it is worth to quote an important note from a similar issue reported by the WPCampus accessibility report on Gutenberg. See https://core.trac.wordpress.org/ticket/47120 and https://github.com/WordPress/gutenberg/issues/15302
Consider moving focus to either the error container or the error container close button when the error appears. This will ensure that Braille-only and screen magnification users are alerted that an error has occurred. Normally, moving focus to errors like this is not advised, however currently the error and the informative text are visually far away from the "Select Files" button, and neither Braille nor screen magnification software announce status messages. A better solution is to change the design to bring the error visually directly under the "Select Files" button (while still also using a live region on the error message).
A major redesign of the Image Editor is tracked in #50523. For now, we opted for a simpler approach.
Worth noting this applies only to notifications that are dynamically added in a page. Instead, the standard "admin notices" rendered on page load, wouldn't benefit from a role=alert or role=status. See #47111 and #46995.