Is cleanList converter still needed in lists feature?

[scofalik]

Provide a description of the task

We have this code in list feature:

export function cleanList( evt, data, conversionApi ) {
	if ( conversionApi.consumable.test( data.viewItem, { name: true } ) ) {
		// Caching children because when we start removing them iterating fails.
		const children = Array.from( data.viewItem.getChildren() );

		for ( const child of children ) {
			const isWrongElement = !( child.is( 'element', 'li' ) || isList( child ) );

			if ( isWrongElement ) {
				child._remove();
			}
		}
	}
}

which cleans <ul>s and <ol>s from incorrect data but... honestly, the main reason was removing whitespaces. At the moment, AFAIK, we have white-spaces somehow covered in DOM converter and also there actually could be elements in <ul> that are there on purpose (let's say some custom elements, or ... incorrectly downcasted markers like described here: #9622).

It seems to me that maybe this converter is not needed at the moment and may be actually harmful.

[pomek]

Two tests fail after removing the converter:

ckeditor5/packages/ckeditor5-list/tests/listediting.js

Lines 438 to 441 in b82d53b

	testList(
	'clears incorrect elements',
	'<ul>x<li>a</li><li>b</li><p>xxx</p>x</ul><p>c</p>', '<ul><li>a</li><li>b</li></ul><p>c</p>'
	);

Expected :"<ul><li>a</li><li>b</li></ul><p>c</p>"
Actual   :"<p>x</p><ul><li>a</li><li>b</li></ul><p>xxx</p><p>x</p><p>c</p>"

ckeditor5/packages/ckeditor5-list/tests/listediting.js

Lines 1844 to 1931 in b82d53b

	it( 'mixed lists deep structure, white spaces, incorrect content, empty items', testList(
	'<p>foo</p>' +
	'<ul>' +
	' xxx' +
	' <li>' +
	' 1' +
	' <ul>' +
	' xxx' +
	' <li>' +
	' <ul><li></li><li>1.1.2</li></ul>' +
	' <ol><li>1.1.3</li><li>1.1.4</li></ol>' + // Will be changed to <ul>.
	' </li>' +
	' <li>' +
	' <ul><li>1.2.1</li></ul>' +
	' </li>' +
	' xxx' +
	' </ul>' +
	' </li>' +
	' <li>2</li>' +
	' <li>' +
	' <ol>' +
	' <p>xxx</p>' +
	' <li>' +
	' 3<strong>.</strong>1' + // Test multiple text nodes in <li>.
	' <ul>' +
	' <li>' +
	' 3.1.1' +
	' <ol><li>3.1.1.1</li></ol>' +
	' <ul><li>3.1.1.2</li></ul>' + // Will be changed to <ol>.
	' </li>' +
	' <li>3.1.2</li>' +
	' </ul>' +
	' </li>' +
	' </ol>' +
	' <p>xxx</p>' +
	' <ul>' + // Since <p> gets removed, this will become <ol>.
	' <li>3.2</li>' +
	' <li>3.3</li>' +
	' </ul>' +
	' </li>' +
	' <p>xxx</p>' +
	'</ul>' +
	'<p>bar</p>',
	'<p>foo</p>' +
	'<ul>' +
	'<li>' +
	'1' +
	'<ul>' +
	'<li>' +
	'&nbsp;' +
	'<ul>' +
	'<li>&nbsp;</li>' +
	'<li>1.1.2</li>' +
	'<li>1.1.3</li>' +
	'<li>1.1.4</li>' +
	'</ul>' +
	'</li>' +
	'<li>' +
	'&nbsp;' +
	'<ul><li>1.2.1</li></ul>' +
	'</li>' +
	'</ul>' +
	'</li>' +
	'<li>2</li>' +
	'<li>' +
	'&nbsp;' +
	'<ol>' +
	'<li>' +
	'3<strong>.</strong>1' +
	'<ul>' +
	'<li>' +
	'3.1.1' +
	'<ol>' +
	'<li>3.1.1.1</li>' +
	'<li>3.1.1.2</li>' +
	'</ol>' +
	'</li>' +
	'<li>3.1.2</li>' +
	'</ul>' +
	'</li>' +
	'<li>3.2</li>' +
	'<li>3.3</li>' +
	'</ol>' +
	'</li>' +
	'</ul>' +
	'<p>bar</p>'
	) );

Expected :"<p>foo</p>\n<ul>\n  <li>1<ul>\n      <li>&nbsp;<ul>\n          <li>&nbsp;</li>\n          <li>1.1.2</li>\n          <li>1.1.3</li>\n          <li>1.1.4</li>\n        </ul>\n      </li>\n      <li>&nbsp;<ul>\n          <li>1.2.1</li>\n        </ul>\n      </li>\n    </ul>\n  </li>\n  <li>2</li>\n  <li>&nbsp;<ol>\n      <li>3<strong>.</strong>1<ul>\n          <li>3.1.1<ol>\n              <li>3.1.1.1</li>\n              <li>3.1.1.2</li>\n            </ol>\n          </li>\n          <li>3.1.2</li>\n        </ul>\n      </li>\n      <li>3.2</li>\n      <li>3.3</li>\n    </ol>\n  </li>\n</ul>\n<p>bar</p>"
Actual   :"<p>foo</p>\n<p>xxx</p>\n<ul>\n  <li>1</li>\n</ul>\n<p>xxx</p>\n<ul>\n  <li>&nbsp;<ul>\n      <li>&nbsp;</li>\n      <li>1.1.2</li>\n      <li>1.1.3</li>\n      <li>1.1.4</li>\n    </ul>\n  </li>\n  <li>&nbsp;<ul>\n      <li>1.2.1</li>\n    </ul>\n  </li>\n</ul>\n<p>xxx&nbsp;</p>\n<ul>\n  <li>2</li>\n  <li>&nbsp;</li>\n</ul>\n<p>xxx</p>\n<ol>\n  <li>3<strong>.</strong>1<ul>\n      <li>3.1.1<ol>\n          <li>3.1.1.1</li>\n          <li>3.1.1.2</li>\n        </ol>\n      </li>\n      <li>3.1.2</li>\n    </ul>\n  </li>\n</ol>\n<ul>\n  <li>3.2</li>\n  <li>3.3</li>\n</ul>\n<p>xxx</p>\n<p>bar</p>"

In short, invalid elements inside <ul>/<ol> are converted to paragraphs and moved before/after the list.

[CKEditorBot]

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

[CKEditorBot]

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).