NodeView selection is lost if div.ProseMirror is altered on tab focus

We’ve discovered some strange behavior that I wanted to discuss before filing a bug. A 3rd party library in our application is listening to blur/focus events and on focus, they are setting their own data-attribute on our root ProseMirror div. (I believe they are binding to document.activeElement, which at the time that their code runs is the editor).

The result is that, if a NodeView is selected when you switch tabs away and return, the editor selection is recomputed and actually moves to the previous block (out of the NodeView).

Here’s a GIF showing the behavior and a link to a Codesandbox.

While I understand that it would be preferable that the 3rd party doesnt alter DOM elements that it isn’t supposed to, we also reproduced this with a Chrome extension.

Is it a bug that the selection is changing? Is there any way we can ignore this mutation that happens directly on the ProseMirror root itself?

CodeSandbox: ProseMirror Node View Example - CodeSandbox

prosemirror nodeview bug

Are you able to work around this by defining an ignoreMutation method on your node views that recognizes this kind of attribute change and ignores it?

No, because the mutation is happening on the view root (.ProseMirror), not a nodeview inside.

I’m encountering some other bugs where nodeselections are lost when blurring the view as well. Do you have any ideas for why, on focus changes, a nodeselection would turn into a textselection? It seems like the observer is trying to recreate a selection but can’t recognize a nodeselection from the DOM itself, and therefore finds the nearest text.

Can that be reproduced in a minimal setup? If so, I’ll gladly debug it.

I haven’t been able to create a minimal version yet, but will keep trying.

Conceptually though, could you tell me if my understanding of what’s happening in domobserver.js is correct?

  • Initially, there is a NodeSelection
  • Whenever the DOMObserver notices a change in the doc, it runs view.root.getSelection()
  • This will always return a text selection, because node selections aren’t a native selection concept
  • If no parent nodeview surrounding the new text selection (not the atom node that was previously selected) prevents the mutation via ignoreMutation, this text selection then becomes the new selection

If my understanding is right, it seems that any DOM mutation that happens inside will break a NodeSelection, and the logic here is assuming that DOM mutations would only happen as a result of user input. But if a browser extension or other library reaches in to modify the DOM, that could trigger unintended nodeselection loss.

Does that sound right, or am I missing some built-in “defenses” for keeping NodeSelections in these cases?

Only if that change ends up modifying the DOM selection—if it leaves it the way it was, the current selection is preserved.

The DOM selection reader is capable of creating a node selection in some circumstances, but indeed, because there’s no clear representation of a node selection in the DOM, reading selections back from the DOM is lossy.

The library works with the assumption that the DOM selection doesn’t change unless something is explicitly moving it (in which case it needs to re-read the selection from the DOM). It’s possible that the extension in this scenario is doing something which violates this—though on the surface of it, it seems unlikely that adding an attribute would cause this.

Thanks very much for the explanation @marijn , I think with that info I’ve been able to narrow down the problem

Only if that change ends up modifying the DOM selection—if it leaves it the way it was, the current selection is preserved.

which I assume refers to this line in domObserver.js:

let newSel = !this.suppressingSelectionUpdates && !this.currentSelection.eq(sel) && hasSelection(this.view) && !this.ignoreSelectionChange(sel)

When I put a logger in, it turns out that this.currentSelection is an empty dict {} when the second check runs. Searching through the code, I found this section of input.js:

handlers.blur = view => {
  if (view.focused) {
    view.domObserver.stop()
    view.dom.classList.remove("ProseMirror-focused")
    view.domObserver.start()
    view.domObserver.currentSelection.set({})
    view.focused = false
  }
}

So, when the view is blurred, we empty out the currentSelection to {}. Then when a library or extension modifies the DOM itself, we interpret the selection in the DOM as a change from the empty base. This then causes us to try to read the selection back from the DOM, which then returns a TextSelection (I’m not sure why.)

Does this sound right to you? If so, do you think there’s a way for us to avoid the currentSelection resetting on blur, and/or change selectionFromDOM to recognize the NodeSelection?

When the editor doesn’t have focus, it shouldn’t be reading DOM selections at all — the idea is that DOM-to-editor selection syncing only happens when the editor has focus. Is that going wrong in this case?

Hmm, I don’t think so, but it may be insufficient protection. We’re seeing two different bugs:

  1. When you change tabs, the selection is lost as soon as you return. I think leaving the tab resets the currentSelection to {}, and then on refocus a mutation happens that causes it read selection back from DOM. This one happens consistently.
  2. Within the same tab, as soon as you blur the view (either to another element, or even to the developer tools), it resets the currentSelection to {} and then at the same time a mutation is triggered that causes it to read selection back from DOM. This one is inconsistent, which makes me think it’s a race condition. If the blur registers before the mutation, then we lose the selection. But if the blur happens after, then we don’t.

By the way, we’ve isolated both problems to common libraries and browser extensions. #1 is caused by this polyfill, which adds and removes classes on the view on focus change GitHub - WICG/focus-visible at ea5722da398ae6cdc3bb23251486e07985d5e2e4

#2 is caused by a password manager which adds classes on iframes, which we use in one of our nodeviews. Adding an ignoreMutation to these nodeviews solves this second issue perfectly.

We still don’t know how to work around #1, because there’s no place to ignoreMutation at the level of the entire view.

1 Like

Can you boil this down to a simple script? I’ve tried giving an editor a node selection, blurring it, and then messing with the attributes on its outer element with the devtools, but that doesn’t seem to reset to a text selection.

I believe the original codesandbox @jamis0n posted shows this: ProseMirror Node View Example - CodeSandbox

Repro steps:

  1. Select the image
  2. Change tabs
  3. Go back to the tab

The node selection will be lost, because on focus an attribute has been added to the view root.

To clarify, I don’t think the issue happens after the blur, but instead just after refocusing (issue 1) or during the blur (issue 2).

Ah, sorry, I can indeed reproduce it with the sandbox. This patch narrows down the last-selection-clearing hack, so that it doesn’t run in the tab-changing scenario (which is atypical in that it moves focus without changing the DOM selection).

Thank you! Confirmed this works in both the codesandbox and our app.

For others encountering this thread, I’ve also had good results with ignoreMutation on one of our higher level nodeviews (corresponding to a section of a doc) using code like this:

ignoreMutation: ({ mutation }) => {
            const selection = this.editor.state.selection
            const element =
              mutation.target instanceof HTMLElement
                ? mutation.target
                : mutation.target.parentElement // This happens with text nodes

            if (element && !element.closest(CONTENT_SELECTOR)) {
              // Ignore any mutations that aren't on the contentDOM itself
              return true
            } else if (
              mutation.type === 'selection' &&
              selection instanceof NodeSelection &&
              isMediaNode(selection.node)
            ) {
              // Protect selection when the media drawer is open
              return true
            }
            return false
          },
        })

Apologies for reviving an old thread, but I’m seeing some odd behavior around this issue still.

Is it possible that the DOMObserver.prototype.flush method should be checking for focus where it checks for hasSelection? The editor view listens for focus events and resets the DOM selection, so if the editor doesn’t have focus I don’t know if flush should ever treat a DOM change as having a new selection.

What I’m seeing is that a blur that moves the focus to an element of a node view will cause the selection to be lost if the node view DOM mutates. This is slightly different from the original issue, where a focus event changes an attribute. In my case, supplemental UI changes an attribute while the tab still has focus.

I opened a PR to focus the discussion there: Only flush the selection when the editor view has focus by tilgovi · Pull Request #122 · ProseMirror/prosemirror-view · GitHub