Skip to content

[lexical][lexical-playground] Bug Fix: Move caret past trailing characters in accepted macOS text replacements#8414

Closed
nmggithub wants to merge 7 commits into
facebook:mainfrom
nmggithub:fix-macos-text-replacements-on-chrome
Closed

[lexical][lexical-playground] Bug Fix: Move caret past trailing characters in accepted macOS text replacements#8414
nmggithub wants to merge 7 commits into
facebook:mainfrom
nmggithub:fix-macos-text-replacements-on-chrome

Conversation

@nmggithub

@nmggithub nmggithub commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Description

This adds workaround behavior where typing a character to accept a macOS text replacement used to leave the caret before that character instead of after.

(edit: branch name is a bit of a misnomer, as this behavior exhibits in Firefox as well)

(edit 2: this branch now covers the Tab and Enter cases as well, but the title is kept the same)

Closes #8413

Test plan

There is a test added.

Before

Screen.Recording.2026-04-28.at.14.11.05.mov

After

Screen.Recording.2026-04-28.at.14.11.32.mov

@vercel

vercel Bot commented Apr 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
lexical Ready Ready Preview, Comment Apr 28, 2026 10:42pm
lexical-playground Ready Ready Preview, Comment Apr 28, 2026 10:42pm

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 28, 2026
@etrepum etrepum added the extended-tests Run extended e2e tests on a PR label Apr 28, 2026
@etrepum

etrepum commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

pnpm run ci-check is failing, looks like a formatting issue that pnpm run prettier:fix should sort out

@nmggithub nmggithub changed the title [lexical][lexical-playground] Bug Fix: Move caret past trailing spaces in text replacements [lexical][lexical-playground] Bug Fix: Move caret past trailing characters in text replacements Apr 28, 2026
@nmggithub nmggithub changed the title [lexical][lexical-playground] Bug Fix: Move caret past trailing characters in text replacements [lexical][lexical-playground] Bug Fix: Move caret past trailing characters in accepted macOS text replacements Apr 28, 2026
@nmggithub

Copy link
Copy Markdown
Contributor Author

pnpm run ci-check is failing, looks like a formatting issue that pnpm run prettier:fix should sort out

I see no errors on my end for either of those commands.

@etrepum

etrepum commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Your branch is out of sync with main so perhaps you're getting an error that would occur if your branch was updated

@nmggithub

Copy link
Copy Markdown
Contributor Author

Your branch is out of sync with main so perhaps you're getting an error that would occur if your branch was updated

Want me to merge in main before or after this upcoming set of CI runs?

@etrepum

etrepum commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

The CI run is already failing again, it's always a good idea to have your branch in sync with main.

@nmggithub

Copy link
Copy Markdown
Contributor Author

The CI run is already failing again, it's always a good idea to have your branch in sync with main.

Got it! My apologies for not doing so earlier.

@nmggithub

Copy link
Copy Markdown
Contributor Author

On a tangential note, this bug for macOS text replacement where Backspace accepts the replacement still exhibits in Chrome, but I have a patch in for it on Electron (at the very least).

@nmggithub

Copy link
Copy Markdown
Contributor Author

Ok, Prettier should (hopefully) be happy now. Sorry for all the back and forth!

@etrepum

etrepum commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Some of the firefox e2e tests are failing

@nmggithub

Copy link
Copy Markdown
Contributor Author

Some of the firefox e2e tests are failing

Yeah I noticed this. It seems to be mostly my own test. Since it is a macOS-specific test, would it be ok if I just put it behind a platform guard?

@etrepum

etrepum commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Ideally any platform guard or detection of misbehavior (better than a platform guard in case the platform fixes the bug) would be in the workaround and the tests would run uniformly across platforms and browsers. This would make it more likely that we don't accidentally cause regressions in environments that didn't have this particular issue to begin with.

@nmggithub

Copy link
Copy Markdown
Contributor Author

I'll do some testing on my end on Linux to see where the behavior is breaking. If I can get away without a platform guard in the workaround code itself, I'd prefer it.

@nmggithub

nmggithub commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Firefox tests work well for me on Linux (Fedora 43). I know that distro isn't officially supported by Playwright, but I got the tests running and working. I'm not sure what's wrong with the CI runners. EDIT: Seems to be a plain-text-mode discrepancy. Currently investigating. EDIT 2: I've figured it out, but have found more edge cases. Working on those now.

@nmggithub

nmggithub commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Added fix for plain-text mode. I then realized Enter and Tab were still broken so I added logic for those. I added a test for Enter, but left Tab without one as really only Chrome treats that as an acceptance of a text replacement. Not even Safari does it.

Another note I'll add: the vast majority of this workaround code is because Firefox and Chrome fire events in a different order than Safari. The bugs this is meant to address do not exhibit in Safari at all.

@nmggithub

nmggithub commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of a combined effort in #8417.

@nmggithub nmggithub closed this Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: pressing space to accept a macOS text replacement leaves the caret before the space

2 participants