-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Post editor: iframe for block-based themes #46212
Conversation
Size Change: +397 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
87d90e5
to
4178a01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this is a good idea given the accessibility implications of iFrames. Before it is mentioned, yes, it exists in FSE but that doesn't mean it should. iFrames make the whole interaction model for screen readers much harder. I think we should leave the post editor well alone even for block themes.
I tested the Jetpack map block that had issues when the site editor was first iframed and it works as expected in the iframed post editor so the initial fix implemented for that copies across. Other blocks that are loading scripts but not using |
@alexstine Could you please elaborate on the issues? I'm sure we can solve them. This has been planned for years now, with a lot of work leading up to it. It's also worth noting that many other editors use iframes, including our classic editor TinyMCE. The customiser also uses an iframe. It doesn't help much to just say that it's not a good idea and that it makes things much harder. Would love to hear specifics so we can improve. |
@glendaviesnz Of course, this will for sure get a elaborate Dev Note! 😄 |
Even if the classic editor were wrapped in an iFrame, it would not suffer from the same issues introduced by Gutenberg and therefor, this iFrame is premature for the post editor if there is any hope of not further wrecking UX for screen reader users. Using other editors as an argument does not apply here. Other editors are either not functionally the same as Gutenberg or suffer from equal accessibility issues. I cannot say for certain how long ago the planning started for this but my comments would have likely been the same back then. Do you not find the UX to be worse with Voiceover? I believe Voiceover on Mac requires you to interact with the iFrame as it is seen as a different window. I would bet accessibility with iFrames is probably worse in Mac than Windows. Thanks. |
It is also worth mentioning that iFrames do some weird things with context menus in Windows. For example, it seems impossible for me to inspect elements in the iFrame so I have to open my inspector outside of the iFrame and navigate down the DOM until I find it. I am sure that would never qualify as a blocker, but it would hurt contributors who are trying to improve the project as it would increase the time it takes to debug locally. |
@alexstine Are these issues reported? If not, could you report them so we can fix them? They must be fixed for FSE, template mode, device previews, all of which are iframed. |
@ellatrix I have no idea TBH. I do not have the bandwidth to really focus on FSE so much of it is quite unknown to me. |
391c293
to
be43ef0
Compare
Accessibility issues should be addressed separately and shouldn't be considered as a blocker here. But just to add details for the sake of discussion, iFrames are well supported by most assistive technologies, their content is parseable just as any other HTML content and WCAG simply recommends setting a title to describe what is happening inside the iframe (we could even eventually decide to update the title dynamically to reflect the block selected inside the iframe ex.: "Block editor. Block number 4 "columns" is currently selected"). It should be ordered very high if not first within the main page to ensure it's quickly available. They also require bypass blocks which is usually just an anchor to skip focus onto the first selectable element and bypass repeating elements such as navigation. That's why we often seen links "Skip to content" at the top of many themes nowadays. Most of the issues raised by people about iFrames are due to the lack of control when loading third party websites and such. We, on the other hand, will have full control on the rendering and markup, so we should be able to provide an equally accessible experience as it is right now if not better. iFraming the block editor has been awaited by developers for years (since the beginning of Gutenberg, really) because it's a major headache to scope the CSS styles appropriately, especially since scoped styles were dropped by major browsers and W3C. |
Disagree. Chances of this getting fixed unless it is a blocker is very slim. The project moves too quickly and if these things are active blockers of work that needs to get done, I can get attention on them. Otherwise, it may be a few major releases from now. Agree with your mention of third-party iFrames, this has generally been true. The flip side to what you say is yes we have control over the content rendered but such fixes actually have to be made. This PR puts the cart before the horse. |
I'll gladly look into any accessibility issues.
This makes it sound like these were know issues to solve before this PR. They were not. I don't see any issues related to iframing reported.
I'm sure we can reduce this verbosity? Would be great to have reports of what people are experiencing.
I'm not able to reproduce. After which steps does focus return not work properly?
Is this a bug with the iframe or without as well? I'm trying a columns block at the end of the content and then Shift+Tab from the sidebar and it works fine? Would be good to have steps to reproduce. |
Most of why this happens to be such a big issue for me now is due to the fact that FSE is confusing and really hard to use for keyboard users. I have made the choice to focus around the post editor as I have limited time and limited resources to contribute back to the project. I picked the place I thought I could make the most impact. In theory, if I improve the post editor and general components, FSE should slowly get better. However, not being in FSE since my focus is not there, I only start to get involved when parts of FSE start migrating in to the post editor. Hope this clears up my position. I'll try to open another issue later about the verbosity, but these things are hard to put in words without recordings. Maybe samples from the speech viewer will work out. |
Agree with @alexstine. Isolating accessibility issues is problematic. We wouldn't isolate UX/design/performance and not consider those blockers, and I think what developers want, while important, is significantly less important than what users of assistive technologies want (I say this as a developer who doesn't require assistive technology). That doesn't diminish the work that has gone into this PR. What it should do, is highlight how problematic accessibility issues are in the project, as they block enhancements like this from moving forward. Unblocking a blocker is done by solving the problem, not by declassifying it and kicking it down the road. |
Just opened another issue that would be a really big nice to have. I would not call it a blocker but the experience would be better if it was solved. |
Mmm... we would and we do all the time?
Far from what I meant, I apologize if it was received that way. Regardless, my point was to branch those as separate issues to avoid entangling, and it is happening so that's good, but also to advocate for not blocking, if possible, the iframing progress that has been in the works by Ella for almost 2 years, possibly more. I hope this moves quickly and I'll gladly help testing keyboard tabbability and voiceover. |
@ellatrix Correct, just the verbosity issue. I believe the other issue mentioned is now eliminated. Thanks. |
The block editor's iframe needs to know if the theme is a block theme. This changeset adds a setting to `get_block_editor_settings()`. Reference: * WordPress/gutenberg#46212 Props ellatrix, alexstine, costdev, dgwyer, glendaviesnz, scruffian. Fixes #57549. git-svn-id: https://develop.svn.wordpress.org/trunk@55147 602fd350-edb4-49c9-b593-d223f7449a82
The block editor's iframe needs to know if the theme is a block theme. This changeset adds a setting to `get_block_editor_settings()`. Reference: * WordPress/gutenberg#46212 Props ellatrix, alexstine, costdev, dgwyer, glendaviesnz, scruffian. Fixes #57549. Built from https://develop.svn.wordpress.org/trunk@55147 git-svn-id: http://core.svn.wordpress.org/trunk@54680 1a063a9b-81f0-0310-95a4-ce76da25c4cd
The block editor's iframe needs to know if the theme is a block theme. This changeset adds a setting to `get_block_editor_settings()`. Reference: * WordPress/gutenberg#46212 Props ellatrix, alexstine, costdev, dgwyer, glendaviesnz, scruffian. Fixes #57549. Built from https://develop.svn.wordpress.org/trunk@55147 git-svn-id: https://core.svn.wordpress.org/trunk@54680 1a063a9b-81f0-0310-95a4-ce76da25c4cd
The block editor's iframe needs to know if the theme is a block theme. This changeset adds a setting to `get_block_editor_settings()`. Reference: * WordPress/gutenberg#46212 Props ellatrix, alexstine, costdev, dgwyer, glendaviesnz, scruffian. Fixes #57549. git-svn-id: https://develop.svn.wordpress.org/trunk@55147 602fd350-edb4-49c9-b593-d223f7449a82
The block editor's iframe needs to know if the theme is a block theme. This changeset adds a setting to `get_block_editor_settings()`. Reference: * WordPress/gutenberg#46212 Props ellatrix, alexstine, costdev, dgwyer, glendaviesnz, scruffian. Fixes #57549. Built from https://develop.svn.wordpress.org/trunk@55147 git-svn-id: http://core.svn.wordpress.org/trunk@54680 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@ellatrix: It appears that in latest WordPress core the Gutenberg editor now isolates the styles without CSS selector post-processing, just by injecting the styles into the iframe? So this iframe style isolation feature is used now? |
Right, we no longer scope the CSS because it's no longer needed |
What?
Iframes the post editor for block based themes.
Context: #33346.
Previously #21102.
Fixes #20797.
Fixes #21193.
Fixes #6156.
See #25775 for FSE.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast