Synced Patterns: Non-overridable content is erroneously editable in Navigation Mode when using keyboard

[artemiomorales]

Description

Actual: When inserting a synced pattern into a post, if one navigates using a keyboard in Navigation Mode, one is able to press Enter on non-overridable blocks and change their content.

Expected: Non-overridable blocks in should not be editable.

Note that changes to these non-overridable blocks do not persist when a post is saved; even so, it's a confusing experience for anyone navigating with a keyboard.

Step-by-step reproduction instructions

1. Create a synced pattern and add some content (headings, paragraphs, etc.)
2. Enable overrides on some elements, but not others
3. Save the pattern and insert it into a post
4. Tab through pattern using a keyboard
5. On a non-overridable block, press Enter and see that the content can be edited

Screenshots, screen recording, code snippet

keyboard-navigation-synced-patterns.mp4

Environment info

WP 6.5, Gutenberg 18.7.0-rc.1

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

[talldan]

This is also a bug in the site editor when editing pages. A user can edit parts of the template that are supposed to be inaccessible:

Kapture.2024-06-28.at.12.01.09.mp4

In both cases changes can't be saved by a user (at least from WP6.6), so that's a positive.

The code that drives this navigation is here:

gutenberg/packages/block-editor/src/components/block-tools/block-selection-button.js

Lines 190 to 203 in 55f3a98

	let focusedBlockUid;
	if ( navigateUp ) {
	focusedBlockUid = selectionBeforeEndClientId;
	} else if ( navigateDown ) {
	focusedBlockUid = selectionAfterEndClientId;
	} else if ( navigateOut ) {
	focusedBlockUid =
	getBlockRootClientId( selectedBlockClientId ) ??
	selectedBlockClientId;
	} else if ( navigateIn ) {
	focusedBlockUid =
	getClientIdsOfDescendants( selectedBlockClientId )[ 0 ] ??
	selectedBlockClientId;
	}

I think it could be changed to check the blockEditingMode for each block, but it's not completely straightforward, the code will have to be changed to check recursively through the block tree for blocks that don't have the disabled editing mode. For example, when pressing the up or down arrow, we'll have to perform a depth first search on neighbouring blocks to find any editable blocks.

I'm wondering wherther it'd be better to build a filtered version of the block editor tree instead. If we build a tree like this already (e.g. for List View) then it might be more performant and cleaner.

[talldan]

I think this one will need to be punted to 6.7 give the complexity and the fact it was introduced in the 6.5 cycle (or earlier).

[alexstine]

If the goal here is to disable the keyboard interaction, contributors are once again approaching this incorrectly. These blocks are visible visually to show the difference or structure between a page/post and the template it resides in. I agree we should not allow editing if not in the right context but removing the ability for users without sight to explore this is not the right solution. This goes back to my problem with the usage of inert, which still goes unaddressed many releases later.

#54369

Everyone here needs to be on the same page of understanding. If the context is communicated to sighted users, it should also be communicated to users without sight and by restricting keyboard interaction, that is forever lost.

Please come up with a way to communicate to non-sighted users that a block is not editable. Do not restrict how keyboard users navigate.

Thanks.

[talldan]

@alexstine Sighted users also can't see the disabled blocks, the content from the disabled blocks is there, but the blocks themselves are not distinguishable. Users can only see and select the editable blocks.

The goal of the feature is to simplify the block hierarchy by removing the ability to interact with blocks that are not relevant to the document currently being edited (so when editing pages or posts, the template is implicit content that's not editable).

Solving this issue should hopefully bring more parity by fixing something that was overlooked.

That said, I'd appreciate feedback on the fix when it's ready, because I wouldn't want the template content to be completely hidden from screenreaders, and if that's the case then I'd be happy to try something else. The PR is still in the works, but I'll ping you for review when it's ready if that's ok.

[alexstine]

@talldan It is just super confusing to me. I understand sighted users could not edit the block but there must be some type of visual representation or else we wouldn't add it in the DOM, right?

I may not have time to review the PR as I am stepping away from WordPress, this just caught my eye as it looked to be part of the inert issue I linked. Although this does seem different. I just want to make sure whatever is available to sighted users is also somehow communicated to non-sighted users.

Thanks.

[talldan]

this just caught my eye as it looked to be part of the inert issue I linked

On closer inspection I think you're right, it unfortunately is related to that. I didn't know about the usage of inert, though did notice the other day how the screenreader was skipping past the template content and was wondering why.

The way I look at it is that Ideally the block HTML of those disabled blocks would still be available in the accessibility tree, but the blocks that generate that content aren't selectable in any way. Almost like it was serialized on the frontend (but then also without most interactivity/events).

I think it might be better to try solving #54369 before this one in that case. Or finding an intermediate fix that doesn't completely make discovering the content of disabled blocks impossible.

I may not have time to review the PR as I am stepping away from WordPress

Sad to hear that Alex, I've enjoyed working with you over the years.