Skip to content

Rich Text: Restore selection when focus returns from a popover#77435

Open
kraftbj wants to merge 6 commits intoWordPress:trunkfrom
kraftbj:press-this-issue-116
Open

Rich Text: Restore selection when focus returns from a popover#77435
kraftbj wants to merge 6 commits intoWordPress:trunkfrom
kraftbj:press-this-issue-116

Conversation

@kraftbj
Copy link
Copy Markdown
Contributor

@kraftbj kraftbj commented Apr 16, 2026

What?

Fixes a bug where the cursor jumps to position 0 after inserting a link via Ctrl+K and pressing Escape to close the popover. This is the upstream fix for WordPress/press-this#116. A workaround in Press This is at WordPress/press-this#118.

Why?

When a popover closes and programmatically returns focus to a contenteditable element via .focus(), the browser may reset the selection to position 0. This bug lives in @wordpress/rich-text but is masked in the full Gutenberg editor by two layers of protection:

  1. Iframe isolation: The editor canvas runs inside an [name="editor-canvas"] iframe. When focus moves to the link popover (rendered in the parent frame), the iframe maintains its own selection state independently. On refocus, the browser restores the iframe's selection automatically.

  2. Store-driven correction: The full editor's onSelectionChange dispatch -> useSelect subscription -> useLayoutEffect cycle in useRichText (hook/index.js:193-205) re-applies the correct selection from the block editor store, even if the browser momentarily loses it.

Standalone BlockEditorProvider setups -- like Press This, which renders BlockEditorProvider > WritingFlow > ObserveTyping > BlockList directly in the admin page with no iframe -- lack both of these safety nets. The popover and the contenteditable share the same document, so focusing the popover's URL input clears the contenteditable's selection. When Escape returns focus, the browser places the cursor at position 0.

The onFocus handler in input-and-selection.js calls applyRecord with { domOnly: true } to sync the DOM without overwriting the browser's selection -- a deliberate choice from 2019 (PR #13896) to prevent selection fighting on normal click-to-focus. However, this also means a selection that the browser lost is never corrected. The subsequent handleSelectionChange microtask then reads position 0 from the DOM and overwrites the correct position in the record, completing the corruption.

How?

Adds a restoreSelectionOnFocus flag in the rich-text event listener module (input-and-selection.js):

  1. Set in onFocus: When the element was already selected (the isSelected branch), the flag is set after applyRecord({ domOnly: true }).
  2. Checked in handleSelectionChange: The deferred microtask reads the DOM selection as usual. If the flag is set and the DOM selection doesn't match the record, it restores the record's selection instead of accepting the browser's bogus position.
  3. Self-clearing: The flag is reset on every handleSelectionChange call, so it only affects the first selection sync after focus.

This avoids a synchronous createRecord() call during the focus event (where the selection may not yet be settled) and doesn't affect normal click-to-focus behavior -- in that case the browser's selection is correct, so handleSelectionChange finds a match and the flag is simply cleared.

Slim editor test plugin

This PR includes a test plugin (slim-editor-block-editor) that provides a minimal admin page at /wp-admin/admin.php?page=slim-editor-test with a standalone BlockEditorProvider setup matching the Press This architecture: SlotFillProvider > BlockEditorProvider > WritingFlow > ObserveTyping > BlockList, rendered directly in the page with no iframe. This reliably reproduces the bug and serves as a regression test for standalone editor integrations.

Testing Instructions

  1. Install and activate the "Gutenberg Test Slim Editor Block Editor" test plugin (included in this PR).
  2. Visit Admin > Slim Editor Test (/wp-admin/admin.php?page=slim-editor-test).
  3. Click the appender area and type Before link text after.
  4. Select the words "link text" using Shift+Arrow keys.
  5. Press Ctrl+K (or Cmd+K on Mac) to open the link popover.
  6. Type https://example.com in the URL field and press Enter.
  7. Press Escape to close the popover.
  8. Type MARKER.
  9. Expected: The paragraph reads Before MARKER after (MARKER replaces the selected link text).
  10. Before this fix: The paragraph reads MARKERBefore link text after (cursor jumped to position 0).

Note: This bug does not reproduce in the normal post editor because the iframe preserves the selection. You must use the slim editor test page or a standalone BlockEditorProvider setup like Press This.

Testing Instructions for Keyboard

Same as above -- the entire flow is keyboard-driven.

Use of AI Tools

AI tooling (Claude) was used to investigate the root cause, design the fix, and write the E2E test plugin. All code was reviewed and verified manually.

kraftbj added 3 commits April 16, 2026 12:09
When a popover (e.g. the link UI opened via Ctrl+K) closes and
programmatically returns focus to a contenteditable element, the
browser may reset the selection to position 0. This is especially
visible in standalone BlockEditorProvider setups (like Press This)
where there is no iframe to independently preserve selection state.

The onFocus handler already calls applyRecord with { domOnly: true }
to sync the DOM without overwriting the browser's selection — a
deliberate choice from 2019 to prevent selection fighting on normal
click-to-focus. However, this means a lost selection is never
corrected.

Add a `restoreSelectionOnFocus` flag that is set in onFocus and
checked in the deferred handleSelectionChange microtask. When the
flag is set and the DOM selection doesn't match the record, restore
the record's selection instead of accepting the browser's bogus
position. The flag is cleared on every handleSelectionChange call
so it only affects the first selection sync after focus.

Add a slim-editor E2E test plugin that mirrors how Press This uses
BlockEditorProvider directly (no iframe, no full editor chrome) to
reproduce the bug and verify the fix.

See WordPress/press-this#116
@github-actions
Copy link
Copy Markdown

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.

  • Required label: Any label starting with [Type].
  • Labels found: .

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.

1 similar comment
@github-actions
Copy link
Copy Markdown

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.

  • Required label: Any label starting with [Type].
  • Labels found: .

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.

@github-actions github-actions Bot added the [Package] Rich text /packages/rich-text label Apr 16, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 16, 2026

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: kraftbj <kraftbj@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@t-hamano t-hamano added the [Type] Bug An existing feature does not function as intended label Apr 17, 2026
@Mamaduka
Copy link
Copy Markdown
Member

So technically, this could happen in core when the iframed editor is disabled. I think similar cases are already covered in test/e2e/specs/editor/blocks/links.spec.js though, they most likely run in the iframed editor. Not sure if this warrants a special test suite or if we can reproduce it normally.

@kraftbj
Copy link
Copy Markdown
Contributor Author

kraftbj commented Apr 21, 2026

Thanks for the review, @Mamaduka! You're right that this could happen in core when the iframe is off — I dug into that to see if we could swap the test plugin for something simpler.

TL;DR: We actually do need a truly-standalone BlockEditorProvider setup to reproduce this. The post editor has a second selection safety net beyond the iframe that masks the bug even in legacy-canvas mode.

What I tried

I suspected the existing editor.switchToLegacyCanvas() utility would be enough, so I added a test case to links.spec.js using it:

await admin.createNewPost();
await editor.switchToLegacyCanvas();
// ...same Ctrl+K → Enter → Escape → type MARKER flow

Then I ran it in two states:

Scenario Without the fix With the fix
Post editor, switchToLegacyCanvas() ✅ passes ✅ passes
Slim-editor test plugin (BlockEditorProvider directly) ❌ fails — cursor jumps to 0 ✅ passes

So the legacy-canvas variant of the post editor doesn't actually exercise this code path — the bug only shows up when you go one layer deeper.

Why I think that is

The post editor wraps the block editor in additional providers (EditorProvider, core-data entity layer, etc.) that also participate in selection management. One of those layers appears to re-apply the correct selection from the store after a focus roundtrip, even without the iframe.

Press This and similar integrations skip all of that and render BlockEditorProviderWritingFlowBlockList directly. Without the post editor's store-driven correction layer, the bug in @wordpress/rich-text's onFocus handler surfaces.

Is the test plugin worth it?

I went back and forth on this. The plugin is ~100 lines of JS plus a small PHP admin page, so it's not free. But:

  • It's the only E2E reproduction we have
  • It also doubles as a regression test for the BlockEditorProvider-as-standalone-integration use case, which AFAICT isn't exercised anywhere else in the suite and is a supported way to consume the package

Happy to revisit if you'd prefer a different approach — e.g. a Jest-level unit test against the onFocus handler in isolation, or moving the plugin somewhere less prominent. Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Rich text /packages/rich-text [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants