Skip to content

fix(diffs): Handle IME composition input - #823

Merged
ije merged 1 commit into
beta-1.3from
fix-ime-composition-diffs-editor
Jun 17, 2026
Merged

fix(diffs): Handle IME composition input#823
ije merged 1 commit into
beta-1.3from
fix-ime-composition-diffs-editor

Conversation

@necolas

@necolas necolas commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Fixes the following issue:

  1. Open an editable diffs editor and place the caret in content.
  2. Start a CJK IME or accented/dead-key composition.
  3. Type a preview candidate, then commit it or press Esc to cancel.

Previously, each preview beforeinput was prevented and warned as an unknown input type, so the browser could not show native preview text and canceled composition text could still be inserted on compositionend.

Let insertCompositionText beforeinput events stay with the browser. Track composition updates and only commit non-canceled compositionend data into the editor model. Add regression coverage for preview, commit, warning noise, and canceled composition text.

@necolas
necolas requested review from amadeus and ije June 17, 2026 03:03
@vercel

vercel Bot commented Jun 17, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
pierre-docs-diffs Ready Ready Preview Jun 17, 2026 6:41am
pierre-docs-diffshub Ready Ready Preview Jun 17, 2026 6:41am
pierre-docs-trees Ready Ready Preview Jun 17, 2026 6:41am
pierrejs-diff-demo Ready Ready Preview Jun 17, 2026 6:41am

Request Review

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

this.textDoucmentCache.set(file, textDocument);

P2 Badge Use edited contents on full file rerenders

When an edit changes the line count, this remembers the live TextDocument but leaves renderCache.file/this.file pointing at the original contents. If a full file render is forced afterward (for example a theme/options change or cache eviction while the editor is mounted), the highlighter rebuilds code from stale file.contents while processFileResult uses the edited document's larger line count, so inserted lines index past code and the render throws Line doesnt exist. Update the cached file contents or use this TextDocument consistently during full rerenders.


const composedRange = selectionRaw?.getComposedRanges({
shadowRoots: [shadowRoot],
})?.[0];

P2 Badge Guard unsupported composed selection APIs

On browsers or embedded WebViews that do not implement Selection.getComposedRanges (MDN marks it newly available: https://developer.mozilla.org/en-US/docs/Web/API/Selection/getComposedRanges), selecting inside the editor calls an undefined method from every selectionchange and throws before #selections can update, making the editable surface unusable. Check for the method and fall back to getRangeAt() or skip gracefully instead of invoking it unconditionally.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ije ije left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!!!!

Fixes the following issue:
1. Open an editable diffs editor and place the caret in content.
2. Start a CJK IME or accented/dead-key composition.
3. Type a preview candidate, then commit it or press Esc to cancel.

Previously, each preview beforeinput was prevented and warned as an
unknown input type, so the browser could not show native preview text
and canceled composition text could still be inserted on compositionend.

Let insertCompositionText beforeinput events stay with the browser.
Track composition updates and only commit non-canceled compositionend
data into the editor model. Add regression coverage for preview,
commit, warning noise, and canceled composition text.

Also guard the selectionchange handler against browsers and embedded
WebViews that lack Selection.getComposedRanges, which previously threw
out of the listener on every selection change and left the editable
surface unusable.
@necolas

necolas commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Fixed by the getComposedRanges issue by feature-checking the method and bailing out early. I did not use the getRangeAt() fallback. The editor renders inside a shadow root, and getRangeAt() doesn’t read across the shadow boundary like getComposedRanges does, so it would return the wrong range. Added a regression test that reproduces the throw without the guard.

Stale file contents on full rerenders seems to be pre-existing code in the highlight/render path, unrelated to this IME change. The suggested “update cached contents” fix risks regressing highlighting and can be tracked separately.

@ije

ije commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Fixed by the getComposedRanges issue by feature-checking the method and bailing out early. I did not use the getRangeAt() fallback. The editor renders inside a shadow root, and getRangeAt() doesn’t read across the shadow boundary like getComposedRanges does, so it would return the wrong range. Added a regression test that reproduces the throw without the guard.

yep, it's vary trick to handle selection in shadow dom.

Stale file contents on full rerenders seems to be pre-existing code in the highlight/render path, unrelated to this IME change. The suggested “update cached contents” fix risks regressing highlighting and can be tracked separately.

i think this is ok. the editing won't trigger full-rerender with the stable file.

@ije
ije merged commit 559dfc1 into beta-1.3 Jun 17, 2026
8 checks passed
@ije
ije deleted the fix-ime-composition-diffs-editor branch June 17, 2026 07:02
amadeus pushed a commit that referenced this pull request Jun 22, 2026
fix(diffs): Handle IME composition input

Fixes the following issue:
1. Open an editable diffs editor and place the caret in content.
2. Start a CJK IME or accented/dead-key composition.
3. Type a preview candidate, then commit it or press Esc to cancel.

Previously, each preview beforeinput was prevented and warned as an
unknown input type, so the browser could not show native preview text
and canceled composition text could still be inserted on compositionend.

Let insertCompositionText beforeinput events stay with the browser.
Track composition updates and only commit non-canceled compositionend
data into the editor model. Add regression coverage for preview,
commit, warning noise, and canceled composition text.

Also guard the selectionchange handler against browsers and embedded
WebViews that lack Selection.getComposedRanges, which previously threw
out of the listener on every selection change and left the editable
surface unusable.
amadeus pushed a commit that referenced this pull request Jun 24, 2026
fix(diffs): Handle IME composition input

Fixes the following issue:
1. Open an editable diffs editor and place the caret in content.
2. Start a CJK IME or accented/dead-key composition.
3. Type a preview candidate, then commit it or press Esc to cancel.

Previously, each preview beforeinput was prevented and warned as an
unknown input type, so the browser could not show native preview text
and canceled composition text could still be inserted on compositionend.

Let insertCompositionText beforeinput events stay with the browser.
Track composition updates and only commit non-canceled compositionend
data into the editor model. Add regression coverage for preview,
commit, warning noise, and canceled composition text.

Also guard the selectionchange handler against browsers and embedded
WebViews that lack Selection.getComposedRanges, which previously threw
out of the listener on every selection change and left the editable
surface unusable.
amadeus pushed a commit that referenced this pull request Jun 24, 2026
fix(diffs): Handle IME composition input

Fixes the following issue:
1. Open an editable diffs editor and place the caret in content.
2. Start a CJK IME or accented/dead-key composition.
3. Type a preview candidate, then commit it or press Esc to cancel.

Previously, each preview beforeinput was prevented and warned as an
unknown input type, so the browser could not show native preview text
and canceled composition text could still be inserted on compositionend.

Let insertCompositionText beforeinput events stay with the browser.
Track composition updates and only commit non-canceled compositionend
data into the editor model. Add regression coverage for preview,
commit, warning noise, and canceled composition text.

Also guard the selectionchange handler against browsers and embedded
WebViews that lack Selection.getComposedRanges, which previously threw
out of the listener on every selection change and left the editable
surface unusable.
tjni pushed a commit to tjni/pierre that referenced this pull request Jun 25, 2026
fix(diffs): Handle IME composition input

Fixes the following issue:
1. Open an editable diffs editor and place the caret in content.
2. Start a CJK IME or accented/dead-key composition.
3. Type a preview candidate, then commit it or press Esc to cancel.

Previously, each preview beforeinput was prevented and warned as an
unknown input type, so the browser could not show native preview text
and canceled composition text could still be inserted on compositionend.

Let insertCompositionText beforeinput events stay with the browser.
Track composition updates and only commit non-canceled compositionend
data into the editor model. Add regression coverage for preview,
commit, warning noise, and canceled composition text.

Also guard the selectionchange handler against browsers and embedded
WebViews that lack Selection.getComposedRanges, which previously threw
out of the listener on every selection change and left the editable
surface unusable.
amadeus pushed a commit that referenced this pull request Jul 21, 2026
fix(diffs): Handle IME composition input

Fixes the following issue:
1. Open an editable diffs editor and place the caret in content.
2. Start a CJK IME or accented/dead-key composition.
3. Type a preview candidate, then commit it or press Esc to cancel.

Previously, each preview beforeinput was prevented and warned as an
unknown input type, so the browser could not show native preview text
and canceled composition text could still be inserted on compositionend.

Let insertCompositionText beforeinput events stay with the browser.
Track composition updates and only commit non-canceled compositionend
data into the editor model. Add regression coverage for preview,
commit, warning noise, and canceled composition text.

Also guard the selectionchange handler against browsers and embedded
WebViews that lack Selection.getComposedRanges, which previously threw
out of the listener on every selection change and left the editable
surface unusable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants