WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10k stars 4.02k forks source link

Editable blocks have inconsistent ARIA role: sometimes textbox, sometimes document #42158

Open afercia opened 2 years ago

afercia commented 2 years ago

Description

Blocks that have an editable area use the RichText component that renders an HTML element with a contenteditable HTML attribute. This provide users with an area where they can enter text.

Semantically, these editable areas need to be exposed to assistive technology as editable 'fields'.

In terms of ARIA roles and attributes:

The RichText component defaults to role="textbox" and that's correct.

However, with the introduction of useBlockWrapperProps in https://github.com/WordPress/gutenberg/pull/23034 (later renamed to useBlockProps) things have changed and now editable areas have inconsistent ARIA roles.

Note that, initially, useBlockWrapperProps used to pass an ARIA role="group" even though I think the group role was added even before in https://github.com/WordPress/gutenberg/pull/19701. That was incorrect and was fixed in https://github.com/WordPress/gutenberg/pull/33627 by changing the group role to document. However, seems to me that fixed the symptom rather than the root cause. Editable areas should always have a textbox semantics.

Seems to me the root cause is that useBlockProps is used inconsistently. Sometimes, it's used on the RichText wrapper. Sometimes, it's used directly on the RichText component itself. In this last case, the role="document" provided by useBlockProps overrides the RichText role textbox.

I read along the discussion on https://github.com/WordPress/gutenberg/issues/29526 and https://github.com/WordPress/gutenberg/pull/33627 and I see the fix provided there was discussed and tested by the accessibility team. I'd like to hear from the team and have some discussion but I'd tend to think all the editable areas should have a consistent ARIA role of textbox.

Step-by-step reproduction instructions

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

afercia commented 2 years ago

From a technical side: I'm not sure why useBlockProps provides a default prop role: 'document' to start with.

As noted earlier, seems to me useBlockProps is used in a couple ways:

What is the actual purpose of the role: 'document' prop in useBlockProps?

afercia commented 2 years ago

I forgot to ping some members of the accessibility team 🙂 @alexstine @joedolson for some insights on previous testing and discussion. Thanks!

alexstine commented 2 years ago

This is a tricky one because I believe role document allows us to place focus on the block wrapper and have screen readers announce it. I tried removing the role once and it produced terrifying results. In the table block for example, it read the figure tag instead of the aria-label that adds context. There is a lot wrong with how we currently do things in Gutenberg and I hope it gets better with time, but I just do not have the bandwidth to take on much at the moment. If you want to open a quick test PR and change the default role in useBlockProps hook, we can see what happens.

afercia commented 2 years ago

Thanks @alexstine A quick way to test this is by testing a Quote block. In the Quote block there are two editable fields rendered by the RichText component: the Quote text and the Citation text. Both have a role=textbox. The wrapper is a <blockquote> element with role=document and aria-label Block: Quote. I'm not sure this is entirely correct as it overrides the native blockquote role. Also, not sure a document can be labelled. Regardless, the two editable RichText are exposed as textbox, which is the point of this issue. Are they announced with the correct semantics with Windows screen readers?

alexstine commented 2 years ago

@afercia

Regardless, the two editable RichText are exposed as textbox, which is the point of this issue. Are they announced with the correct semantics with Windows screen readers?

Doubt it but cannot say for sure. I can test this weekend if you need but I never really made it as far as testing specific blocks. My main focus so far has been trying to improve the blockers around the editor as a whole. E.g. functionality that effects all blocks and how you interact with the content.

If you think about it though, I am not sure screen readers would be better off announcing blockquote. This seems like it could make the situation worse. It will output a blockquote, but in theory, it is not a blockquote yet. That is how I think about it though.

Thanks.

afercia commented 2 years ago

If you think about it though, I am not sure screen readers would be better off announcing blockquote.

@alexstine agree. I think we're mixing up a bit different topics. Sorry if I wasn't clear. The block wrapper and the block editable area are (or better: should be) two different things.

The RichText component renders the actual block editable area. Basically, it's the equivalent of a HTML <input> or <textarea>. As such, it should use correct semantics and labelling. Since it's editable, it should always have a role=textbox. The actual announcement varies depending on the screen reader. VoiceOver announces it as edit text. It should also be properly labelled, to clarify what users are editing.

The block wrapper is a different thing. I explored a bit previous versions of the editor and I see the block DOM structure has changed a lot over time. I think many of these changes originate from the work done to 'lighten' the blocks but the situation now is pretty confused and the HTML of many blocks is inconsistent.

Sometimes block do have a wrapper element. Sometimes don't. To my understanding, in previous versions of the editor, all blocks used to have a wrapper element. This has a impact when it comes to the way ARIA roles and attributes are applied to the blocks markup.

A quick example based on the Heading block:

In WordPress 6.0, the Heading block doesn't have a wrapper element:

<h2 role="document" aria-multiline="true" aria-label="Block: Heading" tabindex="0" contenteditable="true" ...

I don't mind that much about the H2 semantics being overridden. What's wrong here is:

In WordPress 5.3, for example, there was a a wrapper element:

<div tabindex="0" aria-label="Block: Heading" ...
    <h2 role="textbox" aria-multiline="true" contenteditable="true" aria-label="Write heading…" ...

Overall, the current blocks markup is largely inconsistent and I think it does have an impact on assistive technology. To my understanding there are two main issues here:

This inconsistency is less than ideal from an accessibility perspective.

alexstine commented 2 years ago

@afercia Oh yes, I understand now. I am not sure why not all blocks have wrappers or why some blocks try to have wrappers/content in one. It makes little sense as an insider looking in, you are correct. @talldan @tellthemachines Do you have any idea why this was changed? Cleaning up HTML would be my best guess?

@afercia Have you tried overriding the prop for a single block or even in RichText? Something like this?

const blockProps = useBlockProps( { role: 'textbox' } );

I am fairly positive you can override that property and if not, you can change it for testing at block-editor/src/components/block-list/use-block-props/index.js I believe. I have done a fair amount of work here and other related called hooks.

Would be interesting to see what happens with correct role usage. In theory, screen readers should still switch in to edit mode so I would be happy with that.

talldan commented 1 year ago

@talldan @tellthemachines Do you have any idea why this was changed? Cleaning up HTML would be my best guess?

Pretty much. There has been an effort to make the block HTML in the (backend) editor match the block HTML on the frontend of a site. On the frontend, a paragraph block will only output a paragraph element, so the editor now does the same. It makes it possible for a block in the editor to more closely match the frontend styling.

I can't recall exactly how it worked before in the editor, possibly there were lots of div wrappers.

I think this is the PR that changed things - https://github.com/WordPress/gutenberg/pull/23034.

afercia commented 1 year ago

@alexstine thanks for the hint. Not sure overriding the prop would be the best option as it would require to fix the role on a case by case basis, only when useBlockProps is used directly on RichText.

@talldan thanks for looking into this.

possibly there were lots of div wrappers.

Yep, some block still use div elements or other elements as wrappers. I'm not sure that's the root cause though. It's just that useBlockProps is meant to provide a set of default props to make an element a 'block'. These props include the role=document, which overrides the RichText role prop. Previously, RichText was just using its default role prop.

Update: I noticed the problem of the overridden props is not limited to the role. Sometimes, useBlockProps overrides also the RichText aria-label. For example. in the Heading block, the aria-label is:

aria-label="Block: Heading"

It should be:

aria-label="Heading text"

The first label is the one that used to be set on the block wrapper. It now overrides the label of the editable RichText.

This has an impact on the way the various block are exposed to assistive technology. Actually, the labelling is pretty inconsistent.

vcanales commented 1 year ago

Currently, the RichText component considers role an overridable prop.

https://github.com/WordPress/gutenberg/blob/875628f63a84abc5d60efc727994b75547ab6a5e/packages/block-editor/src/components/rich-text/index.js#L392

I'm wondering if there's a need for this to be so; from an accessibility perspective, is there a valid reason for role to be anything other than textbox in the RichText component? Making it permanently textbox is a — perhaps too naive — way to fix consistency on blocks that rely on this component.

vcanales commented 1 year ago

Furthermore, aria-multiline=true should only be used if role='textbox', but the current logic doesn't make sure that this doesn't happen. This error is caught by axe devTools.

afercia commented 1 year ago

To recap the current situation and further clarify my comment on #47276:

Basically, the presence of the wrapper element and the usage of useBlockProps are greatly inconsistent across blocks. This inconsistency has a deep impact on assistive technologies and affects also other aria attributes and labelling.

afercia commented 1 year ago

A good way to test the inconsistencies across blocks would be comparing the way various screen readers behave on the Paragraph block and on the Quote (or Pullquote) block.

To me, a contenteditable element should always have a `role="textbox'.

Feedback from assistive technology makers would be very very welcome ❤️ 🙏 to make sure that wouldn't introduce weird behaviors for old versions. See the discussion on https://github.com/WordPress/gutenberg/issues/29526 Cc @ggordon-vispero, @michaelDCurran, @seanbudd

alexstine commented 9 months ago

I wanted to revive this discussion as I am currently playing around in a PR where I am looking at removing role="document" all together and letting a blocks implicit ARIA roles take over. I think trying to define a role for every block is too difficult globally so it should be up to developers to ensure their blocks are accessible.

As far as blocks with wrappers and without wrappers, apparently this was done to match the front-end visually and will likely not change so we've got to make the best of it.

salvatoremark commented 3 months ago

@alexstine, do we know where this issue stands today? I tried overwriting the role="document" attribute where it was suggested:

useBlockProps({ role: "presentation" })

But apparently, that feature hasn't been implemented yet? Setting blocks to default to role="document" might be ok as long was we can overwrite the value in useBlockProps().

alexstine commented 3 months ago

@salvatoremark No movement at this point. BTW, why are you adding the presentation role? That will remove all semantics for assistive tech. I can understand the ability to override the role but not with presentation.

salvatoremark commented 3 months ago

This particular block is decretive. From what I understand, the “presentation” role is the same as “none”. The assistive tech would simply ignore it.

Thanks Alex!

On Apr 5, 2024, at 5:44 PM, Alex Stine @.***> wrote:

@salvatoremark https://github.com/salvatoremark No movement at this point. BTW, why are you adding the presentation role? That will remove all semantics for assistive tech. I can understand the ability to override the role but not with presentation.

— Reply to this email directly, view it on GitHub https://github.com/WordPress/gutenberg/issues/42158#issuecomment-2040668640, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARSVYTK6ERTY5EWS6XBISMDY34LKLAVCNFSM52WJNHG2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBUGA3DMOBWGQYA. You are receiving this because you were mentioned.

joedolson commented 3 months ago

Can you provide more detail on how this is decorative, @salvatoremark ? I'd argue that it's not possible for any block to be decorative in the admin; if an editor is working with the document, they always need to be able to identify the block in a meaningful way. It's similar to how images in the media library cannot be decorative, because the purpose of the media library is to display the images in the library.

salvatoremark commented 3 months ago

Yes, I can see that it’s not possible for a block to be decorative in the admin. I was assuming that the role would carry over into the frontend, where it should likely be possible to designate a block as “presentation”. I think Alex suggested it could be passed with useBlockProps.save( { role: “presentation” } ) in save.js.

Just so you know, the blocks I’m building are for my purpose of learning. This question arose while building a block that applies a choice of eye-catching effects to a string of text. It carries no meaning aside from being eye candy, so it could likely fall into the category of presentation.

So back to my original question and the current state of things, the ARIA role=“document” is auto-inserted for blocks in the admin. And as of yet, there’s no way to designate that role, or any ARIA role for a block on the frontend?

On Apr 5, 2024, at 6:11 PM, Joe Dolson @.***> wrote:

Can you provide more detail on how this is decorative, @salvatoremark https://github.com/salvatoremark ? I'd argue that it's not possible for any block to be decorative in the admin; if an editor is working with the document, they always need to be able to identify the block in a meaningful way. It's similar to how images in the media library cannot be decorative, because the purpose of the media library is to display the images in the library.

— Reply to this email directly, view it on GitHub https://github.com/WordPress/gutenberg/issues/42158#issuecomment-2040695031, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARSVYTPJIZRLHGX4PEPP7W3Y34OQLAVCNFSM52WJNHG2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBUGA3DSNJQGMYQ. You are receiving this because you were mentioned.

alexstine commented 3 months ago

Not that I know of. You might be able to do it with index.php file but I do not think you can in save.js. Would need someone else to confirm.