ProseMirror / prosemirror

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

liftToOuterList (called by liftListItem) can improperly join listItems which should not be joined when called as an intermediary step in chained commands #1397

Closed Nantris closed 1 year ago

Nantris commented 1 year ago

The issue occurs because of this line: https://github.com/ProseMirror/prosemirror-schema-list/blob/19810fae27c75fc77fdb644be035646587c3e59d/src/schema-list.ts#L186

Unfortunately I cannot share the exact code which might reproduce this, and I apologize because I understand that without it this is not a very good bug report.

Additionally, please excuse the nonsense text and apparent inclusion of unnecessary text in this example JSON below. The issue only occurs in this extreme edge case. The affected lines are those with the text zz and xx. If I change the text of xx to xxx the issue no longer occurs, even though that text is actually outside of the affected range.

When I change xx to xxx the after value in the screenshot below goes from 61 (issue occurs) to 62 (issue doesn't occur)

image

I'm not at all clear on why this affects the join operation.


Below are the JSON output states of the editor before/after the problem liftListItem command. In every other case I've come across the problem has been my own code but it finally appears I've found a tiny bug in ProseMirror?

The intended outcome here is for zz and xx to appear in their own listItems (as they began) but instead they're merged into a single listItem despite not being within the range of any commands I run (although they are children of an affected listItem)

If I run identical steps without chain() this issue never occurs, but of course that's only feasible for testing and not production.

Pre-listListItem (but during chained commands) { "type": "doc", "content": [ { "type": "paragraph" }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "abc" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "def" } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "ghi" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "jkl" } ] } ] } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "><" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "pqrd" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph" } ] }, { "type": "listItem", "content": [ { "type": "paragraph" }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "zz" } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "xx" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": ">" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "asdfasd" } ] } ] } ] } ] } ] } ] } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "aasd" } ] } ] } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "asdas" } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "asd" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "asd" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "xz" } ] } ] } ] } ] } ] } ] } ] } ] } ] } ] } ] }, { "type": "paragraph" } ] }
After liftListItem { "type": "doc", "content": [ { "type": "paragraph" }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "abc" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "def" } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "ghi" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "jkl" } ] } ] } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "><" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "pqrd" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph" } ] }, { "type": "listItem", "content": [ { "type": "paragraph" }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "zz" } ] }, { "type": "paragraph", "content": [ { "type": "text", "text": "xx" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": ">" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "asdfasd" } ] } ] } ] } ] } ] } ] } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "aasd" } ] } ] } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "asdas" } ] } ] }, { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "asd" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "asd" } ] }, { "type": "bulletList", "content": [ { "type": "listItem", "content": [ { "type": "paragraph", "content": [ { "type": "text", "text": "xz" } ] } ] } ] } ] } ] } ] } ] } ] } ] } ] } ] }, { "type": "paragraph" } ] }
Diffed "text": "zz" } ] - } - ] - }, - { - "type": "listItem", - "content": [ + }, { "type": "paragraph", "content": [
marijnh commented 1 year ago

What is chain()? And could you set up an actual script that creates a schema, editor state, and run the command that produces the unexpected result?

Nantris commented 1 year ago

Sorry I forget that chain() is from TipTap. The equivalent here, as I understand it, would be chainCommands.

Unfortunately I haven't found a way to reproduce it without sharing proprietary code which I cannot share. Fortunately, at least it's a very difficult edge case to create in my experience.

marijnh commented 1 year ago

The equivalent here, as I understand it, would be chainCommands.

chainCommands should have absolutely no effect on how the individual commands operate, so it seems unlikely that an issue could only be reproduced when that is used.

Nantris commented 1 year ago

Looks like chain has nothing to do with chainCommands. I find the naming of chainCommands rather confusing. Sorry for the issue chaff @marijnh.

I'd appreciate if you could answer: I guess ProseMirror has no native concept of chaining commands together to accumulate the effects, and that's why these edge cases exist when chaining them together as TipTap allows?