Contrib.social

[upstream] CKEditor 5 mangles table structure

Open on Drupal.org →
Created on 7 February 2023, over 1 year ago
Updated 1 February 2024, 8 months ago

Problem/Motivation

This table structure cannot be saved when using CKEditor 5:

<table>
  <tr>
    <th>Header</th>
    <th>Header</th>
    <th>Header</th>
  </tr>
  <tr>
    <th>Header</th>
    <td>Data</td>
    <td>Data</td>
  </tr>
</table>

Steps to reproduce

Paste code above into the editor source view, then either save or toggle back out of source view. All of the <td> tags will be converted to <th> tags.

There seems to be an upstream bug here: https://github.com/ckeditor/ckeditor5/issues/3172

However this happens regardless of any tbody structure or not.

Working examples

headings in <thead>, first column headings inside <tbody>:

<table>
  <thead>
    <tr>
      <th>Heading</th>
      <th>Heading</th>
      <th>Heading</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <th>Heading</th>
      <td>Data</td>
      <td>Data</td>
    </tr>
    <tr>
      <th>Heading</th>
      <td>Data</td>
      <td>Data</td>
    </tr>
  </tbody>
</table>

colspan heading in <thead>, first column headings inside <tbody>:

<table>
  <thead>
  <tr>
    <th colspan="2">Heading</th>
    <th>Heading</th>
  </tr>
  </thead>
  <tbody>
  <tr>
    <th>Heading</th>
    <td>Data</td>
    <td>Data</td>
  </tr>
  <tr>
    <th>Heading</th>
    <td>Data</td>
    <td>Data</td>
  </tr>
  </tbody>
</table>

Breaking examples

(paste these into source, toggle out of source or save the content and all <td> will be converted to <th>)

<table>
  <thead>
  <tr>
    <th>Heading</th>
    <th>Heading</th>
    <th>Heading</th>
  </tr>
  </thead>
  <tbody>
  <tr>
    <th colspan="3">Heading</th>
  </tr>
  <tr>
    <td>Data</td>
    <td>Data</td>
    <td>Data</td>
  </tr>
  </tbody>
</table>

<table>
  <tbody>
  <tr>
    <th colspan="3">Heading</th>
  </tr>
  <tr>
    <td>Data</td>
    <td>Data</td>
    <td>Data</td>
  </tr>
  </tbody>
</table>

This one flips the <th> to a <td>

<table>
  <tbody>
  <tr>
    <td>Data</td>
    <td>Data</td>
    <th>Heading</th>
  </tr>
  </tbody>
</table>

After trying to narrow all of these down, it seems like the behavior is "a <th> may only exist as a non-colspan first column inside a <tbody>"—any other structure results in tag data loss.

Edit: It's also not allowing <td> inside a <thead> at all, and that's also valid HTML.

🐛 Bug report
Status

Fixed

Version

10.1 ✨

Component
CKEditor 5  →

Last updated about 6 hours ago

Created by

🇺🇸United States karlshea Minneapolis 🇺🇸

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @karlshea
  • Status changed to Postponed over 1 year ago
  • Comment over 1 year ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    This is a borderline data loss case. 😬

    Thank you for finding the upstream bug report and for reporting your findings there too! 👍 I have a meeting with the CKEditor 5 team tomorrow and will raise this there.

  • Comment over 1 year ago →
    🇺🇸United States karlshea Minneapolis 🇺🇸
  • Comment over 1 year ago →
    🇺🇸United States karlshea Minneapolis 🇺🇸
  • Comment over 1 year ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • Comment over 1 year ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Discussed this with @Reinmar from CKEditor 5!

    This bug has also been frequently reported by other CKEditor 5 users; it's already scheduled to be fixed in the coming months! 👍

  • Comment over 1 year ago →
    🇵🇱Poland Reinmar Warsaw, Poland
  • Comment over 1 year ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Good news from @Reinmar!

    https://github.com/ckeditor/ckeditor5/issues/3172 landed, which superseded https://github.com/ckeditor/ckeditor5/issues/11536 🚀

    That means the next CKEditor 5 update will fix this 😊

  • Comment over 1 year ago →
    🇺🇸United States karlshea Minneapolis 🇺🇸

    I don't believe that PR will fix most of these issues, and I think the reason for that is shown in the test from that PR that shows their expected data model:

    '<table headingRows="1">' +
    '<tableRow>' +
        '<tableCell><paragraph>Date</paragraph></tableCell>' +
        '<tableCell><paragraph>Event</paragraph></tableCell>' +
        '<tableCell><paragraph>Venue</paragraph></tableCell>' +
    '</tableRow>' +

    They don't seem to be able to represent an arbitrary table structure within the editor, only rows and cells with the number of headings as attributes on the table container. So most of my examples above cannot be input into Drupal without using an input format that doesn't use CKEditor at all.

    Issue 13609 might be a better one to track for this Drupal issue, but until they change their data model to match what you can do with a real table CKEditor can't be used on input formats where your editors may need to add complicated tables.

  • Comment over 1 year ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #9: are you saying that the examples in the issue summary will not work? Could you test the latest upstream version with all the test cases you have in mind? 🙏

  • Comment over 1 year ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Just discussed with @Reinmar & Witek. Witek says 2 of the 3 problems surfaced in the issue summary are 100% fixed, and the third is no longer a data loss bug but just a semantics loss bug: a <th> may get converted to a <td>, which is far less severe.

    (Or at least that's what I think I understood, my not having a working dev environment right now made this more complicated to follow 😅)

    So actually this issue is currently critical data loss (clarifying in metadata), but after the next release should become "major data loss" (because semantics loss).

    I'd still like @KarlShea to confirm though 😊 🙏

  • Status changed to Fixed over 1 year ago
  • Comment over 1 year ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    See @smustgrave's report at #3355358-8: Update CKEditor 5 to 37.1.0 → . He was able to confirm what I wrote in #11, that there definitely is no longer a data loss bug. 🚀

    I'm going to mark this issue as fixed. @KarlShea, if you have additional edge cases relating to table handling in CKEditor 5, could you please create a new issue? 🙏

  • Comment over 1 year ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • Comment over 1 year ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • Comment over 1 year ago →
    System Message

    Automatically closed - issue fixed for 2 weeks with no activity.

  • Status changed to Fixed 8 months ago
  • Comment 8 months ago →
    🇺🇸United States j. ayen green

    Was a separate issue forked from this for the semantics loss? I understand that semantics is less critical than data, but try to tell editors that their tables getting munged because CKEditor cannot handle th vs td.

  • Comment 8 months ago →
    🇺🇸United States karlshea Minneapolis 🇺🇸

    No, it's been brought up multiple times on both Slack and in the CKEditor issue queue and no one seems very concerned.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024