Markers should not be lost on list items and in table cells in the da…

…ta pipeline.
ckeditor · Jan 20, 2023 · fe78ca0 · fe78ca0
1 parent 30159ba
commit fe78ca0
Show file tree

Hide file tree

Showing 11 changed files with 763 additions and 58 deletions.
diff --git a/packages/ckeditor5-clipboard/tests/dragdrop.js b/packages/ckeditor5-clipboard/tests/dragdrop.js
@@ -863,7 +863,7 @@ describe( 'Drag and Drop', () => {
 				expect( spyClipboardOutput.firstCall.firstArg.method ).to.equal( 'dragstart' );
 				expect( spyClipboardOutput.firstCall.firstArg.dataTransfer ).to.equal( dataTransferMock );
 				expect( stringifyView( spyClipboardOutput.firstCall.firstArg.content ) ).to.equal(
-					'<figure class="table"><table><tbody><tr><td>abc</td></tr></tbody></table></figure>'
+					'<figure class="table"><table><tbody><tr><td><p>abc</p></td></tr></tbody></table></figure>'
 				);
 
 				dataTransferMock.dropEffect = 'move';
@@ -929,7 +929,7 @@ describe( 'Drag and Drop', () => {
 				expect( spyClipboardOutput.firstCall.firstArg.method ).to.equal( 'dragstart' );
 				expect( spyClipboardOutput.firstCall.firstArg.dataTransfer ).to.equal( dataTransferMock );
 				expect( stringifyView( spyClipboardOutput.firstCall.firstArg.content ) ).to.equal(
-					'<figure class="table"><table><tbody><tr><td>abc</td></tr></tbody></table></figure>'
+					'<figure class="table"><table><tbody><tr><td><p>abc</p></td></tr></tbody></table></figure>'
 				);
 			} );
 

diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts
@@ -29,7 +29,8 @@ import {
 	indexOf,
 	getAncestors,
 	isText,
-	isComment
+	isComment,
+	first
 } from '@ckeditor/ckeditor5-utils';
 
 import type ViewNode from './node';
@@ -558,7 +559,8 @@ export default class DomConverter {
 			}
 
 			const transparentRendering = childView.is( 'element' ) &&
-				( childView.getCustomProperty( 'dataPipeline:transparentRendering' ) as boolean | undefined );
+				!!childView.getCustomProperty( 'dataPipeline:transparentRendering' ) &&
+				!first( childView.getAttributes() );
 
 			if ( transparentRendering && this.renderingMode == 'data' ) {
 				yield* this.viewChildrenToDom( childView, options );

diff --git a/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js b/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js
@@ -1291,6 +1291,68 @@ describe( 'DomConverter', () => {
 					sinon.match.string // Link to the documentation
 				);
 			} );
+
+			it( 'should not be transparent in the data pipeline if has any attribute', () => {
+				converter.renderingMode = 'data';
+				converter.blockFillerMode = 'nbsp';
+
+				const warnStub = testUtils.sinon.stub( console, 'warn' );
+
+				const viewList = parse(
+					'<container:div>' +
+						'<attribute:ul>' +
+							'<attribute:li>' +
+								'<container:p class="style">foo</container:p>' +
+							'</attribute:li>' +
+							'<attribute:li>' +
+								'<container:p data-foo="123">bar</container:p>' +
+							'</attribute:li>' +
+							'<attribute:li>' +
+								'<container:p>baz</container:p>' +
+							'</attribute:li>' +
+						'</attribute:ul>' +
+					'</container:div>'
+				);
+
+				const bogusParagraph1 = viewList.getChild( 0 ).getChild( 0 ).getChild( 0 );
+				const bogusParagraph2 = viewList.getChild( 0 ).getChild( 1 ).getChild( 0 );
+				const bogusParagraph3 = viewList.getChild( 0 ).getChild( 2 ).getChild( 0 );
+
+				bogusParagraph1._setCustomProperty( 'dataPipeline:transparentRendering', true );
+				bogusParagraph2._setCustomProperty( 'dataPipeline:transparentRendering', true );
+				bogusParagraph3._setCustomProperty( 'dataPipeline:transparentRendering', true );
+
+				const domDivChildren = Array.from( converter.viewChildrenToDom( viewList ) );
+
+				expect( domDivChildren.length ).to.equal( 1 );
+				expect( domDivChildren[ 0 ].tagName.toLowerCase() ).to.equal( 'ul' );
+
+				const domUlChildren = Array.from( domDivChildren[ 0 ].childNodes );
+
+				expect( domUlChildren.length ).to.equal( 3 );
+				expect( domUlChildren[ 0 ].tagName.toLowerCase() ).to.equal( 'li' );
+				expect( domUlChildren[ 1 ].tagName.toLowerCase() ).to.equal( 'li' );
+				expect( domUlChildren[ 2 ].tagName.toLowerCase() ).to.equal( 'li' );
+
+				const domUl1Children = Array.from( domUlChildren[ 0 ].childNodes );
+				const domUl2Children = Array.from( domUlChildren[ 1 ].childNodes );
+				const domUl3Children = Array.from( domUlChildren[ 2 ].childNodes );
+
+				expect( domUl1Children.length ).to.equal( 1 );
+				expect( domUl1Children[ 0 ].tagName.toLowerCase() ).to.equal( 'p' );
+				expect( domUl1Children[ 0 ].getAttribute( 'class' ) ).to.equal( 'style' );
+				expect( domUl1Children[ 0 ].firstChild.data ).to.equal( 'foo' );
+
+				expect( domUl2Children.length ).to.equal( 1 );
+				expect( domUl2Children[ 0 ].tagName.toLowerCase() ).to.equal( 'p' );
+				expect( domUl2Children[ 0 ].getAttribute( 'data-foo' ) ).to.equal( '123' );
+				expect( domUl2Children[ 0 ].firstChild.data ).to.equal( 'bar' );
+
+				expect( domUl3Children.length ).to.equal( 1 );
+				expect( domUl3Children[ 0 ].data ).to.equal( 'baz' );
+
+				sinon.assert.notCalled( warnStub );
+			} );
 		} );
 	} );
 

diff --git a/packages/ckeditor5-html-support/tests/htmlcomment-integration.js b/packages/ckeditor5-html-support/tests/htmlcomment-integration.js
@@ -1157,8 +1157,8 @@ describe( 'HtmlComment integration', () => {
 						'<tbody>' +
 							'<tr>' +
 								'<td>' +
-									'<!-- c1 -->' +
 									'&nbsp;' +
+									'<!-- c1 -->' +
 								'</td>' +
 							'</tr>' +
 						'</tbody>' +

diff --git a/packages/ckeditor5-list/src/documentlist/converters.ts b/packages/ckeditor5-list/src/documentlist/converters.ts
@@ -369,12 +369,15 @@ export function bogusParagraphCreator(
 			return null;
 		}
 
-		const viewElement = writer.createContainerElement( 'span', { class: 'ck-list-bogus-paragraph' } );
-
-		if ( dataPipeline ) {
-			writer.setCustomProperty( 'dataPipeline:transparentRendering', true, viewElement );
+		if ( !dataPipeline ) {
+			return writer.createContainerElement( 'span', { class: 'ck-list-bogus-paragraph' } );
 		}
 
+		// Using `<p>` in case there are some markers on it and transparentRendering will render it anyway.
+		const viewElement = writer.createContainerElement( 'p' );
+
+		writer.setCustomProperty( 'dataPipeline:transparentRendering', true, viewElement );
+
 		return viewElement;
 	};
 }