Make WordPress Core

Opened 4 months ago

Closed 2 weeks ago

Last modified 2 weeks ago

#60799 closed defect (bug) (fixed)

Fix ARIA attributes for Classic Editor link inserter

Reported by: lyonmuller's profile lyonmuller Owned by: jwgoedert's profile jwgoedert
Milestone: 6.6 Priority: normal
Severity: normal Version: 6.4.3
Component: Editor Keywords: has-patch has-testing-info commit
Focuses: accessibility Cc:

Description

Problem:

Currently, the WordPress link insertion modal in the text editor uses an <h1> tag for its title ("Insert/edit link"). This can create accessibility issues, as multiple <h1> tags on a single page can confuse screen readers and undermine the semantic structure of the content, potentially leading to a poor user experience for people with disabilities.

Proposed Solution:

I suggest changing the <h1> tag in the link insertion modal to a <h2> tag, or alternatively to a <span> tag if the heading level is not appropriate in the context of the modal's use. This change would improve the semantic HTML structure and enhance accessibility by maintaining a proper heading hierarchy.

Justification:

According to the HTML5 specification and accessibility guidelines, a page should have a clear and logical heading hierarchy. The main content should start with an <h1> tag, followed by <h2>, <h3>, and so on. The current use of an <h1> tag in the link modal disrupts this hierarchy, especially since this modal can appear on any page regardless of the existing heading structure.

Implementing this change will make WordPress more accessible and compliant with WCAG (Web Content Accessibility Guidelines) standards, which is crucial for users relying on screen readers and for SEO best practices.

Steps to Reproduce:

  1. Go to the WordPress text editor.
  2. Click on the 'Add link' button to open the link insertion modal.
  3. Inspect the modal's title element, which is currently marked up as an <h1>.

Possible Implementation:

The change can be implemented by modifying the HTML structure in the core WordPress files where the link modal is defined. Additionally, testing should be conducted to ensure that this change does not affect the modal's functionality and that it enhances the accessibility as intended.

I am looking forward to the community's input on this proposal and am happy to contribute to the implementation and testing phases.

Attachments (4)

60799_InsertEditLinkModal.png (77.0 KB) - added by jwgoedert 3 months ago.
Issue 60799 Modal View of Insert/Edit Link
60799_ClassicEditorModalScreenshot.png (120.5 KB) - added by jwgoedert 3 months ago.
Issue 60799 Modal View of Classic Editor
60799_AddMediaModal.png (94.6 KB) - added by jwgoedert 3 months ago.
Issue 60799 Modal View of Add Media
60799.diff (1.5 KB) - added by joedolson 4 weeks ago.
Patch to implement aria-modal and aria-hidden for link insertion.

Download all attachments as: .zip

Change History (28)

This ticket was mentioned in Slack in #core-test by ankit-k-gupta. View the logs.


4 months ago

#2 @sabernhardt
3 months ago

  • Component changed from TinyMCE to Editor

Hi and thanks for the report!

Adrian Roselli has stated that an h1 in a dialog is technically not correct, though it should not be changed for only one dialog.

The Classic Editor's "Insert/edit link" dialog has had the h1 since [36991].

<div id="wp-link-wrap" class="wp-core-ui has-text-field" style="display: block;" role="dialog" aria-labelledby="link-modal-title">
	<form id="wp-link" tabindex="-1">
		<input type="hidden" id="_ajax_linking_nonce" name="_ajax_linking_nonce" value="4cab82dbc7">
		<h1 id="link-modal-title">Insert/edit link</h1>

Similar dialogs in the Classic Editor do not have a heading tag:

  • <div id="mceu_84" class="mce-container mce-panel mce-floatpanel mce-window mce-in" hidefocus="1" style="border-width: 1px; z-index: 100101; left: 263px; top: 96.5px; width: 460px; height: 448px;" role="dialog" aria-labelledby="mceu_84" aria-describedby="mceu_84-none" aria-label="Keyboard Shortcuts"><div class="mce-reset" role="document"><div id="mceu_84-head" class="mce-window-head"><div id="mceu_84-title" class="mce-title">Keyboard Shortcuts</div>
  • <div id="mceu_98" class="mce-container mce-panel mce-floatpanel mce-window mce-in" hidefocus="1" style="border-width: 1px; z-index: 100101; left: 157px; top: 136.5px; width: 672px; height: 368px;" role="dialog" aria-labelledby="mceu_98" aria-describedby="mceu_98-none" aria-label="Special character"><div class="mce-reset" role="application"><div id="mceu_98-head" class="mce-window-head"><div id="mceu_98-title" class="mce-title">Special character</div>
  • <div id="mceu_112" class="mce-container mce-panel mce-floatpanel mce-window mce-in" hidefocus="1" style="border-width: 1px; z-index: 100101; left: 311.5px; top: 161.5px; width: 363px; height: 318px;" role="dialog" aria-labelledby="mceu_112" aria-describedby="mceu_112-none" aria-label="Color"><div class="mce-reset" role="application"><div id="mceu_112-head" class="mce-window-head"><div id="mceu_112-title" class="mce-title">Color</div>

The "Add media" dialog also has an h1:

<div tabindex="0" class="media-modal wp-core-ui" role="dialog" aria-labelledby="media-frame-title">
	<button type="button" class="media-modal-close"><span class="media-modal-icon"><span class="screen-reader-text">Close dialog</span></span></button>
	<div class="media-modal-content" role="document">
		<div class="media-frame mode-select wp-core-ui" id="__wp-uploader-id-0">
			<div class="media-frame-title" id="media-frame-title"><h1>Add media</h1></div>

Likewise, the block editor's Classic block dialog (when the editor is in an iframe) has an h1 heading.

<div class="components-modal__frame block-editor-freeform-modal__content" role="dialog" aria-labelledby="components-modal-header-0" tabindex="-1"><div class="components-modal__content" role="document"><div class="components-modal__header"><div class="components-modal__header-heading-container"><h1 id="components-modal-header-0" class="components-modal__header-heading">Classic Editor</h1>

Last edited 3 months ago by sabernhardt (previous) (diff)

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


3 months ago

#4 @joedolson
3 months ago

  • Milestone changed from Awaiting Review to 6.6
  • Owner set to jwgoedert
  • Status changed from new to assigned

#5 @joedolson
3 months ago

Assigning to @jwgoedert to explore the scope of dialogs that need change. If this gets too large, it may not be a good candidate right now; but we'll see, first.

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


3 months ago

#7 @alexstine
3 months ago

Hello,

Noting here that I really disagree with the whole headings assessment in modals. I am a blind user and here is the conclusion I've come to for modals, noting this does not apply to non-modals.

  1. I open a page.
  2. Navigate to the first heading.
  3. Find a button to open a modal.
  4. Navigate through the modal.
  5. Want to hear the title again, maybe it is a prompt asking me if I want to delete something.
  6. I'll press Shift+1 on Windows to find the first heading 1.
  7. Title is announced.

The whole argument that headings should follow order is an important one, but not when the content outside of a modal is inert. If you were anywhere else but a modal, the Shift+1 shortcut would have landed you on the page title. If inside a modal with a heading 2 as the title, all you hear is "no previous heading 1" because the h1 like the rest of the content, is as said before, inert.

I predict this will lead to very confusing experiences for users and it's not one I am in favor of pushing in WP. No sense in following heading order if there are no headings present in the accessibility tree proceeding the modal title.

I believe strongly the W3C example should be updated to be more in line with how users will experience this.

https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/examples/dialog/

If we take a look at the example from Deque University, it is more of the approach I would take.

https://dequeuniversity.com/library/aria/simple-dialog

Thanks.

@jwgoedert
3 months ago

Issue 60799 Modal View of Insert/Edit Link

@jwgoedert
3 months ago

Issue 60799 Modal View of Classic Editor

@jwgoedert
3 months ago

Issue 60799 Modal View of Add Media

#8 @jwgoedert
3 months ago

Thank you @alexstine for giving this valuable feedback.

I spent some time going through the classic editor and confirming the findings of @sabernhardt and captured relevant code below.

I think after @alexstine's input I agree that these as modals may stand alone and warrant h1 tags as they seem to stand alone. I have only tested with voiceover and have had mixed results.

Images above are the "Insert/Edit Link" menu(accessed via 'link options' in after hitting 'Insert/Edit Link', "Classic Editor" modal, and the "Add Media" modal which contains six menu items which dynamically become the heading after being clicked.

The "Insert/Edit Link" h1 tag is hardcoded in the class-wp-editor.php file.(https://github.com/WordPress/wordpress-develop/blob/80096ddf29d3ffa4d5654f5f788df7f598b48756/src/wp-includes/class-wp-editor.php#L1881)

<h1 id="link-modal-title"><?php _e( 'Insert/edit link' ); ?></h1>

The "Add Media" headers are set dynamically using the 'createTitle' function in 'media-frame.js' https://github.com/WordPress/wordpress-develop/blob/80096ddf29d3ffa4d5654f5f788df7f598b48756/src/js/media/views/media-frame.js#L162 by setting the value of 'tagName' on line 165 to 'h1'.

	/**
	 * @param {Object} title
	 * @this wp.media.controller.Region
	 */
	createTitle: function( title ) {
		title.view = new wp.media.View({
			controller: this,
			tagName: 'h1'
		});
	},

Styles are applied in the 'media-views.css' https://github.com/WordPress/wordpress-develop/blob/80096ddf29d3ffa4d5654f5f788df7f598b48756/src/wp-includes/css/media-views.css#L805 to the 'h1' tag.

.media-frame-title h1 {
	padding: 0 16px;
	font-size: 22px;
	line-height: 2.27272727;
	margin: 0;
}

I will wait for discussion and clarificaiton before diving further or creating any PRs.


#9 @joedolson
3 months ago

The big question here, I think, is "is a modal dialog within the content of the surrounding page or not".

Adrian's argument is semantic: the modal is accessed within the context of the parent page, and the H1 of the parent page still applies to that context, so the heading should be an H2 in most cases.

Alex's argument is user-flow based: at the time you access the heading, it is the primary heading for your current context. Therefore, if you are seeking for an H1, and the modal header is an H2, you will not be able to find it, because it is not an active element on the page. Since every view should have an H1 to define the purpose of the view, the modal should have an H1.

Both arguments have merit, and I think we just have to make a decision within the team which perspective we thinking is preferable. In my opinion, the semantic argument is valid, but tenuous, and it's not clear it benefits the user. I'm not sure we should let semantic purity take precedence over user experience; but I think we should pursue additional input from other team members before making a decision.

@alh0319 @afercia Would welcome your thoughts here!

#10 @afercia
2 months ago

Thanks everyone for this interesting discussion. First, I'd like to encourage everyone to use a correct terminology, which is also important to better understand the underlying issue.

The Insert/edit link is a dialog. Dialogs can be modal and non-modal. Actually, 'modal' is a behavior that other design patterns can potentially use. The term 'modal' has been used incorrectly for ages in place of 'dialog'.

That said, the difference is important.

  • Modal dialogs: the rest of the page is supposed to be inert and not perceivable by assistive technology. Usually, this is achieved by setting aria-modal="true" on the dialog and setting aria-hidden="true" on all the other direct children of the body element. Actually, the modal dialog is the only content perceived by a screen reader and all the rest of the page is ignored. As such, the assumption the H1 'can create accessibility issues, as multiple <h1> tags on a single page can confuse screen readers' isn't correct in the first place. In fact, when the modal dialog is open, its H1 is the only H1 perceived by a screen reader.
  • Non-modal dialogs: the rest of the page is not hidden from assistive technology. In this case, the heading hierarchy should take into account both the rest of the page and the dialog. However, non-modal dialogs are rare. It's a pretty uncommon pattern.

To me, using a H1 for a modal dialog is semantically correct considering that when the modal dialog is open, its H1 is the only perceived H1 on the page.

By changing it to a H2, given the rest of the page is ignored, that would result in a document with a missing H1 where the heading hierarchy actually starts with a H2. To me, that would be incorrect.

On top of that, as Alex pointed out, using a H1 makes screen reader users experience better.

Rather than considering to change the heading level, I'd suggest to check whether the Insert/edit link modal dialog, which is pretty old, correctly uses the aria-modal attribute and whether it makes the rest of the page inert by using aria-hidden.

#11 @joedolson
4 weeks ago

  • Keywords has-patch needs-testing has-testing-info added
  • The Insert/edit link modal does not use the aria-modal attribute (no surprise, since it predates the attribute's support significantly)
  • The page is not made inert, and does not use aria-hidden.

Patch incoming shortly that implements both.

Testing info:

  • Enable classic editor
  • Add text, initiate a link.
  • Click on the 'gear' icon to trigger the link insertion modal.
  • Observe that the modal container has the attribute aria-modal="true" and #wpwrap has the attribute aria-hidden="true". There are other elements that are children of the body and not children of the modal, but they already have display: none; and have their states controlled elsewhere.
  • Observe that the aria-hidden="true" attribute is removed when the modal is closed.

@joedolson
4 weeks ago

Patch to implement aria-modal and aria-hidden for link insertion.

#12 @rajinsharwar
4 weeks ago

I have tested the 60799.diff, and found that:

  1. The Modal Container has the attribute aria-modal="true": https://prnt.sc/pmJQiSinxl14
  2. #wpwrap has the attribute aria-hidden="true": https://prnt.sc/Emm6epnUZeWN
  3. #wpwrap has attribute aria-hidden="true" removed when model closed: https://prnt.sc/63wUzbeE86wY

Overall, this patch looks good and has the issues mentioned fixed.

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


4 weeks ago

#14 @rcreators
3 weeks ago

  • Keywords needs-testing removed

Patch Tested. Works Fine.

Dialogue Open: https://prnt.sc/Zz4AD4jqo1NF

  • wrap getting area-hidden = true

Dialogue Close: https://prnt.sc/A2wT-0qPKY6c

  • wrap getting area-hidden = true removed

Dialogue Box

  • Have area-model = true all the time

#16 @audrasjb
3 weeks ago

  • Keywords needs-testing added

I implemented Joe's patch into the above PR to facilitate testing (with Playground for example).

#17 @hmbashar
3 weeks ago

Test Report

Patch tested: 60799.diff and PR 6816

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.3.8
  • Server: nginx/1.25.4
  • Database: mysqli (Server: 8.3.0 / Client: mysqlnd 8.3.8)
  • Browser: Chrome 125.0.0.0
  • OS: macOS
  • Theme: Twenty Fifteen 3.7
  • MU Plugins: None activated
  • Plugins:
    • Classic Editor 1.6.3
    • FakerPress 0.6.6
    • Test Reports 1.1.0

Actual Results

wrap getting area-hidden = true only for Media Modal https://prnt.sc/Ie--OC2G_7vK and after closing the window the area-hidden will be removed.

but the link inset modal doesn't change https://prnt.sc/ogfTApeXhwKZ

#18 @joedolson
3 weeks ago

@hmbashar I'm not completely understanding your report. Do you mean that if you first make use of the Media Modal, and then use the Link insert modal, then the link insert modal doesn't add `aria-hidden?

#19 @afercia
3 weeks ago

Note: wouldn't be better to update this ticket Summary to better illustrate the proposed change? It's not about changing the heading level any longer.

#20 @sabernhardt
3 weeks ago

  • Summary changed from Change the Header Tag in Link Modal from <h1> to <h2> for Better Accessibility to Fix ARIA attributes for Classic Editor link inserter

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


3 weeks ago

#22 @joedolson
2 weeks ago

  • Keywords commit added; needs-testing removed

I'm unable to reproduce the results reported by @hmbashar; every test I've done adds and removes aria-hidden="true" as specified.

Given the confirming test by @rcreators and multiple confirming tests of my own, I'm inclined to think that the test by @hmbashar may not have had a completed build, so that the JS wasn't correct.

This is a fairly simple patch, and low risk (if it fails to perform the intended job in all cases, it just means that most environments will be improved, and some environments will be unchanged.)

Marking for commit.

#23 @joedolson
2 weeks ago

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

In 58450:

Editor: A11y: Set ARIA attributes for Classic Editor link inserter.

Set aria-modal and aria-hidden attributes when the Classic Editor link inserter modal is active, so that content behind the modal will be properly treated as inert when interacting with the modal.

Props lyonmuller, jwgoedert, sabernhardt, alexstine, afercia, rajinsharwar, rcreators, audrasjb, hmbashar, joedolson.
Fixes #60799.

@joedolson commented on PR #6816:


2 weeks ago
#24

In r58450

Note: See TracTickets for help on using tickets.