Fix: Adding empty td tag inside table throws RangeError #6241

Meyazhagan · 2025-04-06T12:55:16Z

Changes Overview

Closes: #6237

By default we using DOMParser.parseSlice (this function is not parsing the full node.), and no option to disable it
so, I created an option useParseSlice,

https://discuss.prosemirror.net/t/domparser-parseslice-not-parsing-correctly-empty-table-cell/5510

Implementation Approach

Testing Done

Verification Steps

Additional Notes

Checklist

I have created a changeset for this PR if necessary.
My changes do not break the library.
I have added tests where applicable.
I have followed the project guidelines.
I have fixed any lint issues.

Related Issues

changeset-bot · 2025-04-06T12:55:20Z

🦋 Changeset detected

Latest commit: 6621e38

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 54 packages

Name	Type
@tiptap/core	Major
@tiptap/extension-blockquote	Major
@tiptap/extension-bold	Major
@tiptap/extension-bubble-menu	Major
@tiptap/extension-bullet-list	Major
@tiptap/extension-character-count	Major
@tiptap/extension-code-block-lowlight	Major
@tiptap/extension-code-block	Major
@tiptap/extension-code	Major
@tiptap/extension-collaboration-cursor	Major
@tiptap/extension-collaboration	Major
@tiptap/extension-color	Major
@tiptap/extension-document	Major
@tiptap/extension-dropcursor	Major
@tiptap/extension-floating-menu	Major
@tiptap/extension-focus	Major
@tiptap/extension-font-family	Major
@tiptap/extension-gapcursor	Major
@tiptap/extension-hard-break	Major
@tiptap/extension-heading	Major
@tiptap/extension-highlight	Major
@tiptap/extension-history	Major
@tiptap/extension-horizontal-rule	Major
@tiptap/extension-image	Major
@tiptap/extension-italic	Major
@tiptap/extension-link	Major
@tiptap/extension-list-item	Major
@tiptap/extension-list-keymap	Major
@tiptap/extension-mention	Major
@tiptap/extension-ordered-list	Major
@tiptap/extension-paragraph	Major
@tiptap/extension-placeholder	Major
@tiptap/extension-strike	Major
@tiptap/extension-subscript	Major
@tiptap/extension-superscript	Major
@tiptap/extension-table-cell	Major
@tiptap/extension-table-header	Major
@tiptap/extension-table-row	Major
@tiptap/extension-table	Major
@tiptap/extension-task-item	Major
@tiptap/extension-task-list	Major
@tiptap/extension-text-align	Major
@tiptap/extension-text-style	Major
@tiptap/extension-text	Major
@tiptap/extension-typography	Major
@tiptap/extension-underline	Major
@tiptap/extension-youtube	Major
@tiptap/html	Major
@tiptap/react	Major
@tiptap/starter-kit	Major
@tiptap/suggestion	Major
@tiptap/vue-2	Major
@tiptap/vue-3	Major
@tiptap/pm	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify · 2025-04-06T12:55:37Z

✅ Deploy Preview for tiptap-embed ready!

Name	Link
🔨 Latest commit	`6621e38`
🔍 Latest deploy log	https://app.netlify.com/sites/tiptap-embed/deploys/680a40ae4db9cb00074f1851
😎 Deploy Preview	https://deploy-preview-6241--tiptap-embed.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

its-me-dhina · 2025-04-08T10:01:02Z

Verified the changes on local and the fix seems to be working fine. Can we prioitize merging this request?

Thanks!

timothyerwin · 2025-04-09T22:12:53Z

@Meyazhagan thanks, I'm seeing the same issue

arnaugomez · 2025-04-10T11:38:20Z

packages/core/src/helpers/createNodeFromContent.ts

@@ -10,7 +10,7 @@ import { Content } from '../types.js'
 import { elementFromString } from '../utilities/elementFromString.js'

 export type CreateNodeFromContentOptions = {
-  slice?: boolean


Isn't it already possible to disable DOMParser.parseSlice by calling createNodeFromContent with the parameter slice: false? @Meyazhagan . I believe your PR only changes the name of the parameter from slice to useParseSlice. It also changes its default value from true to false, which is a breaking change.

Therefore, I believe you could achieve the desired behavior of disabling DOMParser.parseSlice with the current code, by calling createNodeFromContent with the slice parameter to false. Could you check that this is true?

If it is already possible to disable DOMParser.parseSlice by calling createNodeFromContent with the slice parameter set to false, then this PR is likely not necessary. Or is there anything I'm missing? Correct me if I'm wrong.

@arnaugomez is it possible to disable it while inserting the content into the editor?

Ref: #6237

I think it's possible to achieve the same behavior of disabling DOMParser.parseSlice by calling createNodeFromContent with the slice parameter set to false. Have you tried it? @hafeezulkareem-yellow @Meyazhagan

@arnaugomez yeah I tried this changes, it is working!

quick question: createNodeFromContent function is being used in two places, createDocument and in insertContentAt both the place also we setting slice to false to use parse function, instead we can change in createNodeFromContent, by default to using parse function!

is there any good reason to have parseSlice as default parser ?

@Meyazhagan Yes, sure, if you think we can refactor it so that the code is simpler, create another PR with your changes and we'll review it. Thanks for your suggestion!

arnaugomez

Add a changeset

arnaugomez · 2025-04-13T09:19:46Z

@bdbch Could you review this? To me it looks good but I'm not familiar with this code and I want to make sure it does not break things.

bdbch

I don't like that we hard-code slice: false into the insertContentAt function as it can potentially break the behavior for existing implementations.

Could we please have that as an option for insertContent and insertContentAt if this needs to be changed depending on what kind of content you want to insert and how you want to do it?

bdbch · 2025-04-17T17:28:58Z

packages/core/src/commands/insertContentAt.ts

@@ -74,6 +74,7 @@ export const insertContentAt: RawCommands['insertContentAt'] = (position, value,

    try {
      content = createNodeFromContent(value, editor.schema, {
+        slice: false,


Should this be hard-coded in the insertContentAt function?

Maybe we could expose this as an option rather than enforcing this.

Also make sure to pass through this new option in insertContent

bdbch · 2025-04-17T17:30:22Z

.changeset/warm-yaks-speak.md

+"@tiptap/core": minor
+---
+
+fix parsing of empty html table in insertContentAt func


Please update the changeset accordingly.

hafeezulkareem-yellow · 2025-04-24T09:51:54Z

Hi @Meyazhagan @bdbch, I appreciate the work you are doing. It would be extremely helpful if we fast-track a fix for it. Thanks again for your time and support!

arnaugomez

LGTM

arnaugomez · 2025-04-24T15:01:40Z

packages/core/src/commands/insertContentAt.ts

      ...options,
    }

    let content: Fragment | ProseMirrorNode

    try {
      content = createNodeFromContent(value, editor.schema, {
+        slice: options.slice,


I am concerned this is a breaking change. Before these changes, the createNodeFromContent was called with the slice value being false. But now it is called with a default value of true. If the default value of options.slice was false then this would not be a breaking change. But if the value should be true by default, then you have my approval. Just wanted to point this out.

I would also rather keep the previous default to not cause breaking changes.

Meyazhagan changed the title ~~Fix():~~ Fix: Adding empty td tag inside table throws RangeError Apr 6, 2025

Meyazhagan marked this pull request as ready for review April 8, 2025 16:04

arnaugomez requested a review from bdbch April 10, 2025 11:28

arnaugomez requested changes Apr 10, 2025

View reviewed changes

arnaugomez removed the request for review from bdbch April 10, 2025 11:42

insertContentAt: set slice to false

57d1b89

Meyazhagan force-pushed the empty-table-6237 branch from 3265f9d to 57d1b89 Compare April 13, 2025 04:05

Meyazhagan requested a review from arnaugomez April 13, 2025 04:06

arnaugomez requested changes Apr 13, 2025

View reviewed changes

arnaugomez requested a review from bdbch April 13, 2025 09:16

add changeset

83f7c4e

Meyazhagan requested a review from arnaugomez April 13, 2025 12:06

bdbch requested changes Apr 17, 2025

View reviewed changes

add slice option in insertContent, insertContentAt

63bd9d3

Meyazhagan added 2 commits April 24, 2025 19:14

add test case

66053b0

updated changeset

6621e38

Meyazhagan requested a review from bdbch April 24, 2025 13:47

arnaugomez approved these changes Apr 24, 2025

View reviewed changes

Uh oh!

Fix: Adding empty td tag inside table throws RangeError #6241

Are you sure you want to change the base?

Fix: Adding empty td tag inside table throws RangeError #6241

Conversation

Meyazhagan commented Apr 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes Overview

Implementation Approach

Testing Done

Verification Steps

Additional Notes

Checklist

Related Issues

Uh oh!

changeset-bot bot commented Apr 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

netlify bot commented Apr 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for tiptap-embed ready!

Uh oh!

its-me-dhina commented Apr 8, 2025

Uh oh!

timothyerwin commented Apr 9, 2025

Uh oh!

arnaugomez Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arnaugomez Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arnaugomez left a comment

Choose a reason for hiding this comment

Uh oh!

arnaugomez commented Apr 13, 2025

Uh oh!

bdbch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hafeezulkareem-yellow commented Apr 24, 2025

Uh oh!

arnaugomez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Meyazhagan commented Apr 6, 2025 •

edited

Loading

changeset-bot bot commented Apr 6, 2025 •

edited

Loading

netlify bot commented Apr 6, 2025 •

edited

Loading

arnaugomez Apr 10, 2025 •

edited

Loading

arnaugomez Apr 11, 2025 •

edited

Loading