Skip to content

don't treat programmatic selection changes as cursor movement in AceNodeView - #1100

Open
kevinushey wants to merge 1 commit into
quarto-dev:mainfrom
kevinushey:bugfix/ace-node-view-programmatic-selection-scroll
Open

don't treat programmatic selection changes as cursor movement in AceNodeView#1100
kevinushey wants to merge 1 commit into
quarto-dev:mainfrom
kevinushey:bugfix/ace-node-view-programmatic-selection-scroll

Conversation

@kevinushey

Copy link
Copy Markdown
Contributor

Fixes #1099.

AceNodeView.setSelection() sets this.updating, focuses the Ace editor and applies a selection range. That fires changeCursor, and the handler set cursorDirty without consulting this.updating — so the next Ace render called scrollCursorIntoView() and scrolled the editing root to that node view, even though nothing moved the cursor. The value-change handlers just above (L508/L513) already guard on this.updating; this one didn't.

ProseMirror calls setSelection() from selectionToDOM() on every EditorView.focus(), so any focus restore scrolled the document to whichever embedded editor held the selection. For a freshly opened document that selection is at position 0 — the YAML front matter block — so the document snapped to the very top. In RStudio this shows up as the visual editor jumping to the top of the document whenever you run a code chunk in a file you have just opened (rstudio/rstudio#18490); clicking once anywhere in the prose moves the selection out of an AceNodeView and the symptom disappears until the file is reopened.

Verification

Tested against an RStudio development build by serving a patched panmirror.js to the browser and leaving everything else identical, measuring .pm-scroll-container scrollTop across a chunk run:

run 1 run 2
before jumps 215px to the top, then scrolls back jumps 163px to the top, then scrolls back
after no movement no movement

Genuine cursor-driven scrollback is unchanged: scrolling a focused code chunk out of view and pressing an arrow key still brings the cursor back into view, identically before and after.

Notes

No apps/vscode/CHANGELOG.md entry — aceExtension is only installed when options.codeEditor === 'ace' (editor-extensions.ts:222), which is RStudio's configuration; the VS Code and Positron visual editors use the CodeMirror code view and are unaffected. Happy to add an entry if you'd rather have one.

AceNodeView.setSelection() sets this.updating, focuses the Ace editor and
applies the selection range; that fires 'changeCursor', so cursorDirty was set
even though the user never moved the cursor. The next Ace render then called
scrollCursorIntoView() and scrolled the editing root to that node view.

ProseMirror calls setSelection() from selectionToDOM() on every
EditorView.focus(), so any focus restore scrolled the document to whichever
embedded editor held the selection -- for a freshly opened document that is the
YAML front matter block, at the very top.

Addresses quarto-dev#1099.
@posit-snyk-bot

posit-snyk-bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

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.

AceNodeView: programmatic setSelection() marks the cursor dirty, scrolling the document to the front matter

2 participants