Looks-Like-Enter heuristic in `readDOMChange` produces wrong state

[salmenf]

Hello! I encountered a bug in the DOM observer logic. The setup: I have a custom element webwriter-choice (becoming a node webwriter_choice) and an element webwriter-choice-item (webwriter_choice_item). Then, I have this logic on webwriter-choice:

    const choiceItem = this.ownerDocument.createElement("webwriter-choice-item")
    const p = this.ownerDocument.createElement("p")
    choiceItem.appendChild(p)
    this.appendChild(choiceItem)
    this.ownerDocument.getSelection().setBaseAndExtent(p, 0, p, 0)

This works fine outside of ProseMirror. Inside a ProseMirror view, it breaks exactly when the selection is at the end of the previous item (see GIFs below).

I am sure the heuristic added in #175 is the problem. I specifically found that if I remove the heuristic, the issue doesn't occur at all. EDIT: Not true, see below.

With the heuristic

  if (((browser.ios && view.input.lastIOSEnter > Date.now() - 225 &&
        (!inlineChange || addedNodes.some(n => n.nodeName == "DIV" || n.nodeName == "P"))) ||
       (!inlineChange && $from.pos < parse.doc.content.size && !$from.sameParent($to) &&
        (nextSel = Selection.findFrom(parse.doc.resolve($from.pos + 1), 1, true)) &&
        nextSel.head == $to.pos)) &&
      view.someProp("handleKeyDown", f => f(view, keyEvent(13, "Enter")))) {
    view.input.lastIOSEnter = 0
    return
  }

Without the heuristic

  if ((browser.ios && view.input.lastIOSEnter > Date.now() - 225 &&
        (!inlineChange || addedNodes.some(n => n.nodeName == "DIV" || n.nodeName == "P"))) &&
      view.someProp("handleKeyDown", f => f(view, keyEvent(13, "Enter")))) {
    view.input.lastIOSEnter = 0
    return
  }

Possible solutions

The heuristic is from 2016 - is it still needed 8 years later? Otherwise, it might be worth improving since this is close to impossible to work around in user land (can't disable it, can't influence the DOM reading process).

Platform info

• "prosemirror-view": "1.33.8"
• OS: Windows 10.0.19045 X64
• WebView2: 126.0.2592.113 (basically Chromium, running in Tauri)

[marijnh]

Is this some kind of widget you're displaying as part of a node view? If so, wouldn't just filtering out the DOM changes with ignoreMutation be what you want to do here?

And yes, the heuristic is still very much necessary.

[salmenf]

Thanks for the quick reply! You're right, I did further testing and noticed removing the heuristic introduces many other issues, anyway - not a solution.

The issue is that I am dealing with nested custom elements via slots. Custom elements may contain other custom elements that way, so in turn, ProseMirror nodes are nested as well. Ignoring the changes was the first thing I tried, but this just leads to faulty editor states. Next I tried calculating the new node from the changed DOM, but that is just reinventing ProseMirror's wheel.

For example, this may be a subtree:

<webwriter-choice class="ww-widget ww-v0.0.1" contenteditable="" mode="single">
  <p slot="prompt">What came first?</p>
  <webwriter-choice-item class="ww-widget ww-v0.0.1" contenteditable=""><p>Chicken</p></webwriter-choice-item>
  <webwriter-choice-item class="ww-widget ww-v0.0.1" contenteditable=""><p>Egg</p></webwriter-choice-item>
</webwriter-choice>

Using the normal DOM methods, <webwriter-choice> may manipulate its own children, for example.

[marijnh]

Arbitrary DOM manipulation within its content is not really a usage model ProseMirror supports, I'm afraid.

[salmenf]

A shame! I understand, though, it introduces a lot of cases to consider. I'll fork and experiment more to see if I can make it work for myself. Thanks anyway. :)

[salmenf]

If someone else encounters this, I worked around the issue. I wouldn't be surprised if this introduces more issues, but it works for my case so far.

Simply:

1. Add a NodeView for the element.
2. Implement ignoreMutation so that for mutations of type childList, the whole element is read instead of the default behavior of just the changed section (I assume).

ignoreMutation(mutation: MutationRecord) {
  ...
  if(type === "childList") {
    // re-parse the whole node, readDOMChange is from prosemirror-view/src/domchange.ts
    readDOMChange(this.view, this.getPos(), this.getPos() + this.node.nodeSize, true, Array.from(mutation.addedNodes))
    // Prevent the default handling
    return true
  }
  ...
}

@marijnh It would only be handy if readDOMChange was also exported by prosemirror-view in some way. This could give users more control in customizing the DOM parsing process in a NodeView. Or in a more limited form, one could return the start and end pos to re-parse from ignoreMutation, e.g.:

ignoreMutation(mutation: MutationRecord) {
  ...
  if(type === "childList") {
    return {start: this.getPos(), end: this.getPos() + this.node.nodeSize}
  }
  ...
}