WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 4 days ago

#47611 assigned defect (bug)

Media views: hide the "file upload button" from assistive technologies

Reported by: afercia Owned by: anevins
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-screenshots has-patch dev-feedback
Focuses: accessibility, javascript Cc:
PR Number:

Description

Noticed while working on #47145, which proposes to make the media modal a real ARIA modal dialog.

When navigating content within the media modal using the keyboard (with or without a screen reader running), at some point there's a tab stop at the bottom of the modal content. Screen readers announce "file upload button", see attached screenshot.

This button is the one that gets used to upload attachments and should be perceivable only in the "Upload FIles" tab. Instead, it's always perceivable and focusable in all the various media views.

A tab stop on an invisible control is confusing for keyboard users. Screen readers users will be confused as well, as this button is announced also in work flows that don't relate at all with file uploading.

I'd like to propose to find a way to hide this button from assistive technologies when it's not supposed to be used, making sure its functionality is preserved.

Attachments (2)

media modal file upload button.jpg (116.8 KB) - added by afercia 6 months ago.
47611.diff (364 bytes) - added by audrasjb 3 weeks ago.
Hide moxie-shim container and input via CSS

Download all attachments as: .zip

Change History (24)

#1 @afercia
6 months ago

Note: if I remember correctly this issue was solved at some point in a previous release cycle, but I couldn't find a reference. Just to note that's likely a regression.

#2 @afercia
4 months ago

Similar issue: #47144

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.


4 months ago

#4 @joemcgill
4 months ago

+1 on making this less confusing for those using assistive technology. @afercia is this only happening in the editor context, or also in the media library? If the former, it might be a regression that was introduced when the media modal was implemented in the new editor.

#5 @afercia
4 months ago

@joemcgill happens also in Classic Editor.

It's an <input type="file" ... with a CSS class moxie-shim moxie-shim-html5.

This ticket was mentioned in Slack in #core-media by mike. View the logs.


3 months ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 months ago

#8 @joedolson
3 months ago

  • Owner set to anevins
  • Status changed from new to assigned

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 months ago

#10 @audrasjb
2 months ago

  • Milestone changed from 5.3 to Future Release

This ticket still needs some work and improvements.
Moving it to Future release.

Feel free to ask for reintegration in 5.3 if a patch is available in the next few days.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


2 months ago

#12 @afercia
2 months ago

  • Milestone changed from Future Release to 5.3.1

Discussed during today's accessibility bug-scrub: agreed this ticket seems a quick fix as it would likely need the element to be hidden via CSS. Milestoning for next minor release. See related #47144.

This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.


3 weeks ago

@audrasjb
3 weeks ago

Hide moxie-shim container and input via CSS

#14 @audrasjb
3 weeks ago

  • Keywords has-patch dev-feedback added

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


11 days ago

#16 @audrasjb
7 days ago

Hi there,

@afercia @joedolson this one should be committed as soon as possible if we are all ok with the current patch.
Do you have time to review and commit it?

Otherwise, we could punt it to 5.4, so no emergency (just need to know if it can land in 5.3.1 this week).
Indeed, it's a regression, but not a regression from WP 5.3.

Cheers,
Jb

Last edited 7 days ago by audrasjb (previous) (diff)

#17 @azaozz
6 days ago

  • Keywords needs-testing added

Has this been tested in "all" recent browsers? If I remember correctly there are some security restrictions in the browsers preventing the use of "hidden" <input type="file"> elements, especially from JS.

#18 follow-up: @audrasjb
6 days ago

@azaozz I tested it on Firefox, Chrome and Safari, but I'm not sure I understand your point. The field is already hidden, we just need to hide it from assistive technologies.

(in any case, I think this ticket is going to be moved to 5.4)

#19 @audrasjb
6 days ago

  • Milestone changed from 5.3.1 to 5.4

Moving this ticket to milestone 5.4.

#20 in reply to: ↑ 18 @azaozz
6 days ago

Replying to audrasjb:

The field is already hidden, we just need to hide it from assistive technologies.

"Technically" it's not hidden, it is "off-screen". Current code:

<div
    id="html5_1drm54kle1qkop61h77ia9j53_container"
    class="moxie-shim moxie-shim-html5"
    style="position: absolute; top: 0px; left: 0px; width: 0px; height: 0px; overflow: hidden; z-index: 0;"
>
<input
    id="html5_1drm54kle1qkop61h77ia9j53"
    type="file"
    style="font-size: 999px; opacity: 0; position: absolute; top: 0px; left: 0px; width: 100%; height: 100%;"
    multiple=""
...

Note the position: absolute; top: 0px; left: 0px; width: 100%; height: 100%; which I think is needed for drag/drop upload to work.

Looks like hiding it from assistive technologies may not be as easy as adding display: none !important. But yes, lets look there during 5.4 :)

Last edited 6 days ago by azaozz (previous) (diff)

#21 @afercia
4 days ago

  • Keywords needs-testing removed

Yep, I don't think this input can be hidden via CSS. On #30392 / [31117] we made some work to move this input out from the "tabbable" elements within the media modal dialog thus this input can't be tabbed to.

The problem is that it still can be navigated to when arrowing with a screen reader as well as when using other navigation tools screen readers provide.

Since it's already non-tabbable, there's just the need to hide it from assistive technologies. It is already meant to be visually hidden thus it should be non-visible for all users. I guess an aria-hidden="true" would work. Unfortunately, as far as I see, this input is injected with innerHTML and not exposed in the mOxie object so I can't think of a non-hackish way to add a HTML attribute to it.

Worth noting that the WordPress moxie.js file is pretty out of date. For example, on the upstream latest version a tabindex="-1" attribute was added to this input 2 years and a half ago. See https://github.com/moxiecode/moxie/commit/d2e3bfdf8fbf5e1674fae09db445af811ae36c61

I guess a good first step would be updating the WordPress moxie.js file to its latest version. This should happen early in the 5.4 release cycle.

Before that though, I'd suggest to ask the Ephox team to add aria-hidden="true". Fixing this issue upstream seems to me the most reliable way to make this input not perceivable to all users.

Note: See TracTickets for help on using tickets.