Skip to content

[Breaking Change][lexical][lexical-extension][lexical-rich-text][lexical-plain-text] Feature: Move triple click selection handling to NormalizeTripleClickSelectionExtension#8520

Merged
etrepum merged 17 commits into
facebook:mainfrom
etrepum:triple-click-overflow-ext
May 19, 2026

Conversation

@etrepum
Copy link
Copy Markdown
Collaborator

@etrepum etrepum commented May 15, 2026

Description

Change the logic of the triple-click selection workaround to precisely address the situation where it will jump over a LineBreakNode and into the start of the next TextNode and/or next ElementNode. The previous workaround would always select the entire containing block of the triple-click.

Moves the logic to a NormalizeTripleClickSelectionExtension extension that RichTextExtension and PlainTextExtension depend on.

Closes #7592
Closes #8490 (a different attempt that doesn't quite address things properly)
Closes #8518 (a more limited in scope draft of this, with no extension)

Test plan

New e2e test coverage

etrepum added 2 commits May 14, 2026 21:26
…cal-plain-text] Feature: Move triple click selection handling to NormalizeTripleClickSelectionExtension
@vercel
Copy link
Copy Markdown

vercel Bot commented May 15, 2026

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

Project Deployment Actions Updated (UTC)
lexical Ready Ready Preview, Comment May 18, 2026 3:41pm
lexical-playground Ready Ready Preview, Comment May 18, 2026 3:41pm

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 May 15, 2026
@etrepum etrepum added the extended-tests Run extended e2e tests on a PR label May 15, 2026
claude and others added 10 commits May 15, 2026 18:11
Logs Node inspector URL, env vars, and timing of two consecutive
page.pause() calls to /tmp/lexical-pause-probe.log. If either pause
returns in under 200ms, that confirms the debugger isn't engaging.

Run:
  pnpm exec playwright test --debug --project=chromium \
    packages/lexical-playground/__tests__/regression/temp-pause-probe.spec.mjs

Click Resume in the Inspector after each pause, then share the log.
…ge.pause() works under --debug

initialize() was firing setViewportSize without awaiting it. Under
--debug, Playwright's Debugger pauses on the first action with
pause:true metainfo (which setViewportSize has) and stores it in
_pausedCall. Because the test code never awaited the call, _pausedCall
stayed set indefinitely, and every subsequent page.pause() hit the
"already paused, bail" early-return in Debugger._pause and resolved
immediately. Awaiting the call lets the initial pause complete normally
before the test reaches its own page.pause().

Also remove the two temp probe spec files that were used to bisect this.
…lize()

The per-test page.setViewportSize() in initialize() was the source of
the page.pause() bug under --debug (the unawaited version poisoned
Debugger._pausedCall). Setting viewport at the playwright config use
level applies it at context creation, so we no longer need to call
setViewportSize per test. IS_COLLAB still determines width since it's
fixed per run via E2E_EDITOR_MODE.
…in tests

Un-awaited page.setViewportSize calls have pause:true metainfo, so
under --debug they fire the channel call, hit Debugger._pause, and
leave Debugger._pausedCall set indefinitely. That causes every later
page.pause() to bail at the "already paused" early-return in _pause.
Awaiting the call lets the pause complete normally before the test
proceeds. Affects Tables.spec and one regression spec.
… calls

An un-awaited Playwright call with pause:true metainfo (setViewportSize,
goto, click, keyboard.*, mouse.*, etc.) leaves Debugger._pausedCall set
indefinitely under --debug, which silently breaks page.pause(). Adds a
no-restricted-syntax rule scoped to lexical-playground __tests__ that
catches the common offenders by AST so we can't reintroduce the bug.
Retrying after a pause during interactive debugging is just confusing.
--debug sets PWDEBUG=1 before the config loads, so we read it here to
short-circuit retries to 0.
… per-test setViewportSize

The 23 Tables/regression calls to setViewportSize({width: 3000}) were
all gated on isCollab with the same "contextual menu positioning" note.
Bumping the collab default viewport to 3000 in playwright.config.mjs
covers all of them. Also drops the redundant setViewportSize in
Images.spec ("Select image, then select text") since its height matched
the default and 1250/3000 already give enough horizontal room.

The only remaining setViewportSize is in AutoScroll's "Pressing Enter
scrolls new caret above the on-screen keyboard" test, which needs a
600x400 mobile-shaped viewport that can't come from the default.
@etrepum etrepum added this pull request to the merge queue May 19, 2026
Merged via the queue into facebook:main with commit 0965867 May 19, 2026
68 of 69 checks passed
@etrepum etrepum deleted the triple-click-overflow-ext branch May 19, 2026 17:54
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: Triple-click make wrong selection

3 participants