From 98552b2486b52e4dd3c90775c2a029b95e64253c Mon Sep 17 00:00:00 2001
From: Andrea Fercia <a.fercia@gmail.com>
Date: Tue, 27 Feb 2024 16:22:33 +0100
Subject: [PATCH] Fix canvas iframe button accessibility and silent tab stops
 (#59317)

* Improve labeling of the canvas iframe when it has role button.

* Avoid unexpected tab stops in view mode.

* Improve test to make sure there's no silent tab stops.

Co-authored-by: afercia <afercia@git.wordpress.org>
Co-authored-by: carolinan <poena@git.wordpress.org>
Co-authored-by: SergeyBiryukov <sergeybiryukov@git.wordpress.org>
Co-authored-by: abhi3315 <abhi3315@git.wordpress.org>
Co-authored-by: benridane <presents111@git.wordpress.org>
---
 .../src/components/iframe/index.js            | 11 +++++--
 .../components/block-editor/editor-canvas.js  |  9 ++++--
 test/e2e/specs/site-editor/navigation.spec.js | 31 ++++++++++++++++---
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js
index d93f4bf7a548c..3d720f718bc1a 100644
--- a/packages/block-editor/src/components/iframe/index.js
+++ b/packages/block-editor/src/components/iframe/index.js
@@ -107,6 +107,7 @@ function Iframe( {
 	shouldZoom = false,
 	readonly,
 	forwardedRef: ref,
+	title = __( 'Editor canvas' ),
 	...props
 } ) {
 	const { resolvedAssets, isPreviewMode, isZoomOutMode } = useSelect(
@@ -295,9 +296,13 @@ function Iframe( {
 		}
 	}, [ scale, frameSize, marginFromScaling, iframeDocument ] );
 
+	// Make sure to not render the before and after focusable div elements in view
+	// mode. They're only needed to capture focus in edit mode.
+	const shouldRenderFocusCaptureElements = tabIndex >= 0 && ! isPreviewMode;
+
 	return (
 		<>
-			{ tabIndex >= 0 && before }
+			{ shouldRenderFocusCaptureElements && before }
 			{ /* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */ }
 			<iframe
 				{ ...props }
@@ -313,7 +318,7 @@ function Iframe( {
 				// mode. Also preload the styles to avoid a flash of unstyled
 				// content.
 				src={ src }
-				title={ __( 'Editor canvas' ) }
+				title={ title }
 				onKeyDown={ ( event ) => {
 					if ( props.onKeyDown ) {
 						props.onKeyDown( event );
@@ -358,7 +363,7 @@ function Iframe( {
 						iframeDocument.documentElement
 					) }
 			</iframe>
-			{ tabIndex >= 0 && after }
+			{ shouldRenderFocusCaptureElements && after }
 		</>
 	);
 }
diff --git a/packages/edit-site/src/components/block-editor/editor-canvas.js b/packages/edit-site/src/components/block-editor/editor-canvas.js
index c3ee980515e6c..a2b146d8dc95d 100644
--- a/packages/edit-site/src/components/block-editor/editor-canvas.js
+++ b/packages/edit-site/src/components/block-editor/editor-canvas.js
@@ -52,8 +52,11 @@ function EditorCanvas( { enableResizing, settings, children, ...props } ) {
 		}
 	}, [ canvasMode ] );
 
-	const viewModeProps = {
-		'aria-label': __( 'Editor Canvas' ),
+	// In view mode, make the canvas iframe be perceived and behave as a button
+	// to switch to edit mode, with a meaningful label and no title attribute.
+	const viewModeIframeProps = {
+		'aria-label': __( 'Edit' ),
+		title: null,
 		role: 'button',
 		tabIndex: 0,
 		onFocus: () => setIsFocused( true ),
@@ -115,7 +118,7 @@ function EditorCanvas( { enableResizing, settings, children, ...props } ) {
 					}
 				),
 				...props,
-				...( canvasMode === 'view' ? viewModeProps : {} ),
+				...( canvasMode === 'view' ? viewModeIframeProps : {} ),
 			} }
 		>
 			{ children }
diff --git a/test/e2e/specs/site-editor/navigation.spec.js b/test/e2e/specs/site-editor/navigation.spec.js
index 25a5b5dee59ff..4db860b703892 100644
--- a/test/e2e/specs/site-editor/navigation.spec.js
+++ b/test/e2e/specs/site-editor/navigation.spec.js
@@ -45,12 +45,35 @@ test.describe( 'Site editor navigation', () => {
 			page.getByRole( 'button', { name: 'Pages' } )
 		).toBeFocused();
 
-		// Test: Can navigate into the iframe using the keyboard
-		await editorNavigationUtils.tabToLabel( 'Editor Canvas' );
-		const editorCanvasButton = page.getByRole( 'button', {
-			name: 'Editor Canvas',
+		// Navigate to the Saved button first, as it precedes the editor iframe.
+		await editorNavigationUtils.tabToLabel( 'Saved' );
+		const savedButton = page.getByRole( 'button', {
+			name: 'Saved',
 		} );
+		await expect( savedButton ).toBeFocused();
+
+		// Get the iframe when it has a role=button and Edit label.
+		const editorCanvasRegion = page.getByRole( 'region', {
+			name: 'Editor content',
+		} );
+		const editorCanvasButton = editorCanvasRegion.getByRole( 'button', {
+			name: 'Edit',
+		} );
+
+		// Test that there are no tab stops between the Saved button and the
+		// focusable iframe with role=button.
+		await pageUtils.pressKeys( 'Tab' );
 		await expect( editorCanvasButton ).toBeFocused();
+
+		// Tab to the Pages item to move focus back in the UI.
+		await editorNavigationUtils.tabToLabel( 'Pages' );
+		await expect(
+			page.getByRole( 'button', { name: 'Pages' } )
+		).toBeFocused();
+
+		// Test again can navigate into the iframe using the keyboard.
+		await editorNavigationUtils.tabToLabel( 'Edit' );
+
 		// Enter into the site editor frame
 		await pageUtils.pressKeys( 'Enter' );
 		// Focus should be on the iframe without the button role