Repository navigation
Fix EditContext desynchronization when extensions consume type commands - #348384
Open
Sabyasachi Panda (SabyaXSachi) wants to merge 2 commits into
Open
Sabyasachi Panda (SabyaXSachi) wants to merge 2 commits into
Sabyasachi Panda (SabyaXSachi) wants to merge 2 commits into
Conversation
Registers the extension command override from microsoft#348222 (type is consumed without applying a text edit) and simulates two keystrokes against a real EditContext: the first one replaces the selected EditContext text, the second one inserts a character. Without the fix, the EditContext is not resynchronized to the model after the consumed type command, so the second keystroke is dispatched as replacePreviousChar and corrupts the document. The test fails with the EditContext still containing the first keystroke.
The browser modifies its EditContext natively before firing 'textupdate', and that change was only mirrored back to the EditContext when the triggered 'type' command applied a text edit, because the model change triggered a resynchronization. When an extension consumes the 'type' command without applying an edit, _updateEditContext compared the model text against the previously written text instead of the actual EditContext text, wrongly concluded that both were in sync and left the EditContext containing the typed character. The next keystroke was then diffed against the desynchronized state and dispatched as a 'replacePreviousChar' command, replacing parts of the document with the typed characters. Base the resynchronization decision on the live EditContext text so the desynchronization is detected and healed, and drop the stale mirror. Fixes microsoft#348222
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the EditContext synchronization change.
No open issues 🎉
What changed in this PR
Fixes #348222 by synchronizing the browser’s native EditContext with the editor model when an extension consumes a type command without editing text, preventing subsequent keystrokes from incorrectly replacing document content.
Changes:
- Compares model text with live EditContext text and uses its actual length when resynchronizing, removing the stale text mirror.
- Adds a two-keystroke regression test covering command routing, resynchronization, and preservation of model content.
| File | Description |
|---|---|
src/vs/editor/test/browser/widget/codeEditorWidget.test.ts |
Adds regression coverage for an extension consuming type and cancelling selection. |
src/vs/editor/browser/controller/editContext/native/nativeEditContext.ts |
Uses live EditContext text to detect and repair desynchronization. |
Review effort: Balanced
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When an extension overrides the editor's
typecommand and consumes it without applying a text edit (a common pattern for modal editing extensions), the native EditContext becomes desynchronized from the editor model. The next keystroke is then dispatched as areplacePreviousCharcommand computed from the desynchronized state, which replaces parts of the document with the typed characters instead of typing them. No composition is involved.Root cause
NativeEditContextkeeps the browser's EditContext in sync with the model in_updateEditContext(src/vs/editor/browser/controller/editContext/native/nativeEditContext.ts). The browser modifies its EditContext text natively before firing atextupdateevent, and that change was only mirrored back when the triggeredtypecommand applied a text edit, because the model change triggered a resynchronization (onDidChangeContent→_updateEditContext)._updateEditContextdecided whether to rewrite the EditContext by comparing the model text against_previousEditContextText, the text previously written by VS Code, which is never updated when the browser mutates its own EditContext. When thetypecommand is consumed without an edit, the only resynchronization attempt happens on the selection change caused by the extension (for examplecancelSelection), and that comparison wrongly concludes that both are in sync, leaving the EditContext containing the typed character.On the next keystroke,
_emitTypeEventdiffs against the desynchronized state and computes a spuriousreplacePrevCharCnt, which the command delegate routes to thereplacePreviousCharcommand — the corruption reported in the issue.Solution
Base the resynchronization decision on the live EditContext text (
this._editContext.text) instead of the previously written mirror, and use its actual length for theupdateTextrange. The stale mirror is removed.updateSelection(unchanged order), the browser no longer clamps the EditContext selection in the desynchronized case.Testing
New regression test
issue #348222: EditContext stays in sync when an extension consumes the type command without applying a text editinsrc/vs/editor/test/browser/widget/codeEditorWidget.test.ts:typeis consumed,cancelSelectionruns, no text edit is applied).updateText/updateSelection(a native mutation fires no events) plus syntheticTextUpdateEventdispatch, for two keystrokes over a full-line selection.typewith'x', that the EditContext is resynchronized to the model text, that the second keystroke is dispatched astypewith'y'and not asreplacePreviousChar, and that the model is never modified.Note: the test stubs
ICommandServicewith a wired-upTestCommandServicebecause the instance registered bycreateCodeEditorServicescannot dispatch commands (its constructor argument is not a decorated service and is therefore never provided). The same pattern exists insessionsWorkbenchFixtureUtils.ts.Executed checks (Chromium via
test/unit/browser/index.js, Node 24.22.1 as required by.nvmrc):96bdcf26): failed withAssertionError: "x" === "hello world", i.e. the EditContext still contained the first keystroke — the reported desynchronization.af419a7b): passed.vs/editor/**/*.test.js: 6606 passed, 0 failed (39 pending are environment-dependent skips).npm run typecheck-client: passed.npm run oxlint(full repository): passed.git diff --check: clean.gulp hygienecould not run on this Windows machine:build/gulpfile.hygiene.tscomputesdirNamevianew URL(import.meta.url).pathname, which produces aC:\C:\...path on Windows and fails incheck-package-jsonbefore any file is checked. This is a pre-existing tooling issue independent of this change.Risk
textupdates flow through the same synchronization path, and the comparison is strictly more accurate than the previous mirror (which could also go stale mid-composition). Composition event dispatch andcompositionTypebehavior are unchanged._previousEditContextSelection) is unchanged.Issue reference
Fixes #348222