[Bug]: splitListItem fails on Android Chrome with paragraph attributes and NodeView

[TeemuKoivisto]

Affected Packages

core, react, bullet-list, list-item, paragraph

Version(s)

2.8.0

Bug Description

This is a rather horrendous bug I've uncovered involving NodeViews on Android Chrome. I think ProseMirror does not entirely handle Android Chrome behavior which, in conjunction with NodeViews (and attributes it seems), causes splitting list nodes to behave erratically.

There's a screen recording here: https://github.com/TeemuKoivisto/prosemirror-chrome-android-lists-bug

Browser Used

Chrome

Code Example URL

https://codesandbox.io/p/sandbox/runtime-microservice-g3p5ct

Expected Behavior

List item should be split without any extra nodes inserted.

Additional Context (Optional)

Gboard 14.6.03 arm64 Chrome 129.0.6668.81 Android 13

Dependency Updates

• [X] Yes, I've updated all my dependencies.

[Nantris]

You need to use this extension. I tested and it seems to resolves your issue. https://tiptap.dev/docs/editor/extensions/functionality/listkeymap

Demo: https://codesandbox.io/p/sandbox/cool-villani-32vzhk?file=%2Fsrc%2FApp.tsx

---

@nperez0111 maybe there's some way to start including the ListKeymap by default without breaking backwards compatibility by offering an opt-out somehow? I'm not sure how that could work, but these issues crop up fairly regularly and I'm sure each developer is spending a lot of time trying to debug this.

It also clutters up the tracker a tad.

Otherwise I'd suggest this be handled via a stark notice at the top of the docs for BulletList/OrderedList/TaskItem/ListItem docs pages - right at the top and unmissable - to make sure people know they need the ListKeymap.

[TeemuKoivisto]

@Nantris hey thanks! Did you try it with Android Chrome? As I tried that demo, it still produces the same duplicate list_items as in my screen recording.

I posted about it on ProseMirror forums since it seems there's an inconsistency with how paragraph NodeViews are managed in Android Chrome with lists https://discuss.prosemirror.net/t/selection-split-join-misbehavior-with-android-chrome-and-lists/7772/3

[Nantris]

Yes I tried on Android 14 with GBoard and wasn't able to produce the problem anymore with the link I provided (and I just re-tested to make sure I didn't make some mistake.) Can you share more details about your testing device?

[TeemuKoivisto]

Yes I tried on Android 14 with GBoard and wasn't able to produce the problem anymore with the link I provided (and I just re-tested to make sure I didn't make some mistake.) Can you share more details about your testing device?

I just upgraded to Android 14 and updated all my apps and it still fails. It seems it's the id attributes that cause it and after the initial duplicate list_item is created, it works ok. I added this IdPlugin to the sandbox which should make it occur more consistently.

This same bug has occurred with multiple Android users so there shouldn't be anything unique with my environment. Are you sure you are pressing enter at the end of item1?

Android 14, Nokia X20 Chrome 129.0.6668.100 Gboard 14.6.03.665297282-release-arm64-v8a

[Nantris]

Hmm yes I see the mistake in my approach now. Adding ListKeymap seems to have fixed the issue except if the caret is at the end of the listItem. In that case it still spazzes. I wonder why that is.

Confirming this issue.

[TeemuKoivisto]

👍 I have a hunch if you replace the Enter keymap with this command https://github.com/TeemuKoivisto/prosemirror-chrome-android-lists-bug/blob/main/packages/tiptap/src/splitListItemTiptap.ts and run it with debugger it will show something

[Nantris]

Is there some part in particular you changed?

I'm not a maintainer, but I'd be concerned about breaking existing projects/logic if the changes to that function are too extensive. I think the one TipTap uses now is essentially just a wrapped version of the ProseMirror splitListItem command?

[TeemuKoivisto]

Is there some part in particular you changed?

I'm not a maintainer, but I'd be concerned about breaking existing projects/logic if the changes to that function are too extensive. I think the one TipTap uses now is essentially just a wrapped version of the ProseMirror splitListItem command?

Did not change anything, merely extracted the thing.

But I figured out the bug. As ProseMirror, underneath TipTap, is just a wrapper to standardize the contentEditable events, it really matters to what element the contentEditable action is being triggered to. In this example the actual element is not <p> but <div> which is the standard element TipTap renders content to. This works well in most cases as div is interchangeable with almost anything EXCEPT HERE.

I found out that you can pass contentDOMElementTag: 'p' option to the NodeViewRenderer which allows the nodeview to follow the original DOM structure. Which in turn creates the correct contentEditable event, whatever is, in Android Chrome:

https://codesandbox.io/p/sandbox/epic-cookies-996855

That works. Now while this is not a bug per se, it is rather stupid that TipTap has this as="p" prop for NodeView content which is purely for aesthetical reasons, as you should always use contentDOMElementTag for ProseMirror to render the correct element for contentEditable to interact with.

[nperez0111]

it is rather stupid that TipTap has this as="p" prop for NodeView content which is purely for aesthetical reasons, as you should always use contentDOMElementTag for ProseMirror to render the correct element for contentEditable to interact with.

First, tone. Second, they control different elements.

I used as="span" & contentDomElementTag="p" to show they are controlling different things (which has nothing to do with aesthetic reasons).

Strange situation over all. Though, maybe we can change the default element here in Tiptap v3 (doing so now would be a breaking change)

[TeemuKoivisto]

it is rather stupid that TipTap has this as="p" prop for NodeView content which is purely for aesthetical reasons, as you should always use contentDOMElementTag for ProseMirror to render the correct element for contentEditable to interact with.

First, tone. Second, they control different elements.

I used as="span" & contentDomElementTag="p" to show they are controlling different things (which has nothing to do with aesthetic reasons).

Strange situation over all. Though, maybe we can change the default element here in Tiptap v3 (doing so now would be a breaking change)

But that <span> has no functionality beyond being just span. You can use it in CSS selector, you can have it inherit default CSS styles. But it does not change any logic whatsoever in editing, just wraps the contentDOM. Whereas changing the contentDOM's tag, as here, is actually critical and causes bugs if left to default.

[nperez0111]

But that <span> has no functionality beyond being just span. You can use it in CSS selector, you can have it inherit default CSS styles. But it does not change any logic whatsoever in editing, just wraps the contentDOM. Whereas changing the contentDOM's tag, as here, is actually critical and causes bugs if left to default.

The span exists because Prosemirror needs the contentDomElement synchronously, & React cannot render components synchronously (it does not do effects or refs even within a flushSync). So, what NodeViewContent does is attach the contentDomElement that was created synchronously to an element within React's render tree (by default a div but, that can be controlled with the as prop).

This, of course is out of our control on both accounts, React should be able to render synchronously and Prosemirror should allow nodeviews which are callbacks.

It is not so much stupid, as a limitation of the technologies paired, Vue does not have this issue & needs no wrapper element.

Critical is debatable, this library has millions of downloads and this issue must have been around for years, and you are the first to report it... I'm willing to implement it for the next major version that we are working on: https://github.com/ueberdosis/tiptap/discussions/5793 but, this will definitely break more people than it ultimately seeks to help.

[TeemuKoivisto]

But that <span> has no functionality beyond being just span. You can use it in CSS selector, you can have it inherit default CSS styles. But it does not change any logic whatsoever in editing, just wraps the contentDOM. Whereas changing the contentDOM's tag, as here, is actually critical and causes bugs if left to default.

The span exists because Prosemirror needs the contentDomElement synchronously, & React cannot render components synchronously (it does not do effects or refs even within a flushSync). So, what NodeViewContent does is attach the contentDomElement that was created synchronously to an element within React's render tree (by default a div but, that can be controlled with the as prop).

This, of course is out of our control on both accounts, React should be able to render synchronously and Prosemirror should allow nodeviews which are callbacks.

It is not so much stupid, as a limitation of the technologies paired, Vue does not have this issue & needs no wrapper element.

Critical is debatable, this library has millions of downloads and this issue must have been around for years, and you are the first to report it... I'm willing to implement it for the next major version that we are working on: #5793 but, this will definitely break more people than it ultimately seeks to help.

Sure but I was merely pointing out the fact that its tag (span, div, etc) doesn't matter and so changing it with as="x" is kinda pointless. Perhaps there's a very narrow edgecase, say figcaption, where nesting the contentDOM correctly is needed although I don't think anything would break regardless.

Well to me it doesn't matter now since I figured it out. But the way TipTap renders the contentDOM is highly debatable since bugs like these can now leak through. You should be using const { dom, contentDOM } = DOMSerializer.renderSpec(document, toDOM(this.node)) as the default since it's how ProseMirror does it without nodeviews. Not sure whether that would break anything, just perhaps CSS selectors. Well if you were using "as" already that would probably cause errors, with doubly nested eg paragraphs.

And just because I'm the only one reporting it, doesn't mean it's not affecting people. Especially since this just breaks things so deviously it's hard to even notice.