fix: inserting a table from external resources lead Lexical crash #5871

IAmWangyixin · 2024-04-11T03:29:20Z

Fix #5812
Before:
see #5812

After:

reason for crash: When press Enter the program gets stuck in a dead loop while checking for INTERNAL_$isBlock(node).

I haven't thought of a suitable way to fix the problem in lexical or lexical/table, but I think I can avoid the Lexical crash by wrapping Paragraph around elements other than paragraphs, lists, code blocks, Quote in the playground.

Please let me know if there have a better solution.

vercel · 2024-04-11T03:29:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 12, 2024 2:58am
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 12, 2024 2:58am

zurfyx

but I think I can avoid the Lexical crash by wrapping Paragraph around elements other than paragraphs

This is correct, text should never be a top-level element, which does apply in this case because TableCell is a ShadowRoot. However, instead of adding a custom table cell, which will be hard to replicate by others, I would explore why this happens and whether insertNodes can do this for us as it doesn't happen for the RootNode.

potatowagon · 2024-04-11T15:47:44Z

packages/lexical-playground/src/nodes/TableCellNode.tsx

+const CustomTableCell = 'customTableCell';
+const PIXEL_VALUE_REG_EXP = /^(\d+(?:\.\d+)?)px$/;
+
+export class CustomTableCellNode extends TableCellNode {


should this file be named CustomTableCellNode instead of TableCellNode?

potatowagon · 2024-04-11T15:58:20Z

i wonder what html is being copied onto the clipboard. i would be curious to know what (or if theres any) dom element wrapping the text node that results in it being a top level element to table cell.

do u mind sharing what html is being copied to clipboard? u may inspect that by downloading pasteboard: https://github.com/sindresorhus/Pasteboard-Viewer

(i learnt this from @zurfyx , thanks for sharing this useful tool!)

lastly good job debugging and finding the root cause!

IAmWangyixin · 2024-04-12T01:51:52Z

However, instead of adding a custom table cell, which will be hard to replicate by others, I would explore why this happens and whether insertNodes can do this for us as it doesn't happen for the RootNode.

Thanks for the review and for suggesting a better solution. I'll give it a try.

IAmWangyixin · 2024-04-12T01:55:23Z

do u mind sharing what html is being copied to clipboard

@potatowagon of course here the html

<table class="PlaygroundEditorTheme__table">
  <colgroup>
    <col>
    <col>
    <col>
    <col>
  </colgroup>
  <tbody>
    <tr>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">text</span>
      </td>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">something wrong when print enter.</span>
      </td>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">text</span>
      </td>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">5.5,5.6</span>
      </td>
    </tr>
  </tbody>
</table>

potatowagon · 2024-04-12T02:54:32Z

do u mind sharing what html is being copied to clipboard

@potatowagon of course here the html

<table class="PlaygroundEditorTheme__table">
  <colgroup>
    <col>
    <col>
    <col>
    <col>
  </colgroup>
  <tbody>
    <tr>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">text</span>
      </td>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">something wrong when print enter.</span>
      </td>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">text</span>
      </td>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">5.5,5.6</span>
      </td>
    </tr>
  </tbody>
</table>

#5857 may fix this as tgat PR would wrap inline text nodes in a paragraph if theres nothing between the inline node and the root (or shadow root in this case for td

will be testing your case on that pr

potatowagon · 2024-04-12T02:56:39Z

do u mind sharing what html is being copied to clipboard

@potatowagon of course here the html

<table class="PlaygroundEditorTheme__table">
  <colgroup>
    <col>
    <col>
    <col>
    <col>
  </colgroup>
  <tbody>
    <tr>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">text</span>
      </td>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">something wrong when print enter.</span>
      </td>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">text</span>
      </td>
      <td class="PlaygroundEditorTheme__tableCell" style="width: 175px; border: 1px solid black; vertical-align: top; text-align: start;">
        <span style="white-space: pre-wrap;">5.5,5.6</span>
      </td>
    </tr>
  </tbody>
</table>

just realised this is the copied html of the table in lexical editor, could u share instead the copied html after copy from the external app?

IAmWangyixin · 2024-04-12T03:42:16Z

just realised this is the copied html of the table in lexical editor, could u share instead the copied html after copy from the external app?

You're right, what I got before was the html after saving on lexical, I think the following is the original pasted html.

<table style="width: auto; text-align: start">
  <tbody>
    <tr>
      <td
        colspan="1"
        rowspan="1"
        width="auto"
        style="text-align: start; line-height: 1.5">
        text
      </td>
      <td
        colspan="1"
        rowspan="1"
        width="auto"
        style="text-align: start; line-height: 1.5">
        something wrong when print enter.
      </td>
      <td
        colspan="1"
        rowspan="1"
        width="auto"
        style="text-align: start; line-height: 1.5">
        text
      </td>
      <td
        colspan="1"
        rowspan="1"
        width="auto"
        style="text-align: start; line-height: 1.5">
        5.5,5.6
      </td>
    </tr>
  </tbody>
</table>

ivailop7 · 2024-05-21T22:08:16Z

I wonder if the crash is fixed by #6139 since the example here all have TBODY tag, which we didn't respect in the TableObserver. Thoughts @potatowagon @zurfyx ?

potatowagon · 2024-05-28T01:47:18Z

I wonder if the crash is fixed by #6139 since the example here all have TBODY tag, which we didn't respect in the TableObserver. Thoughts @potatowagon @zurfyx ?

afaik this issue is solved already

what could have fixed the crash was text nodes inside of a <td> now being wraped in a <p> on pasting, which wont lead to crash

potatowagon · 2024-05-28T01:48:35Z

closing since this issue is solved

IAmWangyixin requested review from zurfyx, fantactuka, acywatson and Fetz as code owners April 11, 2024 03:29

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 11, 2024

vercel bot deployed to Preview – lexical April 11, 2024 03:32 View deployment

vercel bot deployed to Preview – lexical-playground April 11, 2024 03:33 View deployment

IAmWangyixin force-pushed the fix/5812 branch from 4b04b10 to 94835dd Compare April 11, 2024 03:55

vercel bot deployed to Preview – lexical April 11, 2024 03:56 View deployment

vercel bot deployed to Preview – lexical-playground April 11, 2024 03:59 View deployment

zurfyx reviewed Apr 11, 2024

View reviewed changes

potatowagon reviewed Apr 11, 2024

View reviewed changes

vercel bot deployed to Preview – lexical April 12, 2024 02:08 View deployment

vercel bot deployed to Preview – lexical-playground April 12, 2024 02:08 View deployment

IAmWangyixin added 2 commits April 12, 2024 10:54

fix: inserting a table from external resources lead Lexical crash

8a266c7

fix: rename tableCellNode to CustomTableCellNode

3e8d7f7

IAmWangyixin force-pushed the fix/5812 branch from 23ca78a to 3e8d7f7 Compare April 12, 2024 02:55

vercel bot deployed to Preview – lexical April 12, 2024 02:57 View deployment

vercel bot deployed to Preview – lexical-playground April 12, 2024 02:58 View deployment

Sahejkm added a commit that referenced this pull request Apr 12, 2024

Fix #5871 copy paste cell crashes if it has no paragraph

6dba36d

Sahejkm mentioned this pull request Apr 12, 2024

Fix #5812 copy paste cell crashes if it has no paragraph #5879

Closed

IAmWangyixin marked this pull request as draft April 12, 2024 07:25

potatowagon closed this May 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: inserting a table from external resources lead Lexical crash #5871

fix: inserting a table from external resources lead Lexical crash #5871

IAmWangyixin commented Apr 11, 2024 •

edited

Loading

vercel bot commented Apr 11, 2024 •

edited

Loading

zurfyx left a comment

potatowagon Apr 11, 2024

potatowagon commented Apr 11, 2024

IAmWangyixin commented Apr 12, 2024

IAmWangyixin commented Apr 12, 2024

potatowagon commented Apr 12, 2024

potatowagon commented Apr 12, 2024

IAmWangyixin commented Apr 12, 2024

ivailop7 commented May 21, 2024

potatowagon commented May 28, 2024 •

edited

Loading

potatowagon commented May 28, 2024

fix: inserting a table from external resources lead Lexical crash #5871

fix: inserting a table from external resources lead Lexical crash #5871

Conversation

IAmWangyixin commented Apr 11, 2024 • edited Loading

vercel bot commented Apr 11, 2024 • edited Loading

zurfyx left a comment

Choose a reason for hiding this comment

potatowagon Apr 11, 2024

Choose a reason for hiding this comment

potatowagon commented Apr 11, 2024

IAmWangyixin commented Apr 12, 2024

IAmWangyixin commented Apr 12, 2024

potatowagon commented Apr 12, 2024

potatowagon commented Apr 12, 2024

IAmWangyixin commented Apr 12, 2024

ivailop7 commented May 21, 2024

potatowagon commented May 28, 2024 • edited Loading

potatowagon commented May 28, 2024

IAmWangyixin commented Apr 11, 2024 •

edited

Loading

vercel bot commented Apr 11, 2024 •

edited

Loading

potatowagon commented May 28, 2024 •

edited

Loading