quarto-dev / quarto-dev/quarto

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

Open Beginner friendly
#1,099 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
645
Forks
62
Avg merge
17h 42m
Merged PRs (30d)
13

Description

Bug description

AceNodeView treats a programmatic selection restore as a user cursor movement, which makes the visual editor scroll to whichever embedded Ace editor the ProseMirror selection happens to live in.

The changeCursor handler sets cursorDirty without consulting this.updating:

https://github.com/quarto-dev/quarto/blob/828ae28e53b796fb95a33bd7f3c7c109e0709649/packages/editor/src/optional/ace/ace.ts#L531-L548

this.aceEditor.getSelection().on('changeCursor', () => {
  if (this.dom.contains(document.activeElement) && !this.mouseDown) {
    this.cursorDirty = true;
  }
});

this.aceEditor.renderer.on('afterRender', () => {
  if (this.cursorDirty && !this.mouseDown) {
    this.scrollCursorIntoView();
    this.cursorDirty = false;
  }
});

But setSelection() — which ProseMirror calls from selectionToDOM() whenever EditorView.focus() runs — focuses the Ace editor and applies a selection range, and that fires changeCursor:

https://github.com/quarto-dev/quarto/blob/828ae28e53b796fb95a33bd7f3c7c109e0709649/packages/editor/src/optional/ace/ace.ts#L337-L354

public setSelection(anchor: number, head: number) {
  ...
  if (!this.escaping && !this.gapCursorPending) {
    this.aceEditor.focus();
  }
  this.updating = true;
  ...
  this.editSession.getSelection().setSelectionRange(range);
  this.updating = false;
}

this.updating is set here for exactly this reason, and the value-change handlers a few lines above do check it (L508, L513) — the changeCursor handler is the one that doesn't. So the next Ace render calls scrollCursorIntoView(), which does container.scrollTop -= up on the editing root and yanks the whole document to that node view, even though nothing moved the cursor.

The visible damage depends on where the selection sits. It is worst for a freshly opened document: with no saved editing location the selection is at position 0, which is the YAML front matter block — itself an AceNodeView — so any EditorView.focus() scrolls the document to the very top.

We hit this in RStudio as rstudio/rstudio#18490: running a code chunk in a just-opened .qmd snaps the view to the top of the document and then scrolls back. RStudio calls EditorView.focus() as part of chunk execution (to make the editor the active view), which is enough to trigger it.

Steps to reproduce

In RStudio (any recent build, including current main):

  1. Create a .qmd with YAML front matter, a chunk, enough prose to make the document scroll, and a second chunk near the bottom:
---
title: "Untitled"
format: html
editor: visual
---

```{r setup, include=FALSE}
library(stats)
```

## Section one

<a dozen paragraphs of prose>

```{r}
x <- seq(-4, 4, length.out = 100)
head(x)
```
  1. Open it in Visual mode and do not click anywhere in the document.
  2. Scroll down and run the last chunk (toolbar button or Ctrl+Shift+Enter).
  3. Now click once anywhere in the prose and run the chunk again.

Generic form, independent of RStudio: put the ProseMirror selection inside an embedded code editor, scroll that editor out of view, and call EditorView.focus().

Actual behavior

Step 3 snaps the document to the top (to the front matter block's cursor) in a single frame, then smooth-scrolls back to the chunk. Measuring .pm-scroll-container scrollTop across a run:

run 1   before=413   min=74   -> back to 294
run 2   before=294   min=6    -> back to 294

Step 4 stops it: the selection is now an ordinary ProseMirror text selection rather than one inside an AceNodeView, so setSelection() is no longer routed to an Ace editor. It stays fixed until the document is closed and reopened (which discards the saved editing location and puts the selection back at position 0).

Captured stack at the scroll:

AceNodeView.scrollCursorIntoView
  <- renderer 'afterRender'  (cursorDirty)
  <- Ace editor.focus() / setSelectionRange -> 'changeCursor'
  <- AceNodeView.setSelection
  <- NodeViewDesc.setSelection <- selectionToDOM <- EditorView.focus
Expected behavior

Restoring focus should not move the document. scrollCursorIntoView() should only run for cursor movement the user actually caused.

Suggested fix

Adding the same !this.updating guard the neighbouring handlers use is enough:

 this.aceEditor.getSelection().on('changeCursor', () => {
-  if (this.dom.contains(document.activeElement) && !this.mouseDown) {
+  if (!this.updating && this.dom.contains(document.activeElement) && !this.mouseDown) {
     this.cursorDirty = true;
   }
 });

I A/B tested this by serving a patched panmirror.js to the browser while leaving everything else identical:

unpatched   run 1 jump=215px   run 2 jump=163px
patched     run 1 no jump      run 2 no jump

Legitimate scrollback is unaffected — scrolling a focused chunk out of view and pressing an arrow key still brings the cursor back into view in both builds.

Happy to send this as a PR if you'd like.

Your environment
  • IDE: RStudio, development build of rstudio/rstudio main; also reproduced by users on released 2026.07.1+147
  • OS: macOS 26.6.2 (Apple Silicon); reporter of rstudio/rstudio#18490 on macOS 26.5.2
  • panmirror sources checked against quarto-dev/quarto main @ 828ae28e

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/editor/src/optional/ace/ace.ts at the AceNodeView.setSelection method and the changeCursor handler around the referenced lines. Compare the handler with the neighbouring value-change guards, then reproduce the focus/scroll behavior using the described Visual mode document; done means restoring focus no longer scrolls the document, while user cursor movement still scrolls the cursor into view.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
90/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.