From c29fca42a8152a1d433a83938dabe547250e2451 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 9 Oct 2025 11:02:35 -0500 Subject: [PATCH 1/4] Don't apply selection change for off-canvas text length changes The attempt to keep track of caret position (applyRange) was stealing focus back to the canvas when those changes were coming from an off-canvas input (navigation link block text changes from inspector sidebar input). In these cases, it's not an expected behavior to keep the selection location, as the text length is changing intentionally off canvas. In these cases, it's better to not keep track of that focus. --- packages/rich-text/src/component/index.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index e17a4704d8a370..508c8f174d9424 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -151,8 +151,25 @@ export function useRichText( { } function applyFromProps() { + const previousRecord = recordRef.current; setRecordFromProps(); - applyRecord( recordRef.current ); + const newRecord = recordRef.current; + + // Check if content length changed (text was added/removed, not just formatted) + const contentLengthChanged = + previousRecord && + previousRecord.text?.length !== newRecord.text?.length; + + // Check if focus is on this element + const hasFocus = ref.current?.contains( + ref.current.ownerDocument.activeElement + ); + + // Only skip selection if content changed AND focus is elsewhere + // (e.g., typing in sidebar input) + const shouldApplySelection = ! ( contentLengthChanged && ! hasFocus ); + + applyRecord( newRecord, { domOnly: ! shouldApplySelection } ); } const didMountRef = useRef( false ); From 446ba6f820581ded3e2a239a9223459517abf8ea Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 9 Oct 2025 11:34:50 -0500 Subject: [PATCH 2/4] Add test in firefox to prevent regression of navigation block bug --- .../specs/editor/blocks/navigation.spec.js | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index 17a4118e6f4d1d..51737c799d789e 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -606,6 +606,93 @@ test.describe( 'Navigation block', () => { await pageUtils.pressKeys( 'ArrowDown' ); await expect( navigation.getNavBlockInserter() ).toBeFocused(); } ); + + test( 'should preserve focus in sidebar text input when typing (@firefox)', async ( { + page, + editor, + requestUtils, + pageUtils, + } ) => { + // Create a navigation menu with one link + const createdMenu = await requestUtils.createNavigationMenu( { + title: 'Test Menu', + content: ``, + } ); + + // Insert the navigation block + await editor.insertBlock( { + name: 'core/navigation', + attributes: { + ref: createdMenu?.id, + }, + } ); + + // Click on the navigation link label in the canvas to edit it + const linkLabel = editor.canvas.getByRole( 'textbox', { + name: 'Navigation link text', + } ); + await linkLabel.click(); + await pageUtils.pressKeys( 'primary+a' ); + await page.keyboard.type( 'Updated Home' ); + + // Open the document settings sidebar + await editor.openDocumentSettingsSidebar(); + + // Tab to the sidebar settings panel + // First tab should go to the settings sidebar + await page.keyboard.press( 'Tab' ); + + // Find the text input in the sidebar + const textInput = page.getByRole( 'textbox', { + name: 'Text', + } ); + + // Tab until we reach the Text field in the sidebar + // This may take multiple tabs depending on other controls + for ( let i = 0; i < 10; i++ ) { + const focusedElement = await page.evaluate( () => { + const el = document.activeElement; + return { + tagName: el?.tagName, + label: + el?.getAttribute( 'aria-label' ) || + el?.labels?.[ 0 ]?.textContent, + id: el?.id, + }; + } ); + + if ( + focusedElement.label?.includes( 'Text' ) && + focusedElement.tagName === 'INPUT' + ) { + break; + } + + await page.keyboard.press( 'Tab' ); + } + + await expect( textInput ).toBeFocused(); + await pageUtils.pressKeys( 'ArrowRight' ); + // Type in the sidebar text input + await page.keyboard.type( ' Extra' ); + + // Verify the text was actually typed (change happened) + await expect( textInput ).toHaveValue( 'Updated Home Extra' ); + + // Tab again to move to the next field + await page.keyboard.press( 'Tab' ); + + // Check that focus is still within the document sidebar + const focusIsInSidebar = await page.evaluate( () => { + const activeEl = document.activeElement; + const sidebar = document.querySelector( + '.interface-interface-skeleton__sidebar' + ); + return sidebar?.contains( activeEl ); + } ); + + expect( focusIsInSidebar ).toBe( true ); + } ); } ); test( 'Adding new links to a navigation block with existing inner blocks triggers creation of a single Navigation Menu', async ( { From 0fc1fff74cdf333b9343571ec2d1e5315a23b15d Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 9 Oct 2025 11:51:32 -0500 Subject: [PATCH 3/4] Simplify logic for when to skip selection tracking --- packages/rich-text/src/component/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 508c8f174d9424..6e29ff6f8d4461 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -165,11 +165,11 @@ export function useRichText( { ref.current.ownerDocument.activeElement ); - // Only skip selection if content changed AND focus is elsewhere - // (e.g., typing in sidebar input) - const shouldApplySelection = ! ( contentLengthChanged && ! hasFocus ); + // Skip re-applying the selection state when content changed from external source + // (e.g., typing in sidebar input changes canvas text) + const skipSelection = contentLengthChanged && ! hasFocus; - applyRecord( newRecord, { domOnly: ! shouldApplySelection } ); + applyRecord( newRecord, { domOnly: skipSelection } ); } const didMountRef = useRef( false ); From 5b63b2f509d44f2d97add7728ca92393e7c184b5 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 9 Oct 2025 14:37:38 -0500 Subject: [PATCH 4/4] Simplify --- packages/rich-text/src/component/index.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 6e29ff6f8d4461..7336eb2e63da8a 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -151,14 +151,17 @@ export function useRichText( { } function applyFromProps() { - const previousRecord = recordRef.current; + // Get previous value before updating + const previousValue = _valueRef.current; + setRecordFromProps(); - const newRecord = recordRef.current; // Check if content length changed (text was added/removed, not just formatted) const contentLengthChanged = - previousRecord && - previousRecord.text?.length !== newRecord.text?.length; + previousValue && + typeof previousValue === 'string' && + typeof value === 'string' && + previousValue.length !== value.length; // Check if focus is on this element const hasFocus = ref.current?.contains( @@ -169,7 +172,7 @@ export function useRichText( { // (e.g., typing in sidebar input changes canvas text) const skipSelection = contentLengthChanged && ! hasFocus; - applyRecord( newRecord, { domOnly: skipSelection } ); + applyRecord( recordRef.current, { domOnly: skipSelection } ); } const didMountRef = useRef( false );