ProseMirror / prosemirror

The ProseMirror WYSIWYM editor
http://prosemirror.net/
MIT License
7.61k stars 336 forks source link

Console error when trying to paste nested list content #1374

Closed sarahriouslydt closed 1 year ago

sarahriouslydt commented 1 year ago

Hello! We've observed an interesting bug between our editor schema and prosemirror-transform. To reproduce:

Create a list like so:

Cut the passage between the asterisks, giving us this clipboard data:

<meta charset='utf-8'><li data-pm-slice="3 3 [&quot;ul&quot;,null,&quot;li&quot;,null]"><p data-dovetail-attrs="{}">o</p></li><li><p data-dovetail-attrs="{}">three</p></li>

Pasting into the empty third list item, an error occurs in the console:

Uncaught TypeError: Cannot read properties of null (reading 'append')
    at closeFragment (index.js:1627:1)
    at closeFragment (index.js:1623:1)
    at closeFragment (index.js:1623:1)
    at replaceRange (index.js:1605:109)
    at Transaction.replaceRange (index.js:1863:1)
    at TextSelection.replace (index.js:90:1)
    at TextSelection.replace (index.js:252:1)
    at Transaction.replaceSelection (index.js:598:1)
    at doPaste (index.js:3367:1)
    at editHandlers.paste (index.js:3380:1)

Video demo:

https://user-images.githubusercontent.com/113878532/236139483-1b35fb96-c131-4ed8-8635-1dee9594fa45.mov


I believe this is potentially an edge case interaction between our list schema and the way prosemirror-transform tries to "close" a fragment.

Node spec snippets:

const ulNodeName = "ul";
const olNodeName = "ol";
const liNodeName = "li";
...
export const ul = {
  nodeName: ulNodeName,
  nodeSpec: {
    group: `${docContent} ${columnContent}`,
    content: `${liNodeName}+`,
    parseDOM: [{ tag: "ul" }],
    toDOM() {
      return ["ul", { class: "" }, 0];
    },
    [snipped]
  }
}
...
export const ol = {
  nodeName: olNodeName,
  nodeSpec: {
    group: `${docContent} ${columnContent}`,
    content: `${liNodeName}+`,
    parseDOM: [{ tag: "ol" }],
    toDOM() {
      return ["ol", { class: "" }, 0];
    },
    [snipped]
  },
}
...
export const li = {
  nodeName: liNodeName,
  nodeSpec: {
    content: `${Paragraph.nodeName} (${ulNodeName} | ${olNodeName})?`,
    parseDOM: [{ tag: "li" }],
    toDOM() {
      return ["li", 0];
    },
    defining: true,
  },
  [snipped]
}
...

I've attempted to debug what's going on and this is what I can surmise:

Any help diagnosing what's going wrong here would be greatly appreciated. At this point I'm not sure if it's a bug in prosemirror-transform or a strange nodeSpec that confuses the editor. Cheers!

marijnh commented 1 year ago

Setting up a schema like that and following the reproduction steps does not lead to an error for me. Please set up a minimal, self-contained script that allows me to see this happen.

sarahriouslydt commented 1 year ago

I was able to do some further digging and narrowed down the bug into our custom serializeFragment implementation.

serializeFragment(_: Fragment, options?: { [key: string]: string }) {
    const state = this.getEditorState();

    // Create a new transform doc
    const transform = new Transform(state.doc);
    // apply some marks to the transform in here

    return this.domSerializer.serializeFragment(
      transform.doc.slice(state.selection.from, state.selection.to).content,
      options,
    );
  }

The root issue is that because we want to apply some ephemeral marks, so that we can apply some special logic when we receive the pasted slice, we were constructing a new slice using transform.doc.slice. However we weren't passing in the extra includeParents boolean it supports.

As a result, the fragment prosemirror passes to this function call (that we don't end up using) might look like:

<ul>
  <li>
    <ul>
      <li></li>
      <li></li>
    </ul>
  </li>
</ul>

The fragment our implementation generates is more like:

<li></li>
<li></li>

This then appears to be the root issue: The editor can't handle the weird case where the fragment is malformed, and we try and paste a li into another li, which causes problems.


Adding in this boolean to the call of transform.doc.slice makes the error go away, but I'm wondering if there are any side effects I might want to be wary of from making this change?

Thanks for your help, this is a fantastic library.

marijnh commented 1 year ago

A serializeFragment that ignores the fragment it is passed and serializes some other fragment is just not something you can reasonably expect to work.