Fix: Handle existing picture element correctly on 'sources' downcast.

ckeditor · Oct 15, 2024 · 07892dd · 07892dd
1 parent 4d624ee
commit 07892dd
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 3 deletions.
diff --git a/packages/ckeditor5-image/src/image/converters.ts b/packages/ckeditor5-image/src/image/converters.ts
@@ -249,9 +249,18 @@ export function downcastSourcesAttribute( imageUtils: ImageUtils ): ( dispatcher
 				viewElement = parentElement;
 			}
 
-			// Insert the picture and move img into it.
-			viewWriter.insert( viewWriter.createPositionBefore( imgElement ), pictureElement );
-			viewWriter.move( viewWriter.createRangeOn( imgElement ), viewWriter.createPositionAt( pictureElement, 'end' ) );
+			const prevPictureElement = imgElement.parent!.is( 'element', 'picture' ) ? imgElement.parent : null;
+
+			if ( prevPictureElement ) {
+				// Insert a new picture, move img into it, remove previous picture element (ckeditor5#17192).
+				viewWriter.insert( viewWriter.createPositionBefore( prevPictureElement ), pictureElement );
+				viewWriter.move( viewWriter.createRangeOn( imgElement ), viewWriter.createPositionAt( pictureElement, 'end' ) );
+				viewWriter.remove( prevPictureElement );
+			} else {
+				// Insert the picture and move img into it.
+				viewWriter.insert( viewWriter.createPositionBefore( imgElement ), pictureElement );
+				viewWriter.move( viewWriter.createRangeOn( imgElement ), viewWriter.createPositionAt( pictureElement, 'end' ) );
+			}
 
 			// Apply collected attribute elements over the new picture element.
 			for ( const attributeElement of attributeElements ) {

diff --git a/packages/ckeditor5-image/tests/pictureediting.js b/packages/ckeditor5-image/tests/pictureediting.js
@@ -1857,6 +1857,94 @@ describe( 'PictureEditing', () => {
 
 					expect( editor.getData() ).to.equal( '<p>foo<img src="/assets/sample.png">bar</p>' );
 				} );
+
+				it( 'should downcast changed "sources" attribute on an existing picture element', () => {
+					editor.setData(
+						'<figure class="image">' +
+							'<picture>' +
+								'<source srcset="">' +
+								'<img src="/assets/sample.png">' +
+							'</picture>' +
+							'<figcaption>Caption</figcaption>' +
+						'</figure>'
+					);
+
+					model.change( writer => {
+						writer.setAttribute(
+							'sources',
+							[
+								{
+									srcset: '/assets/sample2.png'
+								}
+							],
+							modelDocument.getRoot().getChild( 0 )
+						);
+					} );
+
+					expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
+						'<figure class="ck-widget image" contenteditable="false">' +
+							'<picture>' +
+								'<source srcset="/assets/sample2.png"></source>' +
+								'<img src="/assets/sample.png"></img>' +
+							'</picture>' +
+							'<figcaption ' +
+								'aria-label="Caption for the image" ' +
+								'class="ck-editor__editable ck-editor__nested-editable" ' +
+								'contenteditable="true" ' +
+								'data-placeholder="Enter image caption" ' +
+								'role="textbox" ' +
+								'tabindex="-1">' +
+									'Caption' +
+							'</figcaption>' +
+						'</figure>'
+					);
+				} );
+
+				it( 'should downcast changed "sources" attribute on an existing linked picture element', () => {
+					editor.setData(
+						'<figure class="image">' +
+							'<a href="https://ckeditor.com">' +
+								'<picture>' +
+									'<source srcset="/assets/sample.png">' +
+									'<img src="/assets/sample.png">' +
+								'</picture>' +
+							'</a>' +
+							'<figcaption>Caption</figcaption>' +
+						'</figure>'
+					);
+
+					model.change( writer => {
+						writer.setAttribute(
+							'sources',
+							[
+								{
+									srcset: '/assets/sample2.png'
+								}
+							],
+							modelDocument.getRoot().getChild( 0 )
+						);
+					} );
+
+					expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
+						'<figure class="ck-widget image" contenteditable="false">' +
+							'<a href="https://ckeditor.com">' +
+								'<picture>' +
+									'<source srcset="/assets/sample2.png"></source>' +
+									'<img src="/assets/sample.png"></img>' +
+								'</picture>' +
+							'</a>' +
+							'<figcaption ' +
+								'aria-label="Caption for the image" ' +
+								'class="ck-editor__editable ck-editor__nested-editable" ' +
+								'contenteditable="true" ' +
+								'data-placeholder="Enter image caption" ' +
+								'role="textbox" ' +
+								'tabindex="-1">' +
+									'Caption' +
+							'</figcaption>' +
+						'</figure>'
+					);
+				} );
 			} );
 		} );
 	} );