Make WordPress Core

Opened 9 months ago

Closed 4 months ago

#63984 closed enhancement (fixed)

Assess if the tabpanels in the media modals should receive focus

Reported by: alh0319's profile alh0319 Owned by: joedolson's profile joedolson
Milestone: 7.0 Priority: normal
Severity: normal Version: 5.3
Component: Media Keywords: has-patch has-test-info commit
Focuses: accessibility Cc:

Description

The media modal has tabpanels that include a tabindex so they are the next tab stop after the tab buttons. Here's an example of the code for one of the panels:

<div class="media-frame-tab-panel" role="tabpanel" aria-labelledby="menu-item-insert" tabindex="0">
			<div class="media-frame-router"><div role="tablist" aria-orientation="horizontal" class="media-router"><button type="button" role="tab" class="media-menu-item active" id="menu-item-upload" aria-selected="true">Upload files</button><button type="button" role="tab" class="media-menu-item" id="menu-item-browse" aria-selected="false" tabindex="-1">Media Library</button></div></div>
			<div class="media-frame-content" role="tabpanel" aria-labelledby="menu-item-upload" tabindex="0"><div class="uploader-inline">
		
		
		<div class="uploader-inline-content no-upload-message">
		
					<div class="upload-ui">
				<h2 class="upload-instructions drop-instructions">Drop files to upload</h2>
				<p class="upload-instructions drop-instructions">or</p>
				<button type="button" class="browser button button-hero" style="position: relative; z-index: 1;" id="__wp-uploader-id-1" aria-labelledby="__wp-uploader-id-1 post-upload-info">Select Files</button>
			</div>

			<div class="upload-inline-status"><div class="media-uploader-status" style="display: none;">
		<h2>Uploading</h2>

		<div class="media-progress-bar"><div></div></div>
		<div class="upload-details">
			<span class="upload-count">
				<span class="upload-index"></span> / <span class="upload-total"></span>
			</span>
			<span class="upload-detail-separator">–</span>
			<span class="upload-filename"></span>
		</div>
		<div class="upload-errors"></div>
		<button type="button" class="button upload-dismiss-errors">Dismiss errors</button>
	</div></div>

			<div class="post-upload-ui" id="post-upload-info">
				
				<p class="max-upload-size">
				Maximum upload file size: 256 MB.				</p>

				

							</div>
				</div>
	</div></div>
		</div>

When hitting the Tab key after selecting a tab in the modal, the focus goes to the panel itself instead of the first interactive element in the panel. In VoiceOver, the panel's name is read out concisely as set by the aria-labelledby attribute. However in NVDA, this results in the entire tabpanel contents being read out which is quite verbose.

Image showing NVDA reading all content of the Add Media Panel

We should assess whether the tab panel receiving focus is desired and consistent with how other tabs are coded throughout the admin.

Change History (14)

#1 @joedolson
9 months ago

  • Owner set to joedolson
  • Status changed from new to accepted

#2 @joedolson
9 months ago

Added in [46363] to fix #47149.

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


7 months ago

#4 @joedolson
7 months ago

  • Milestone changed from Awaiting Review to 7.0
  • Version set to 5.3

#5 @joedolson
4 months ago

I was commenting on a different issue; redacting this comment...

Last edited 4 months ago by joedolson (previous) (diff)

#6 @joedolson
4 months ago

The APG recommendation for a tabpanel is to have tabindex="0" if the first element in the panel is not focusable.

In both views of the media panel, that's true: either an h2 heading with 'Filter media', or the text 'Drop files to upload'. However, for screen reader usage (where this is most relevant), the drop region information is largely irrelevant; and the heading context is pretty well handled by the labels on the filter regions.

Given that, I think it may be worthwhile to remove the tabindex. It means sacrificing immediate access to the first content in favor of lower verbosity in screen readers, but I don't think that loss is significant.

@alh0319 - would you agree with this assessment?

This ticket was mentioned in PR #10884 on WordPress/wordpress-develop by @joedolson.


4 months ago
#7

  • Keywords has-patch added

Reduces verbosity for screen readers which attempt to read the full content of the panel; reduces keypress distance to controls by one.

Trac ticket: https://core.trac.wordpress.org/ticket/63984

## Use of AI Tools

None

#8 @joedolson
4 months ago

Reviewed the conversations surrounding #47149, and there was never any discussion by the team concerning the implementation of tabindex in this context.

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


4 months ago

#10 @joedolson
4 months ago

  • Keywords needs-testing has-test-info added

To test:

  • Open the media modal.
  • Using the keyboard, navigate using tab to reach the tabs. (Upload/Media Library)
  • Select a tab using arrow keys to move and Enter to select.
  • Press tab to navigate into the tabpanel.

Before:

  • First tab focuses the tabpanel itself, second tab focuses the first control in the panel.

After:

  • First tab press focuses the first control in the panel.

#11 @huzaifaalmesbah
4 months ago

  • Keywords needs-testing removed

Patch Testing Report

Patch Tested: https://github.com/WordPress/wordpress-develop/pull/10884

Environment

  • WordPress: 7.0-alpha-61215-src
  • PHP: 8.2.29
  • Server: nginx/1.29.4
  • Database: mysqli (Server: 8.4.7 / Client: mysqlnd 8.2.29)
  • Browser: Chrome 144.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.4
  • Plugins:
    • Classic Editor 1.6.7
    • Test Reports 1.2.1

Steps Taken

  1. Opened Posts → Add New.
  2. Clicked Add Media to open the media modal.
  3. Used Tab key to move focus to the tab list (Upload Files / Media Library).
  4. Used arrow keys to switch between tabs.
  5. Pressed Enter to activate the selected tab.
  6. Pressed Tab to move focus into the tabpanel.
  7. Repeated the test before and after applying the patch.
  8. ✅ Patch is solving the problem.

Expected Result

After activating a tab and pressing Tab:

  • Focus should move directly to the first interactive control inside the tabpanel.
  • The tabpanel container itself should not receive focus.
  • Screen readers should not announce the entire contents of the panel.
  • Keyboard navigation should require one fewer tab stop.

Before patch:

  • First Tab press after activating a tab focuses the tabpanel container.
  • This introduces an extra tab stop.
  • With screen readers (notably NVDA), the entire tabpanel content is read aloud, resulting in excessive verbosity.

After patch:

  • Focus moves directly to the first interactive control inside the panel.
  • The extra tab stop is removed.
  • Screen reader verbosity is reduced.
  • Keyboard navigation flow is more efficient.

Behavior matches expected outcome.

Screenshots / Screencast

Before: https://files.catbox.moe/tkwt58.mp4

After: https://files.catbox.moe/1xti9e.mp4

#12 @alh0319
4 months ago

I re-examined the Add Media dialog, and I think the first element in every tab panel is focusable. Visually, there is a heading, but the heading is not actually in the tab panel; it appears before it in the DOM.

For example, this is the contents of the Add Media panel:

<div class="media-frame-tab-panel" role="tabpanel" aria-labelledby="menu-item-insert" tabindex="0">
	<div class="media-frame-router">
		<div role="tablist" aria-orientation="horizontal" class="media-router">
			<button type="button" role="tab" class="media-menu-item" id="menu-item-upload" aria-selected="false" tabindex="-1">Upload files</button>
			<button type="button" role="tab" class="media-menu-item active" id="menu-item-browse" aria-selected="true">Media Library</button>
		</div>
	</div>
	<div class="media-frame-content" data-columns="5" role="tabpanel" aria-labelledby="menu-item-browse" tabindex="0">
		<div class="attachments-browser has-load-more"><div class="media-toolbar"><div class="media-toolbar-secondary">
			<h2 class="media-attachments-filter-heading">Filter media</h2>

[...]

Most of the panels (Add media, Create gallery, Create audio playlist, Create video playlist, Featured image) have a set of nested tab controls as the first element in the panel.

The insert from URL tab has an input as the first element in the panel.

Within the nested tabs, there is a heading that would be skipped, but I agree with @joedolson that the heading is redundant or trivial, as it is covered by the label of the first focusable element in the panel.

I agree that the tab stop should be removed from both the parent tabpanel and the nested tab panels.

#13 @joedolson
4 months ago

  • Keywords commit added

#14 @joedolson
4 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 61626:

Media: A11y: Remove tabindex on tabpanels.

Tabindex should be set on a tabpanel if there is content in that tabpanel that is not focusable. The media library uses tabpanels, but the first content is either focusable or redundant.

Change the media library tabpanels to omit tabindex attributes.

Props alh0319, joedolson, westonruter, huzaifaalmesbah.
Fixes #63984.

Note: See TracTickets for help on using tickets.