search

ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.49k stars 3.7k forks source link

Todolist lost checked property when reload or set to Source Editing #15602

Closed woaiiimipi closed 2 months ago

woaiiimipi commented 9 months ago

📝 Provide detailed reproduction steps (if any)

When I add todolist to a table cell, I checked the checkbox, and saved the data to server, when I reload the data, the checkbox's checked status is lost.

Before:

image

Reload:

image

Below is my source code, you can copy the code to source editing, the checked="checked" status will be lost

<html><head></head><body>
<figure class="table">
    <table>
        <tbody>
            <tr>
                <td style="background-color:hsl(60, 75%, 60%);text-align:center;">
                    <p style="color:#7F6000;font-family:Calibri;font-size:12.0pt;margin:0in;">
                        <span style="color:hsl(0,0%,0%);"><strong>FOUNDATION</strong></span>
                    </p>
                </td>
            </tr>
            <tr>
                <td style="vertical-align:top;">
                    <ul class="todo-list">
                        <li>
                            <label class="todo-list__label todo-list__label_without-description"><input type="checkbox" checked="checked" disabled="disabled"></label>
                            <p style="font-family:Calibri;font-size:12.0pt;margin:0in;">
                                <strong>Net Worth:&nbsp;</strong>
                            </p>
                        </li>
                    </ul>
                </td>
            </tr>
        </tbody>
    </table>
</figure>
</body></html>

✔️ Expected result

Checked status can reload correctly

❌ Actual result

Checked status is lost.

❓ Possible solution

📃 Other details

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

niegowski commented 9 months ago

Simplest reproduction (with GHS enabled):

<ul class="todo-list">
    <li>
        <label class="todo-list__label todo-list__label_without-description"><input type="checkbox" checked="checked" disabled="disabled"></label>
        <p class="foo">
            foobar
        </p>
    </li>
</ul>
mabryl commented 8 months ago

Also reproducible when changing the alignment on an item in a todo list. Steps to repro:

  1. Create an item in a todo list
  2. Change its alignment, e.g. by centering the text
  3. Check the item on the todo list
  4. Execute editor.setData(editor.getData())

It seems that the checked="checked" attribute goes missing when the text inside the <li> gets wrapped in an additional element, e.g. <p>.

woaiiimipi commented 4 months ago

Any update? Our clients report this issue again, I think it's an very important function.

woaiiimipi commented 2 months ago

@niegowski @Witoso @mabryl Is there any way to avoid it? Too many of our users have encountered this problem.

illia-stv commented 2 months ago

As I see it doesn't work with p element, while with span everything works fine (GHS is not required to check it):

<ul class="todo-list">
    <li>
        <label class="todo-list__label todo-list__label_without-description"><input type="checkbox" checked="checked" disabled="disabled"></label>
        <span class="foo">
            foobar
        </span>
    </li>
</ul>

So conversion treats todo list with p element a little bit differently.

This method override data.modelCursor: https://github.com/ckeditor/ckeditor5/blob/28d0389424dcb74d1163741d33a379e26816fc4c/packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts#L351-L379

Inside modelCursor we could find an array with 3 elements (first and third elements are empty so they will be removed but they have actual attribute which is responsible for checked property while second doesn't have that attribute but have the text node)

image

First element:

image

Second element:

image

Third element:

image

Meanwhile todo list item with span element have only 1 (with text and attributes), not 3:

image image

Here is a method which remove empty elements (so only second element with text data from the array is left but it doesn't have todo-list-checked attribute) so that the reason of data lost: https://github.com/ckeditor/ckeditor5/blob/28d0389424dcb74d1163741d33a379e26816fc4c/packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts#L500-L516

illia-stv commented 2 months ago

Here is example solution: https://github.com/ckeditor/ckeditor5/pull/16785