Iframe: try role=application #53364
Iframe: try role=application #53364
Conversation
ellatrix
added
the
[Focus] Accessibility (a11y)
|
Size Change: +20 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
@ellatrix Thanks for trying this out. The initial results are promising.
The biggest problems I notice come from content that is rendered outside of the portal/iFrame. We could reduce a lot of verbosity if we were able to keep more inside the iFrame.
Is there any conceivable way to move the block toolbar inside the iFrame? It should really be attached to each block, the currently selected block, but I know discussions before indicated this is not really possible.
This is for sure an improvement as the screen reader only announces the application now vs. the iFrame, content body, and document of the block. We could make it even better if we could figure out how to keep the most used tools inside the iFrame that way screen reader does not announce every time you leave/enter again. The sidebar has to stay outside of the iFrame for obvious reasons but I wonder if the block toolbar could somehow join.
I would also like feedback from @afercia about how this behaves on Mac. It should be a better experience but I want to make sure of that.
Things are still crazy with the navigation block and I'm not sure why. The role="application" should force Windows screen readers into focus mode at all times unless the user changes it. However, after inserting the navigation block, focus gets placed on the block appender causing a mode change. I'm not sure if this is some bug I found in NVDA or maybe if this can be caused when focus is placed directly on a button, even inside of the application role. Either way, the PR does not fix this bug and we're going to need to explore this in another PR/issue. I was just silently hoping the insane navigation block might see some improvements from this.
Moving forward:
- Wait on @afercia review for Mac.
- See if there is any good way to move the block toolbar into the iFrame DOM. Maybe try to position it above the title field, as it is now? Might have to play around with the toolbar tabindex to ensure focus does not enter it when tabbing into the application.
Removing the title also made things a lot quieter. I prefer it this way and hopefully it can stay this way. I think we are okay since the application is the first item in the Editor content region.
|
Rendering the block toolbar inside iframe defeats some of its purpose: a boundary between content and editor UI. We don't render the toolbar in the iframe for the same reason that we don't render the inspector in the iframe, it's not content but an interface for controlling the content that is also styled differently. The iframe should be seen as ideally identical to the front end. Maybe the editor as a whole should be wrapped into role application? |
There has always been a lot of thought about this but also a lot of hesitation. When the whole editor is wrapped in this role, we'd have to manage every last keyboard event perfectly. I think at this point, Slack only wraps the message list and threads list but nowhere else. As I mentioned, adding it to the iFrame is certainly a good start but I just wanted to explore if it was possible to add the block toolbar because that would eliminate a bit more verbosity. One thing worth noting, the toolbar is actually inside the application role for the classic editor. Any idea how that is being done? Thanks. |
TinyMCE wraps its whole editor in role=application, which includes the toolbar and the TinyMCE iframe. That's why I suggested wrapping everything in it. Maybe an alternative is wrapping the iframe but additionally the toolbar as well? |
|
@ellatrix If it's easy enough to try that, I think we should. |
|
It is important to understand that role=application is risky, as it 'nullifies' any semantics perceived by screen readers, or at least by some of them. As such, many native screen readers features don't work any longer when inside a role=application. For example:
Basically, most native screen readers navigational tools will not work any longer. I think role=application is OK to use only in very limited cases, only as last resort, and only on small parts of the UI, when there is the need to force screen readers to switch to a sort of 'focus mode' rather than stayin in browse mode (note the terminology varies dependindg on the screen reader). Such usage has been done for example for the List View, see #44291. In that case, role=application may help and it's limited to only this specific part of the UI. The List View is a complex widget and it's more usable when screen readers stop intercepting key strokes. However, setting role=application on the entire Editor would have, in my opinion, a terrible impact as most native screen readers features would not work any longer. I'd strongly recommend against this proposal. |
|
@afercia I tend to agree and that's why this started out as just wrapping the editor content area. I still think it would be wise to somehow place the block toolbar in the editor content region to reduce verbosity moving out/in of the iFrame. The big problem with the editor overall is most of the interactions you described with screen readers don't really work in Gutenberg to begin with, hints the idea that maybe we should be managing the entire experience. For example, switch to browse mode in NVDA after inserting some blocks and tell me if you can tell them apart. You can't because it is largely a bunch of divs with labels, no semantic elements. I think efforts should continue here to move the block toolbar and we'll wait on trying Thanks. |
|
Yes, this is only using it on the iframe, which mean it's only for editor content.
Tbh, it makes sense to me that all these things don't work in the editor content. This is content to be edited, not elements that are part of the interface. Regarding the toolbar, I still don't quite understand the benefit of role=application. Is that verbosity when switching useless? It seems important to know when it's switching from content to UI? I can try wrapping the toolbar separately. Would that work? Or does it have to be part of the same role=application parent element? |
|
Is it worth adding role=application first to the iframe, then see if we expand it to toolbar? Or is it not so useful for just the iframe? |
Not sure how familiar you are with Microsoft Word, but there is actually an option with screen readers to have the best of both worlds. Screen readers can either operate in edit mode where the whole document is a canvas or they can operate in browse/virtual mode where you can jump to lists, tables, links, etc. The problem with Gutenberg is it gives sighted people the idea of a wide open canvas but it isn't that for screen reader users. It is a bunch of blocks that all have different interaction models. For example, its not like Gutenberg is one huge text field that you can insert different elements into, it is a bunch of parts that make a final picture. A practical example might be inserting a table block. Visually, it probably looks like a table would on the front-end but it does not have actual table markup in the editor. This makes it fairly difficult for screen reader users to know at any given time what row or column they are editing in and it takes away the ability to use virtual navigation techniques to move around in the block. On the front-end, you can press "T" to find the first table and then move around the columns and rows with table navigation commands.
It becomes annoying in this situation because every time you enter the toolbar and then return to the iFrame, the screen reader announces the surrounding context of re-entering the iFrame. If I want to bold text in a paragraph block, I don't need to hear that I am entering the iFrame again as I never expected to leave it in the first place. I expect to hear I am in a formatting bar but it is reasonable to expect that if I am editing a block, I will return to said block. Its a balance between not enough information and too much information. It would be great if the toolbar can appear in the DOM, just inside the iFrame so that extra verbosity goes away on return to editing content. Thanks. |
|
Here is a quick example that might help highlight my point. As you can see, this adds very clear structure to each block and because the table block in my example is coded like an actual table, users could use screen reader virtual mode to navigate to the different fields. Instead of just rendering divs with the <html>
<head>
<title>Test</title>
</head>
<body>
<main>
<h1>Gutenberg</h1>
<h2>Post Title</h2>
<textarea name="post_title" aria-label="Post Title"></textarea>
<h2>Paragraph</h2>
<textarea name="paragraph_block_1" aria-label="Paragraph Block">Some paragraph text</textarea>
<h2>Table Block</h2>
<table>
<tr>
<th><input type="text" name="table_block_1_row_1_header_1" value="Table Header 1" /></th>
<th><input type="text" name="table_block_1_row_1_header_2" value="Table Header 2" /></th>
</tr>
<tr>
<td><input type="text" name="table_block_1_row_2_cell_1" value="Table Cell 1" /></td>
<td><input type="text" name="table_block_1_row_2_cell_2" value="Table Cell 2" /></td>
</tr>
</table>
</main>
</body>
</html> |
|
The other problem we must solve for at some point is figuring out how to communicate block previews to screen readers. Currently, the only way to make |
I tracked this down to the https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/document_role
There is nothing to read within the navigation block so this role is actually an anti-pattern. I know @afercia has brought this up specifically before. I don't see much downside to removing it but certain blocks such as table get a little weird due to the announcing of the figure element the block is wrapped in. Part of me wants to remove this role and deal with the fallout vs. keeping this role and all the pain that comes with it. If you insert a paragraph block, it gets the role of textbox after removing the document role. Textbox role is for sure more valid than the generic document role defined as a required value in I think I'll play around a little with this over the weekend and see what the support looks like across Mac and Windows. I know there has to be a reason for the generic document role but I wonder if said reason is still valid these days. |
| @@ -183,7 +183,6 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) { | |||
| ...props, | |||
| ref: mergedRefs, | |||
| id: `block-${ clientId }${ htmlSuffix }`, | |||
| role: 'document', | |||
There was a problem hiding this comment.
Eliminating this may be actual trouble for a little while until we could work through all blocks and fix any accessibility issues. The problem is, keeping this is hurting us because trying to define a generic role for all the different blocks is not a reasonable solution. For example, this role makes no sense for the paragraph block because the paragraph block is an editable field and this role tells Windows screen readers to go into a mode which is not used for editing text, but navigating the virtual buffer of the page. Removing this also fixes the bug in the navigation block and other dynamic blocks where screen reader switches modes.
There was a problem hiding this comment.
It looks like we're always overriding the role to document which seems like a bug. Some blocks like paragraph define role=textbox (from rich text), but useBlockProps is overriding this. Maybe we should only set role=document if no role was defined by the block?
There was a problem hiding this comment.
@ellatrix I don't think that is possible at this point due to my comments around the navigation block. I think this should likely be managed block by block until we can find a suitable role that could work for any type of content. The document role causes Windows screen readers to switch modes and then the keyboard events no longer work in React so it would be good to avoid this pattern. I think it used the group role before this change and it needed to be modified for a bug in JAWS, another Windows screen reader.
There was a problem hiding this comment.
If you're sure about removing role=document from all blocks, I'll update the e2e tests. There's lots of tests depending on this in their selectors.
There was a problem hiding this comment.
Let's wait on a good accessibility review from more people than just me before you commit to that. I will try to wrangle some testing here.
| @@ -259,7 +260,7 @@ function Iframe( { | |||
| // mode. Also preload the styles to avoid a flash of unstyled | |||
| // content. | |||
| src={ src } | |||
| title={ __( 'Editor canvas' ) } | |||
There was a problem hiding this comment.
Turns out this is actually an accessibility violation. This isn't a rule I agree with considering the iFrame is wrapped in a role="region" with a valid aria-label.
@afercia You think this is something we can get away with given the situation?
Having a region labeled as Editor Content followed by the iFrame labeled as Editor Canvas is very verbose and not helpful. The other option, remove the aria-label from this region and the title should be good enough since it is the only element?
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/region_role
Regions seem to require accessible labels so really struggling here as to how labeling should:
A. Not be annoying.
B. Not be an accessibility violation from the lack of labeling.
|
The table in the editor is valid table markup? It's not divs. Here's the markup: <table>
<tbody>
<tr>
<td role="textbox" aria-multiline="true" aria-label="Body cell text" class="block-editor-rich-text__editable wp-block-table__cell-content rich-text" contenteditable="true"></td>
<td role="textbox" aria-multiline="true" aria-label="Body cell text" class="block-editor-rich-text__editable wp-block-table__cell-content rich-text" contenteditable="true"></td>
</tr>
<tr>
<td role="textbox" aria-multiline="true" aria-label="Body cell text" class="block-editor-rich-text__editable wp-block-table__cell-content rich-text" contenteditable="true"></td>
<td role="textbox" aria-multiline="true" aria-label="Body cell text" class="block-editor-rich-text__editable wp-block-table__cell-content rich-text" contenteditable="true"></td>
</tr>
</tbody>
</table>
And I guess removing the title attribute doesn't fix this. The next thing to try that I'm hoping works is adding role=application to just the block toolbar. But it is important to understand that you are leaving the block canvas? You don't want to think you're editing the block toolbar? Visually that is understood because it's styled like the admin and it floats around above the canvas, I'm not sure how that would make sense non visually.
I don't understand what you mean by communicating block previews. Could you elaborate? |
|
@ellatrix Table might have been a bad example. I believe the table specifically doesn't work due to the fact it is wrapped in a Block previews. If you deselect the block with the mouse, some blocks can render different content based on the Yes, what you mention about the block toolbar does make sense. Also, removing the CC @joedolson for further input. I could use some assistance thinking through the block toolbar and how to best present it to screen reader users. We're making progress. Thanks. |
But how come the table works fine on the front-end, where it is also wrapped in a figure? |
ellatrix
force-pushed
the
try/iframe-role-application
branch
from
c624164 to
60f81cc
Compare
|
@alexstine I rebased this PR and added role=application to the block toolbar's popover. Can't add it to the toolbar directly, I think, because it already has role=toolbar? I have no idea what I'm doing... 🙂 |
|
@ellatrix I'll test this weekend, no worries. Not sure why the table block loses its semantic meaning on the back-end and it works fine on the front-end. There are other blocks that have this problem though and its worth looking into why. Probably elsewhere. If you opened up the editor today, inserted a table block, and then used your quick navigation keys to find the table, it would not work. Sorry for drifting though. I need to keep focus on the current issue so eventually I'll explore this elsewhere. Thanks. |
|
It'll probably need auditing globally; but removing Re: the table block. I assume that all of the aria roles are breaking the table semantics. A table cell with I did a quick test on CodePen, and a table where all the cells have |
|
Thanks @joedolson. That gave me just enough to figure out the table issue. Opened another PR to take care of it. #54324 I will continue with testing this PR specifically over the weekend. |
|
@ellatrix I think its okay to revert changes here.
I think my end goal was to have the toolbar inside the I think the changes to Thanks. |
|
Thie Pulls request is trning into a very long discussion that is hard to read with my limited vision. Apoligiesin advance if I'm missing something important.
@alexstine Since the very beginning of this project we tried to make all the blocks based on RichTexr be perceived as textarea elements. It was our idea that was the most intuitive and exepected semantics and interaction. To do so, the RIchText, which is actually an editable field, usd to use role="textbox" and aria-multiline="true". Tha also made screen readers switch to focus mode, which is expected. I would like to remind you that you wanted to change role="textbox" to role="document" to solve some JAWS bug. Of course, role=document makes these editable divs have no semantics and makes screen readers stay in browse mode. I think that should be reverted to role="textbox". In my experience, trying to solve browsers or assistive technologies bugs by hacking around is often not ideal as it often introduces other problems.
@ellatrix i deeply disagree with this point. Nullifying important native screen readers functionalities when inside the content is a no-go for me. This is not a Word process. It is still a web-based application. It does uses form elements, or the ARIA equivalents of them. Pretending to make it behave likw Microsoft Word may work visually, but it will never work semantically and for accessibility. @alexstine To me, the introduction of the iframe is maybe one of the most wrong decisions ever made in the editor. Users needs should prevaile on developers need. The iframe was only introduced to avoid CSS 'bleeding' between CSS used for the content and the CS used for the interface. While there is some value in that, the iframe is a subpar experience for assistive technology users and shouldn't have been introduced in the first place. Users needs above all. Technically, the
This isn't great. As you pointed out, each time users will move from the iframe to the block toolnbar or notice area and back, the will hear the iframe being announced. Sometimes the document title will be announced as well. Actually, the editor is now made of two separete documents. Asrhitecturally, not the best decision I would say. I would totally agree that at least the floating block toolnbar and the content should live in the same document. That would solve a part of the root issue. Removing the title attribute from the iframe could alleviate the verbosity problem but it would be detected as a violation by any automated accessibility checker. Not opposed to experiment that if it actually helps users.
Not sure how that would ever help. The toolbat already has a role=toolbar, which triggers a sort of focus mode for screen readers thusa allowing the expected keyboard interaction. Glad it was removed.
As @joedolson pointed out, the unexpected roles on the table cells have likely a deep impact. Worth also reminding that some CSS properties actually nullity the table semantics. |
Agreed. I fully own that this was my problem as I made the change. However, I am not sure the role it had before, Docs: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/group_role I can kind of see where blocks are groupings of sorts but blocks are not
I removed the group role in favor of document role and now I suggest no role. I am not sure when these blocks ever had the textbox role but it was before my change as the diff shows. I fixed the table block in a follow-up PR: #54324. I agree about the regions. As far as the Microsoft Word editing experience, my comment here explains why Gutenberg is not TinyMCE. The entire canvas is content editable where Gutenberg is not. Re, long PR, I know. Sorry about that. I have a lot of ideas on how to improve the experience without much background from when Gutenberg started. I should have thought better of using this PR as my creative canvas. Thanks. |
@afercia It's not only for CSS bleeding, that's just a nice side effect. The main problem this solves and is the ability to preview relative CSS units correctly. Without this iframe these are broken. As you may remember, the classic editor was also iframed. We made the mistake to not take this over from the start. |
|
| @@ -183,7 +183,6 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) { | |||
| ...props, | |||
| ref: mergedRefs, | |||
| id: `block-${ clientId }${ htmlSuffix }`, | |||
| role: 'document', | |||
There was a problem hiding this comment.
Could try to change this back to group role but would need to land this first.
If not, JAWS compatibility may break.
|
Flaky tests detected in 7d23eb3. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8919106948
|
…e-role-application
|
If this needs JAWS testing, I can do a bit of that; I have a JAWS license, at any rate, though I'm far from proficient with it. Still, I could try and identify possible problems introduced by this change. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
@joedolson Let me try to refresh this Friday or over the weekend and we can both do some more testing. At some point, we'll need to get into a state where the iFrame is an application and I remove the document role from blocks. That was my sin sadly trying to fix older JAWS versions so I think we'll set it back to group for non-content editable blocks. For content editable blocks, I say we let the rich text role take over which is textbox in most cases but can have additional attributes such as I also agree with @afercia, an issue is probably needed but at this point, there are an endless number of issues that exist from my bad choice, the person before me who made a bad choice, and then of course, the iFrame. Might be best to close out this PR and draft a new one so I can organize my thoughts. I don't always do the best in writing, that's the dev in me I guess. |
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
|
@joedolson Okay, I think this is finally ready for some intense screen reader testing. Summary:
Please let me know your thoughts. Early testing is looking good with NVDA. Interested to hear if JAWS behaves okay. Thanks. |
What?
Let's try role=application on the iframe. See #46260 (comment).
This PR also makes an adjustment to the
RichTextcomponent. The component will now userole="textbox"by default with the option to override if necessary. However,role="group"is no longer a valid property here to avoid it being passed down fromuseBlockPropshook directly onRichText. Code comment added to explain.Why?
To fine tune accessibility.
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast