#47120 closed defect (bug) (fixed)
Media modals: Upload errors and field information are not associated with their control
Reported by: | afercia | Owned by: | antpb |
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Media | Keywords: | wpcampus-report has-patch commit |
Focuses: | ui, accessibility | Cc: |
Description
Moved from the WPCampus accessibility report issues on GitHub, see https://github.com/WordPress/gutenberg/issues/15302
Errors and field information are not associated with their control
- Severity: High
- Affected Populations:
- Blind
- Low-Vision
- Cognitively Impaired
- Platform(s):
- Windows - Screen Reader
- Windows - ZoomText
- Mac - VoiceOver
- Android - TalkBack
- iOS - VoiceOver
- Components affected:
- Media Dialog
#### Issue description
When users attempt to upload an image in the "Featured Image" modal
dialog, and the image is too large, an error message appears visibly.
However blind and low-vision users are not alerted with the
announcement, and focus remains on the upload button (except in Edge
browser, where focus goes back to the top of the page after a failed
upload).
The 2MB limit is also not programmatically associated with the "Select
Files" button, so blind and low-vision users may miss that limit unless
they happen to navigate past the button in exploration of the whole
modal.
Users who are zoomed-in or using magnification software may not see the
error as it may be outside their viewports. Screen reader users remain
on the "Select Files" button and need to navigate around to see if
there are any errors or if the upload was successful.
[![Screenshot of the error message and the 2MB limit informative
text](/images/17718_feature_image_bigly_yuge.png)]{.image-wrap}
##### Issue Code
<button type="button" class="browser button button-hero" id="__wp-uploader-id-4" style="display: inline-block; position: relative; z-index: 1;">Select Files</button> ... <div class="upload-errors"> <div class="upload-error"> <span class="upload-error-filename">trump.gif</span> <span class="upload-error-message">trump.gif exceeds the maximum upload size for this site.</span> </div> </div> ... <div class="post-upload-ui"> <p class="max-upload-size">Maximum upload file size: 2 MB.</p> </div>
#### Remediation Guidance
Put role=alert
on the error, and on any additional error or success
messages that are added as users upload media. This will allow screen
reader users to know immediately that there is an error.
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).
Programmatically associate the informative text ("max 2MB size") with
the "Select Files" button using aria-labelledby
. Do this by giving
the informative text container an id
value and set this in order in
an aria-labelledby
attribute on the "Select Files" button. Include
the button's own id
token in the aria-labellebdy
so that the
button's text isn't overridden.
##### Recommended Code
<button type="button" class="..." aria-labelledby="__wp-uploader-id-4 post-upload-info" id="__wp-uploader-id-4" style="...">Select Files</button> ... <button type="button" class="..."> <span class="screen-reader-text">Dismiss Errors</span> </button> ... <div class="upload-errors"> <div class="upload-error" role="alert"> <h3 class="...">trump.gif</h3> <p class="...">trump.gif exceeds the maximum upload size for this site.</p> </div> </div> ... <div class="post-upload-ui" id="post-upload-info"> <p class="max-upload-size">Maximum upload file size: 2 MB.</p> </div>
#### Relevant standards
- 1.3.1 Info and Relationships (Level A)
- 1.3.3 Sensory Characteristics (Level A)
- 3.3.1 Error Identification (Level A)
- 3.3.2 Labels or Instructions (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](https://www.tenon.io) and funded by [WP Campus](https://wpcampus.org/). This issue is GUT-60 in Tenon's report
Attachments (6)
Change History (83)
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
5 years ago
#4
@
5 years ago
@afercia We still need to figure out where to move the focus. The remediation says "moving focus to errors", but at the same time it can be a bit unhelpful to move the focus to a containing DIV that lacks information. Do we move the focus to the element with the Alert role?
#5
@
5 years ago
@anevins thanks for the ping.
For the announcement, I think it's OK to use role=alert or, better, wp.a11y.speak()
with the assertive
parameter, which is equivalent to an alert.
For the focus part, I think the remediation actually says the preferred solution would be bringing the error message "in view" close to the "Select Files" button. Re-reading:
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).
Ideally, I'd recommend to change the design. Alternatively, move focus but I wouldn't move it to the alert or to a DIV. An example can be seen here: https://www.tenon-ui.info/forms-full-demo
- submit the form leaving all fields empty
- focus is moved to the top, to a h2 heading with meaningful text
#6
@
5 years ago
@afercia I'm not sure I understand. The Focus will still need to happen won't it? Because people will be left on the form's submission button and will have to navigate backwards to get to the form field in question. Is navigating backwards okay? I'm asking genuinely.
#8
@
5 years ago
I've uploaded the image from the WPCampus complete technical report PDF document (329 pages) and one more image for clarification, as I think the image from the report identifies only one of the possible cases.
It's important to note that the media uploader behavior actually differs depending on the error type, for example:
- file exceeding allowed size or file with no extension: the error is displayed inline below the "Select Files" button (first image)
- empty file: the modal switches to the media grid and the error is displayed in the sidebar (second image)
This makes things a bit more complicated.
Regardless, in this case there's no text input field nor form submission. There's a "Select Files" button. In any case the error message should be displayed underneath the button otherwise it may be outside the viewport for some users or difficult to find for other users e.g. Braille-only users.
Note: When in doubt, please refer to the WPCampus complete report, as some details may have been list when moving issues from GitHub to Trac. The PDF is published on the WPCampus web site.
This ticket was mentioned in Slack in #core-media by anevins. View the logs.
5 years ago
#10
@
5 years ago
Hi @afercia We've mentioned this in the Media meeting and struggled to define a set of actions we need to take when assigning this to someone to own. Would you be able to help with this? Thanks!
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
5 years ago
#12
@
5 years ago
If you need help with guidance, I can also provide some help here. I'll track the ticket for questions; I can't easily make the media team meetings, however.
#13
@
5 years ago
I would try to investigate:
- why the modal needs to switch view to display some errors in the sidebar: any technical reason for this?
- ideally, the view shouldn't change
- the error should just appear "in place", as close as possible to the "Select Files" button
Re-reading the essential parts of the remediation suggested from the WPCampus report:
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).
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
This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.
5 years ago
#17
@
5 years ago
- Keywords needs-patch added
@mikeschroder does the core media team need some input from accessibility side?
It would be nice to have an owner from the Core media team here. Thanks :)
This ticket was mentioned in Slack in #core-media by mike. 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 afercia. 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
Just wanted to check in!
@antpb: Do you want any more input or help with this one?
This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.
5 years ago
#25
@
5 years ago
- Keywords early added
- Milestone changed from 5.3 to 5.4
Hi,
5.3 beta 3 is on Monday, and this ticket doesn't have any patch for the moment.
Since it will probably deserves some testing before commit, I don't think it's realistic to get this fixed in 5.3.
Moving the ticket to 5.4 early
so it could be fixed on time for 5.4.
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
#30
@
5 years ago
- Milestone changed from 5.4 to 5.5
5.4 beta 1 is on Monday, and this ticket doesn't have any patch for the moment.
Since it will probably deserves some testing before commit, I don't think it's realistic to get this fixed in 5.4.
Moving the ticket to 5.5 early so hopefully it could be fixed on time for 5.5.
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
4 years ago
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
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
4 years ago
#37
@
4 years ago
- Milestone changed from 5.5 to 5.6
This ticket was discussed during today's accessibility bug-scrub: the team agreed to move it to the next release cycle.
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 audrasjb. View the logs.
4 years ago
#40
@
4 years ago
- Milestone changed from 5.6 to Future Release
Moving to Future release as it didn't move during WP 5.6 alpha cycle.
This ticket was mentioned in Slack in #core-media by joedolson. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
4 years ago
#43
@
4 years ago
- Milestone changed from Future Release to 5.7
Here we are again, for real this time! This issue was discussed in the recent Media component meeting and we're all in agreement this has actionable next steps and just needs to be owned by someone with time. I'm going to continue bringing it up in meetings and document next steps as the come toward 5.7.
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-media by joedolson. View the logs.
4 years ago
#46
@
4 years ago
This was discussed in the recent Media component meeting. What is still remaining to be figured out here is how to pass the message to the location recommended in the report. Because of the media library structure I found that the location we are recommended moving it is difficult to receive the error message and in a separate file. I hope this is a lack of my JS skills and might be an easy fix, but any help you could provide would be much appreciated @joedolson Thank you!
#47
@
4 years ago
I'm pretty confident that this is possible; but there's no denying that it's complicated. I've been spending the last couple days exploring, and I've made some progress - mostly in learning how the media library is put together.
It's not absolutely critical that we move the error to a new location, and if we can't get it figured out, we should go ahead and move ahead regardless with the other solution offered by Tenon.io in their initial report by moving focus to the error.
I don't want to let the struggle with getting the perfect solution stop us from making what we have better.
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#50
@
4 years ago
This ticket represents two separate issues:
a) Programmatically associate the informative txt with the Select Files button
b) Announce errors to screen readers and move focus.
The first is fairly simple, and is complete in 47120.diff
The second is trickier. I have a patch that seemed like it should work (using the 'move focus' option, rather than moving the error), but it doesn't work on the first upload; only on subsequent uploads. I haven't yet figured out why that is.
Adding aria-live="assertive"
to upload-inline-status
was sufficient to read out results in the media modal, and that error is already in close visual & DOM proximity.
If somebody with better knowledge of Backbone and JS can try and figure out why upload errors in the non-modal media library context don't trigger on the first error, I think that would resolve this issue.
#51
@
4 years ago
To test:
1) Media > Add New
Navigate to 'Select Files'
- Screen reader should output 'Select Files' and add the descriptive information shown below
Upload a file that triggers an error
- Screen reader should report the error, focus should move to the error container.
Note: this currently only works on repeated errors, the first error does not trigger the alert or the focus move.
2) Add media to post: same tests within the Media Modal.
Note: in the media modal, the alert and focus move *do* work as expected with this patch.
@
4 years ago
Changes a role=alert to aria-live=assertive for consistency. In testing, role=alert double read notices.
This ticket was mentioned in Slack in #core-media by joedolson. View the logs.
4 years ago
This ticket was mentioned in PR #962 on WordPress/wordpress-develop by adamsilverstein.
4 years ago
#54
Trac ticket:
#55
@
4 years ago
Note: this currently only works on repeated errors, the first error does not trigger the alert or the focus move.
@joedolson I reviewed this and experienced the issue you mentioned. Even removing the focus on error I see the same issue.
I don't fully understand what is supports to trigger the speaking here, we don't call wp.a11y.speak() explicitly, so I guess there is a listener for changes to the error areas that is responsible? Is it possible to switch this to a more explicit call when the error is returned?
This ticket was mentioned in Slack in #core-media by joedolson. View the logs.
4 years ago
#57
@
4 years ago
The change is the addition of aria-live="assertive"
on the container that receives the error, as well as the addition of tabindex='-1'
on the parent container.
I can see that the tabindex is added, but yet the container doesn't receive focus & the message isn't spoken on the first error, only on subsequent errors. My theory is that some backbone refresh is occurring around the assignment of focus that disrupts this, but I just can't see what's happening.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#60
@
4 years ago
- Keywords needs-testing added; dev-feedback removed
New patch uses wp.a11y.speak()
and ._defer()
to defer the error message and focus change until Backbone is finished rendering.
Could use some broader testing, but seems to do the trick. Testing process still same as above in https://core.trac.wordpress.org/ticket/47120#comment:51
This ticket was mentioned in Slack in #core-media by joedolson. View the logs.
4 years ago
#63
@
4 years ago
@joedolson I am testing with voiceover in Chrome, let me know if I should try with Safari or another tool. Maybe my test setup is not a usual one for screen reader users?
- Turn on voiceover.
- Using my keyboard only, i upload an image that exceeds the max upload size.
- The error is not spoken, instead the close button (which is now focused) is described
Screencast: https://share.getcloudapp.com/geu4W5dL
I was able to get it working by adding a bit more of a delay (add ,1000
after the function in the defer call). I'm not sure if that is required, and it does feel a bit fragile.
#64
@
4 years ago
@adamsilverstein I'd consider VoiceOver with Chrome a very low priority combination. The most relevant combinations to test are JAWS/Chrome; NVDA/Firefox; NVDA/Chrome; JAWS/IE and VoiceOver/Safari.
Every combination of AT/browser will work a bit differently, and VoiceOver is the furthest outside the norm, and best with Safari.
I'll test with a delay in other environments, and we can see. But no, VoiceOver/Chrome is not a particularly common set up for screen reader users - if this causes problems in other environments, I'd give preference to more common pairings. Did you experiment with a shorter delay? That seems pretty long.
#65
@
4 years ago
I tested the view from the Add New button in the Media Library (wp-admin/upload.php).
47120.3.diff, Windows 10, Chrome Version 88.0.4324.150 with NVDA
When the Select Files button is focused, the information about the max file size is read correctly.
When the upload fails, the focus is on the button that dismisses the error.
I am not able to see a pattern for when the error message is read out loud and when it is not.
It does not seem to be related to if it is the first or second error. Sometimes it is read out on the first error.
The error is always written in the NVDA Speach viewer, but I don't always hear it.
When the upload fails and the error is read out, it reads the title of the page before it reads the error,
and after the error, it reads the title of the page again:
Media Library ‹ My New WordPress Site — WordPress - Google Chrome gutenberg (1).zip exceeds the maximum upload size for this site. Media Library ‹ My New WordPress Site — WordPress document
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
4 years ago
#67
@
4 years ago
- Keywords commit added; needs-testing removed
In testing, I didn't find that adding an argument to _.defer did anything - and reading the documentation, it doesn't seem like it *should* have done anything; did you also switch from _.defer
to _.delay
, @adamsilverstein?
Regardless, I did testing both ways, using JAWS/Chrome, JAWS/Firefox, NVDA/Chrome and NVDA/Firefox, and found that using delay
was a lot more consistent - it seemed to be generally read in numerous situations where the deferred function wasn't.
I find delay to be less elegant; it's just a magic number delay that one hopes will work, where defer should wait until the call stack is clear, but in practice, it seems to work a lot better.
I think we should commit this using _.delay
; if we can find a better solution later, that's fine, but this appears to work in *most* cases.
#68
@
4 years ago
Great! makes sense that delay would work better, defer may not pick up the activity happening in the screen reader, outside the browser. I'll give this another test to see how it works for my setup.
@poena Excellent, thank you for testing! If you can, please test the latest patch version and see if the behavior is improved for you.
I think we should commit this using _.delay; if we can find a better solution later, that's fine, but this appears to work in *most* cases.
+1 to committing this, certainly improves the situation. Do you think in the future browser or screen reader APIs might improve the situation here? If so, maybe add a code comment here that explains the delay could possibly be removed in the future.
#69
@
4 years ago
Hi, I tested 47120.4.diff.
It now reads the error out loud on every failed upload.
But it reads the information for the Select Files button first.
Speech Viewer output:
Media Library ‹ My New WordPress Site — WordPress - Google Chrome
Media Library ‹ My New WordPress Site — WordPress document
button Select Files Maximum upload file size: 2 MB.
Media Library ‹ My New WordPress Site — WordPress - Google Chrome
gutenberg (2).zip exceeds the maximum upload size for this site.
Media Library ‹ My New WordPress Site — WordPress document
button Select Files Maximum upload file size: 2 MB.
clickable Dismiss Errors button
I wanted to ask if this patch should also fix the wp-admin/media-new.php view,
that is reached from the menu under Media > Add New
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#71
@
4 years ago
Thanks @poena for re-testing!
But it reads the information for the Select Files button first.
Hmm, I noticed we call the focus first, then speak the error. Perhaps these should be reversed, what do you think @joedolson? I'll try it that way in my testing to see if that changes the spoken order.
#72
@
4 years ago
The delay is so that all other reading will be finished by the time the error message is read. Ideally, the 'Select Files' button should not be read at all. I suspect that the reason the ._defer call doesn't work is because Backbone does something to add a second callstack after the callstack including the defer is finished, and that is a refresh of the file uploader view. Fixing that would probably require a significant refactor of the media library, however.
The focus/speak order was intentional; moving focus and then calling the error ensure that the new focus won't interrupt the wp.a11y.speak call. I haven't tested switching the order in combination with the ._delay function, however.
I would only expect that to change the order of 'Dismiss Errors' and the error message itself; it would have no impact on the ordering of 'Select Files'.
I wanted to ask if this patch should also fix the wp-admin/media-new.php view, that is reached from the menu under Media > Add New
Yes. That view is a bit wonkier, however; the calls are slightly different, and messier.
#73
@
4 years ago
The focus/speak order was intentional; moving focus and then calling the error ensure that the new focus won't interrupt the wp.a11y.speak call.
Ah, makes sense.
+1 from me @joedolson!
This ticket was mentioned in Slack in #core by monikarao. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by boniu91. View the logs.
4 years ago
hellofromtonya commented on PR #962:
4 years ago
#77
Merged with changeset https://core.trac.wordpress.org/changeset/50352
Note: this applies to all the media views when uploading attachments and upload errors are displayed, usually because of (and can be tested with):
.exe
)