fix: keep the visual editor's insert symbol popup open across decoration-only transactions by kevinushey · Pull Request #1204 · quarto-dev/quarto

kevinushey · 2026-10-20T19:48:24Z

In the visual editor, the Insert Special Characters > Emoji and Insert Special Characters > Unicode popups close on their own about five seconds after the editor is first focused, usually while the user is still typing a search term.

Cause

RealtimeSpellingPlugin (packages/editor/src/behaviors/spelling/spelling-realtime.ts) subscribes to the editor's first FocusEvent and schedules updateSpelling() twice, at 100ms and at 5s. Each call dispatches a transaction carrying only the updateSpelling meta; the realtime-spelling preference only gates the decorations computed from it, so the transaction is dispatched even when spelling is off.

InsertSymbolPlugin (packages/editor/src/behaviors/insert_symbol/insert_symbol-plugin.tsx) closed its popup from the plugin view's update() hook on every transaction. A popup still open when the 5s transaction lands is unmounted under the user.

Call stack from a captured repro (RStudio, Firefox 151): focus -> emitEvent(FocusEvent) -> spelling setTimeout -> updateSpelling -> view.dispatch -> updatePluginViews -> InsertSymbolPlugin.update -> closePopup.

Fix

update(view, prevState) now closes the popup only when the document or the selection changed. Inserting a symbol still dispatches a document change, so the popup still closes after a selection is committed, and clicking elsewhere in the document still dismisses it. Decoration-only transactions leave it alone.

Review also turned up that the plugin's focus listener was registered for 'onfocus', which is not a DOM event, so the popup was never dismissed when focus moved elsewhere. It now listens for 'focusin', like the completion plugin. Because Editor.commands() refocuses the editor after every command unless keepFocus is false, and a toolbar menu takes DOM focus in RStudio, that refocus would have closed the popup before it rendered. The Emoji and Symbol commands therefore pass keepFocus: false; the popup focuses its own search input on mount and already returns focus to the editor on Escape and on insert.

Verification

Harness-free Playwright script against an RStudio dev server, in the Playwright Linux image, with the fix applied to the served panmirror.js by string replacement:

Before: open Insert Emoji right after switching a document to visual mode and idle in the popup. The popup disappears after 3 to 4 seconds in both Chromium 149 and Firefox 151. Typing a 27-character search and pressing Enter fails in Firefox in most runs and passes in Chromium only because it finishes just under the 5s mark.
After: the popup survives a 7s idle in both browsers, and the typed emoji / unicode searches insert the expected characters.

This is what has been failing the "insert special characters" visual editor test in RStudio's Firefox e2e jobs since the panmirror pin moved to release/rstudio-montauk-daisy; the code involved is unchanged there, the newer bundle only shifted the timing.

The realtime spelling plugin dispatches an updateSpelling transaction 100ms and 5s after the editor is first focused, whether or not realtime spelling is enabled. InsertSymbolPlugin closed its popup on every plugin view update, so an Insert Emoji / Insert Unicode popup still open at the 5s mark vanished mid-typing. Only close the popup when the document or selection changed.

gtritchie

Two things to address before merging; details inline.

gtritchie · 2026-10-21T20:57:51Z

 - Fixed a bug where the first character of lines outside code cells (paragraphs, headings, YAML) and of IPython magic lines in Python cells (`%`, `!`) could be highlighted as a comment (<https://github.com/quarto-dev/quarto/issues/985>).
 - Fixed "Quarto: Preview" and render in the terminal when the document path contains shell special characters, such as parentheses, `;`, `@`, `$`, or quotes (<https://github.com/quarto-dev/quarto/pull/1197>).
 - In Positron, the Viewer now shows a stop button for Quarto previews. Clicking it stops `quarto preview` and closes the preview. The button goes away when the preview server stops (<https://github.com/quarto-dev/quarto/pull/1193>).
+- In the visual editor, the Insert Emoji and Insert Unicode popups no longer close on their own about five seconds after the editor is first focused, which could happen while a search term was still being typed (<https://github.com/quarto-dev/quarto/pull/1204>).


1.139.0 has been released since this branch was cut. main now has ## 1.139.0 (Release on 2026-10-20) and an empty ## 1.140.0 (Unreleased) above it. This hunk merges without a conflict, so the entry would land in the released 1.139.0 section. Please move it under 1.140.0.

Merged main and moved the entry under 1.140.0 in 893fdee.

gtritchie · 2026-10-21T20:57:51Z

+          // focused), so only treat a change to the document or selection as
+          // a reason to dismiss the popup.
+          if (!view.state.doc.eq(prevState.doc) || !view.state.selection.eq(prevState.selection)) {
+            this.closePopup();


'onfocus' isn't a DOM event name, so this listener and the one added at line 68 never fire, and focusChanged never runs. completion.ts uses 'focusin' for the same purpose.

This predates the PR, but it matters more now. Previously the popup was sometimes closed after focus moved elsewhere by an unrelated transaction (the spelling updates, a theme change). With this change only document/selection changes, scroll, Escape, and destroy close it. Suggest 'focusin' in both places.

Good catch, switched both to 'focusin' in d906240.

Making the listener live exposed one more thing: Editor.commands() refocuses the editor after a command unless keepFocus is false, and the Emoji/Symbol commands used the default. From a toolbar menu (which takes DOM focus in RStudio), that refocus fires focusin while the popup exists but before it has rendered, and focusChanged closes it immediately. So the two commands now pass keepFocus: false; the popup focuses its own input on mount and already calls view.focus() on Escape and on insert, so nothing else needed the refocus.

…popup-spelling-update

… event 'onfocus' is not a DOM event name, so the focus listener never fired and the popup was never dismissed when focus moved elsewhere. Listen for 'focusin' instead, as the completion plugin does. The editor's command wrapper refocuses the editor after a command runs (keepFocus defaults to true). With a working listener that refocus closes the popup before it renders whenever the command came from a toolbar menu, which takes DOM focus. The popup focuses its own input once mounted, so the two commands now pass keepFocus: false.

gtritchie · 2026-10-21T22:22:20Z

+      // The popup takes focus itself once it mounts, and focusChanged closes it
+      // when focus lands outside of it. Refocusing the editor after the command
+      // would fire that before the popup has rendered, so keepFocus is false.
+      return [new ProsemirrorCommand(EditorCommandId.Emoji, [], performInsertSymbol(key), emojiOmniInsert(ui), false)];


With the focusin listener now working, I think choosing Emoji... or Symbol... from the / omni insert list with the mouse still closes the popup immediately (traced through the code, not run in a browser):

The completion popup is a <div> on document.body with non-focusable rows and no mousedown handler (completion-popup.tsx), so the mousedown moves focus off the editor.

insertCompletion (completion.ts) runs replace, which opens the popup synchronously, then calls view.focus() because neither inserter sets noFocus.

That refocus fires focusin on the editor while the popup is open, so focusChanged closes it before the search input takes focus.

Keyboard selection is unaffected, since the editor already has focus and view.focus() fires no event. Setting noFocus: true in emojiOmniInsert and symbolOmniInsert, as the table inserter does, should cover it.

Agreed, the trace holds up. Both omni inserters now set noFocus: true in 3abffca, matching the table inserter. No regression test: the editor package tests run under plain node --test with no DOM, and the VS Code harness has no bridge into the webview, so this stays a manual check.

Follow-up after actually running this in a VS Code webview (dev build driven over CDP, /emoji then a mouse click on the Emoji... row):

The trace is right, but it did not reproduce as pushed, and the reason is a pre-existing bug that also hides the focusin rename. On the webview document the only focusin listener was Fluent UI's keyborg tracker; neither focusChanged here nor clearCompletions in completion.ts was attached. Both plugins add the listener in the constructor and remove it in the plugin view's destroy. The initial setMarkdown builds a new state with EditorState.create({ plugins: this.state.plugins }), prosemirror-state copies that array, so view.updateState sees a different plugins identity, destroys and recreates every plugin view with the same instances, and the constructor-registered listeners are gone for good.

95e7595 moves both addEventListener calls into the plugin view factory, paired with the existing removeEventListener in destroy. With that in place the scenario you described reproduces exactly (the editor refocus from insertCompletion closes the popup), and 3abffca (noFocus: true on both inserters) fixes it: the popup stays open with its search input focused, and refocusing the editor while it is open now closes it, as intended. Changelog updated to mention that the completion popups now also close on focus change, since that is new behaviour in practice.

…i insert list

gtritchie

Thanks, all three points are addressed. Approving on the condition that the pending CI jobs (Quarto release, Quarto pre-release, Positron API) pass.

… view recreation

kevinushey · 2026-10-21T22:44:48Z

Summary of the changes made in response to review, and what they turned up.

Changes

893fdee moved the changelog entry under 1.140.0.
d906240 switched the insert symbol popup's focus listener from 'onfocus' (not a DOM event) to 'focusin', and made the Emoji and Symbol commands pass keepFocus: false so the editor refocus after a toolbar command does not close the popup before it renders.
3abffca set noFocus: true on the Emoji and Symbol omni inserters, matching the table inserter, so choosing them from the / list with the mouse does not refocus the editor while the popup is opening.
95e7595 registers the focusin listeners of InsertSymbolPlugin and CompletionPlugin in the plugin view factory instead of the constructor, paired with the existing removal in destroy.

Why the last one

Running the mouse-pick scenario in a VS Code webview (dev build driven over CDP) showed that neither popup had a focusin listener on the document at all. Both plugins added it in the constructor and removed it in the plugin view's destroy. The initial setMarkdown creates a new state with EditorState.create({ plugins: this.state.plugins }); prosemirror-state copies that array, updateState sees a new plugins identity, destroys and recreates every plugin view with the same instances, and the listeners were gone for good. So focus-based dismissal never worked, for either the insert symbol popup or the completion popups.

Verified on the final build

The webview document now has focusin listeners for the emoji, symbol and completion plugins.
Choosing Emoji... from the / list with the mouse leaves the popup open with its search input focused.
Refocusing the editor while the popup is open closes it.
Without 3abffca but with the live listener, the popup is closed by the editor refocus, which is the failure described in review.

The completion popups (citations, cross references, / menu) now also close when focus leaves the editor. That is new behaviour in practice and is noted in the changelog.

gtritchie

The focusin change is right, but the scroll subscriptions have the same problem; details inline.

gtritchie · 2026-10-22T01:47:04Z

@@ -65,7 +79,6 @@ export class InsertSymbolPlugin extends Plugin<boolean> {
    this.scrollUnsubscribe = events.subscribe(ScrollEvent, this.closePopup);


The reasoning in the new view() comment applies to this subscription too. It's made once here in the constructor, but destroy calls this.scrollUnsubscribe(), and subscribe really removes the DOM listener (api/events.ts:41-43). After the initial setMarkdown recreates the plugin views, nothing subscribes again, so scrolling the editor no longer closes the popup. The popup is absolutely positioned on document.body, so it stays put on screen while the text scrolls under it.

This predates the PR. Subscribing in view() alongside the focusin listener would fix it, which means scrollUnsubscribe can no longer be readonly and assigned in the constructor.

Confirmed, same failure mode as the focusin listener. Fixed in cf18260: the subscription now happens in view() next to the focusin listener. I kept the unsubscribe function as a local in the view() closure instead of a mutable field, since each view() call is paired with exactly one destroy, so the field could go away entirely. Changelog updated to mention that scrolling now dismisses the popups.

gtritchie · 2026-10-22T01:47:04Z

-    // hide completions when we scroll or the focus changes
+    // hide completions when we scroll
    this.clearCompletions = this.clearCompletions.bind(this);
    this.scrollUnsubscribe = events.subscribe(ScrollEvent, this.clearCompletions);


Same issue as the scroll subscription in InsertSymbolPlugin: subscribed once here, unsubscribed in the view's destroy, and never subscribed again after the first plugin view recreation. So scrolling no longer clears the completion popup. Moving it into view() next to the focusin listener fixes it.

Fixed in cf18260, same approach as InsertSymbolPlugin.

…w recreation The insert symbol and completion plugins subscribed to ScrollEvent once in their constructors and unsubscribed in the plugin view's destroy. The initial setMarkdown recreates every plugin view, so the subscription was gone after that and scrolling no longer dismissed either popup. Subscribe in view() alongside the focusin listener, with the unsubscribe function held in the closure rather than a mutable field.

MathPopupPlugin had the same bug as the insert symbol and completion plugins: the 100ms reposition timer and the scroll and resize subscriptions were created once in the constructor and torn down in the plugin view's destroy, so after the initial setMarkdown recreated the plugin views the equation preview only repositioned on transactions. Create them in view() with the handles held in the closure. Note that this re-enables the 100ms timer, which has been effectively dead after the first setMarkdown. It is cheap: updatePopup bails on the first getMarkRange check whenever the selection is not in math.

kevinushey · 2026-10-22T02:53:31Z

One more instance of the same pattern, fixed in b0c0b73: MathPopupPlugin (marks/math/math-popup.ts) created its 100ms reposition timer and its scroll and resize subscriptions in the constructor and tore them down in the plugin view's destroy, so after the initial setMarkdown the equation preview only repositioned on transactions and sat misaligned after a resize until the next edit. They are now set up in view() like the other two plugins.

Worth knowing: this re-enables the 100ms timer, which has effectively been dead in shipped builds. It is cheap since updatePopup bails on the first getMarkRange check whenever the selection is not in math, but it is the one part of the branch that is not purely restoring a listener. Happy to split it out if you would rather it land separately.

kevinushey added 2 commits October 20, 2026 12:48

add changelog entry

f257f31

kevinushey mentioned this pull request Oct 20, 2026

fix: keep the visual editor's insert symbol popup open across decoration-only transactions (release/rstudio-montauk-daisy) #1205

Closed

gtritchie requested changes Oct 21, 2026

View reviewed changes

kevinushey added 3 commits October 21, 2026 14:27

Merge remote-tracking branch 'origin/main' into bugfix/insert-symbol-…

a339e0d

…popup-spelling-update

move changelog entry under 1.140.0

893fdee

gtritchie reviewed Oct 21, 2026

View reviewed changes

don't refocus the editor after inserting emoji or symbol from the omn…

3abffca

…i insert list

gtritchie approved these changes Oct 21, 2026

View reviewed changes

register the focusin listeners per plugin view so they survive plugin…

95e7595

… view recreation

gtritchie requested changes Oct 22, 2026

View reviewed changes

kevinushey added 2 commits October 21, 2026 19:46

		@@ -65,7 +79,6 @@ export class InsertSymbolPlugin extends Plugin<boolean> {
		this.scrollUnsubscribe = events.subscribe(ScrollEvent, this.closePopup);

Conversation

kevinushey commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cause

Fix

Verification

Uh oh!

gtritchie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gtritchie left a comment

Choose a reason for hiding this comment

Uh oh!

kevinushey commented Oct 21, 2026

Changes

Why the last one

Verified on the final build

Uh oh!

gtritchie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinushey commented Oct 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kevinushey commented Oct 20, 2026 •

edited

Loading