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

woaiiimipi · 2023-12-28T04:03:56Z

📝 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:

Reload:

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

Browser: Latest Chrome
OS: MacOS / Windows
First affected CKEditor version: Latest ckeditor version
Installed CKEditor plugins:
"@ckeditor/ckeditor5-alignment": "^40.0.0",
"@ckeditor/ckeditor5-autoformat": "^40.0.0",
"@ckeditor/ckeditor5-basic-styles": "^40.0.0",
"@ckeditor/ckeditor5-block-quote": "^40.0.0",
"@ckeditor/ckeditor5-cloud-services": "^40.0.0",
"@ckeditor/ckeditor5-dev-translations": "^32.1.2",
"@ckeditor/ckeditor5-dev-utils": "^32.1.2",
"@ckeditor/ckeditor5-editor-classic": "^40.0.0",
"@ckeditor/ckeditor5-editor-inline": "^40.0.0",
"@ckeditor/ckeditor5-essentials": "^40.0.0",
"@ckeditor/ckeditor5-font": "^40.0.0",
"@ckeditor/ckeditor5-heading": "^40.0.0",
"@ckeditor/ckeditor5-horizontal-line": "^40.0.0",
"@ckeditor/ckeditor5-html-support": "^40.0.0",
"@ckeditor/ckeditor5-image": "^40.0.0",
"@ckeditor/ckeditor5-indent": "^40.0.0",
"@ckeditor/ckeditor5-link": "^40.0.0",
"@ckeditor/ckeditor5-list": "^40.0.0",
"@ckeditor/ckeditor5-media-embed": "^40.0.0",
"@ckeditor/ckeditor5-mention": "^40.0.0",
"@ckeditor/ckeditor5-page-break": "^40.0.0",
"@ckeditor/ckeditor5-paragraph": "^40.0.0",
"@ckeditor/ckeditor5-paste-from-office": "^40.0.0",
"@ckeditor/ckeditor5-source-editing": "^40.0.0",
"@ckeditor/ckeditor5-special-characters": "^40.0.0",
"@ckeditor/ckeditor5-table": "^40.0.0",
"@ckeditor/ckeditor5-theme-lark": "^40.0.0",
"@ckeditor/ckeditor5-typing": "^40.0.0",
"@ckeditor/ckeditor5-word-count": "^40.0.0",
"@ckeditor/ckeditor5-ui": "^40.0.0",
"@ckeditor/ckeditor5-undo": "^40.0.0",

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

niegowski · 2024-01-09T17:24:35Z

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 · 2024-02-19T09:50:31Z

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

Create an item in a todo list
Change its alignment, e.g. by centering the text
Check the item on the todo list
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 · 2024-06-12T01:19:41Z

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

woaiiimipi · 2024-07-22T08:13:51Z

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

illia-stv · 2024-07-23T15:11:24Z

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:

ckeditor5/packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts

Lines 351 to 379 in 28d0389

    
           private _updateConversionResult( modelElement: ModelElement, data: UpcastConversionData ): void { 
        
           	const parts = this._getSplitParts( modelElement ); 
        
           	const writer = this.conversionApi.writer!; 
        
           	// Set conversion result range - only if not set already. 
        
           	if ( !data.modelRange ) { 
        
           		data.modelRange = writer.createRange( 
        
           			writer.createPositionBefore( modelElement ), 
        
           			writer.createPositionAfter( parts[ parts.length - 1 ] ) 
        
           		); 
        
           	} 
        
           	const savedCursorParent = this._cursorParents.get( modelElement ); 
        
           	// Now we need to check where the `modelCursor` should be. 
        
           	if ( savedCursorParent ) { 
        
           		// If we split parent to insert our element then we want to continue conversion in the new part of the split parent. 
        
           		// 
        
           		// before: <allowed><notAllowed>foo[]</notAllowed></allowed> 
        
           		// after:  <allowed><notAllowed>foo</notAllowed> <converted></converted> <notAllowed>[]</notAllowed></allowed> 
        
           		data.modelCursor = writer.createPositionAt( savedCursorParent, 0 ); 
        
           	} else { 
        
           		// Otherwise just continue after inserted element. 
        
           		data.modelCursor = data.modelRange.end; 
        
           	} 
        
           }

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)

First element:

Second element:

Third element:

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

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:

ckeditor5/packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts

Lines 500 to 516 in 28d0389

    
           	private _removeEmptyElements(): void { 
        
           		let anyRemoved = false; 
        
           		for ( const element of this._splitParts.keys() ) { 
        
           			if ( element.isEmpty && !this._emptyElementsToKeep.has( element ) ) { 
        
           				this.conversionApi.writer.remove( element ); 
        
           				this._splitParts.delete( element ); 
        
           				anyRemoved = true; 
        
           			} 
        
           		} 
        
           		if ( anyRemoved ) { 
        
           			this._removeEmptyElements(); 
        
           		} 
        
           	} 
        
           }

illia-stv · 2024-07-23T16:39:08Z

Here is example solution: #16785

Fix (list): A to-do list should preserve the state of the checked items on the data load. Closes #15602.

woaiiimipi added the type:bug This issue reports a buggy (incorrect) behavior. label Dec 28, 2023

Witoso added package:list domain:ui/ux This issue reports a problem related to UI or UX. labels Jan 9, 2024

mabryl added the support:2 An issue reported by a commercially licensed client. label Feb 19, 2024

illia-stv mentioned this issue Jul 23, 2024

Fix todoListChecked data lost in to-do list #16785

Merged

niegowski closed this as completed in #16785 Aug 5, 2024

niegowski added a commit that referenced this issue Aug 5, 2024

Merge pull request #16785 from ckeditor/ck/15602

6316589

Fix (list): A to-do list should preserve the state of the checked items on the data load. Closes #15602.

CKEditorBot added this to the iteration 77 milestone Aug 5, 2024

CKEditorBot mentioned this issue Aug 6, 2024

📄 Nightly releases changelog #14724

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

woaiiimipi commented Dec 28, 2023

niegowski commented Jan 9, 2024

mabryl commented Feb 19, 2024

woaiiimipi commented Jun 12, 2024

woaiiimipi commented Jul 22, 2024

illia-stv commented Jul 23, 2024 •

edited

Loading

illia-stv commented Jul 23, 2024 •

edited

Loading

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

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

Comments

woaiiimipi commented Dec 28, 2023

📝 Provide detailed reproduction steps (if any)

✔️ Expected result

❌ Actual result

❓ Possible solution

📃 Other details

niegowski commented Jan 9, 2024

mabryl commented Feb 19, 2024

woaiiimipi commented Jun 12, 2024

woaiiimipi commented Jul 22, 2024

illia-stv commented Jul 23, 2024 • edited Loading

illia-stv commented Jul 23, 2024 • edited Loading

illia-stv commented Jul 23, 2024 •

edited

Loading

illia-stv commented Jul 23, 2024 •

edited

Loading