Przeglądaj źródła

Fix (link): Manual decorators will no longer be corrupted by the link image plugin. Closes #7975.

Marek Lewandowski 5 lat temu
rodzic
commit
73eacd641f

+ 23 - 4
packages/ckeditor5-link/src/linkimageediting.js

@@ -95,7 +95,7 @@ function upcastLink() {
 	return dispatcher => {
 		dispatcher.on( 'element:a', ( evt, data, conversionApi ) => {
 			const viewLink = data.viewItem;
-			const imageInLink = Array.from( viewLink.getChildren() ).find( child => child.name === 'img' );
+			const imageInLink = getFirstImage( viewLink );
 
 			if ( !imageInLink ) {
 				return;
@@ -118,11 +118,11 @@ function upcastLink() {
 			}
 
 			// A full definition of the image feature.
-			// figure > a > img: parent of the link element is an image element.
+			// figure > a > img: parent of the view link element is an image element (figure).
 			let modelElement = data.modelCursor.parent;
 
 			if ( !modelElement.is( 'element', 'image' ) ) {
-				// a > img: parent of the link is not the image element. We need to convert it manually.
+				// a > img: parent of the view link is not the image (figure) element. We need to convert it manually.
 				const conversionResult = conversionApi.convertItem( imageInLink, data.modelCursor );
 
 				// Set image range as conversion result.
@@ -229,6 +229,13 @@ function upcastImageLinkManualDecorator( manualDecorators, decorator ) {
 	return dispatcher => {
 		dispatcher.on( 'element:a', ( evt, data, conversionApi ) => {
 			const viewLink = data.viewItem;
+			const imageInLink = getFirstImage( viewLink );
+
+			// We need to check whether an image is inside a link because the converter handles
+			// only manual decorators for linked images. See #7975.
+			if ( !imageInLink ) {
+				return;
+			}
 
 			const consumableAttributes = {
 				attributes: manualDecorators.get( decorator.id ).attributes
@@ -248,10 +255,22 @@ function upcastImageLinkManualDecorator( manualDecorators, decorator ) {
 			}
 
 			// At this stage we can assume that we have the `<image>` element.
-			const modelElement = data.modelCursor.parent;
+			// `nodeBefore` comes after conversion: `<a><img></a>`.
+			// `parent` comes with full image definition: `<figure><a><img></a></figure>.
+			// See the body of the `upcastLink()` function.
+			const modelElement = data.modelCursor.nodeBefore || data.modelCursor.parent;
 
 			conversionApi.writer.setAttribute( decorator.id, true, modelElement );
 		}, { priority: 'high' } );
 		// Using the same priority that `upcastLink()` converter guarantees that the linked image was properly converted.
 	};
 }
+
+// Returns the first image in a given view element.
+//
+// @private
+// @param {module:engine/view/element~Element}
+// @returns {module:engine/view/element~Element|undefined}
+function getFirstImage( viewElement ) {
+	return Array.from( viewElement.getChildren() ).find( child => child.name === 'img' );
+}

+ 198 - 81
packages/ckeditor5-link/tests/linkimageediting.js

@@ -587,35 +587,57 @@ describe( 'LinkImageEditing', () => {
 		} );
 
 		describe( 'upcast converter', () => {
-			let editor;
-
-			it( 'should upcast attributes from initial data', async () => {
-				editor = await VirtualTestEditor.create( {
-					initialData: '<figure class="image"><a href="url" target="_blank" rel="noopener noreferrer" download="file">' +
-						'<img src="/assets/sample.png"></a></figure>',
-					plugins: [ Paragraph, LinkImageEditing ],
-					link: {
-						decorators: {
-							isExternal: {
-								mode: 'manual',
-								label: 'Open in a new window',
-								attributes: {
-									target: '_blank',
-									rel: 'noopener noreferrer'
-								}
-							},
-							isDownloadable: {
-								mode: 'manual',
-								label: 'Downloadable',
-								attributes: {
-									download: 'file'
+			let editor, model;
+
+			beforeEach( () => {
+				return VirtualTestEditor
+					.create( {
+						plugins: [ Paragraph, LinkImageEditing ],
+						link: {
+							decorators: {
+								isExternal: {
+									mode: 'manual',
+									label: 'Open in a new tab',
+									attributes: {
+										target: '_blank',
+										rel: 'noopener noreferrer'
+									}
+								},
+								isDownloadable: {
+									mode: 'manual',
+									label: 'Downloadable',
+									attributes: {
+										download: 'download'
+									}
+								},
+								isGallery: {
+									mode: 'manual',
+									label: 'Gallery link',
+									attributes: {
+										class: 'gallery'
+									}
 								}
 							}
 						}
-					}
-				} );
+					} )
+					.then( newEditor => {
+						editor = newEditor;
+						model = editor.model;
+					} );
+			} );
 
-				model = editor.model;
+			afterEach( () => {
+				return editor.destroy();
+			} );
+
+			it( 'should upcast attributes', async () => {
+				editor.setData(
+					'<figure class="image">' +
+						'<a href="url" target="_blank" rel="noopener noreferrer" download="download">' +
+							'<img src="/assets/sample.png">' +
+						'</a>' +
+					'</figure>'
+				);
 
 				expect( getModelData( model, { withoutSelection: true } ) ).to.equal(
 					'<image linkHref="url" linkIsDownloadable="true" linkIsExternal="true" src="/assets/sample.png"></image>'
@@ -625,67 +647,20 @@ describe( 'LinkImageEditing', () => {
 			} );
 
 			it( 'should not upcast partial and incorrect attributes', async () => {
-				editor = await VirtualTestEditor.create( {
-					initialData: '<figure class="image"><a href="url" target="_blank" rel="noopener noreferrer" download="something">' +
-						'<img src="/assets/sample.png"></a></figure>',
-					plugins: [ Paragraph, LinkImageEditing ],
-					link: {
-						decorators: {
-							isExternal: {
-								mode: 'manual',
-								label: 'Open in a new window',
-								attributes: {
-									target: '_blank',
-									rel: 'noopener noreferrer'
-								}
-							},
-							isDownloadable: {
-								mode: 'manual',
-								label: 'Downloadable',
-								attributes: {
-									download: 'file'
-								}
-							}
-						}
-					}
-				} );
-
-				model = editor.model;
+				editor.setData(
+					'<figure class="image">' +
+						'<a href="url" target="_blank" rel="noopener noreferrer" download="something">' +
+							'<img src="/assets/sample.png">' +
+						'</a>' +
+					'</figure>'
+				);
 
 				expect( getModelData( model, { withoutSelection: true } ) ).to.equal(
 					'<image linkHref="url" linkIsExternal="true" src="/assets/sample.png"></image>'
 				);
-
-				await editor.destroy();
 			} );
 
-			it( 'allows overwriting conversion process using highest priority', async () => {
-				editor = await VirtualTestEditor.create( {
-					initialData: '',
-					plugins: [ Paragraph, LinkImageEditing ],
-					link: {
-						decorators: {
-							isExternal: {
-								mode: 'manual',
-								label: 'Open in a new window',
-								attributes: {
-									target: '_blank',
-									rel: 'noopener noreferrer'
-								}
-							},
-							isDownloadable: {
-								mode: 'manual',
-								label: 'Downloadable',
-								attributes: {
-									download: 'file'
-								}
-							}
-						}
-					}
-				} );
-
-				model = editor.model;
-
+			it( 'allows overwriting conversion process using highest priority', () => {
 				// Block manual decorator converter. Consume all attributes and do nothing with them.
 				editor.conversion.for( 'upcast' ).add( dispatcher => {
 					dispatcher.on( 'element:a', ( evt, data, conversionApi ) => {
@@ -706,8 +681,150 @@ describe( 'LinkImageEditing', () => {
 				);
 
 				expect( editor.getData() ).to.equal( '<figure class="image"><a href="url"><img src="/assets/sample.png"></a></figure>' );
+			} );
 
-				await editor.destroy();
+			it( 'should upcast the decorators when linked image (figure > a > img)', () => {
+				// (#7975)
+				editor.setData(
+					'<figure class="image">' +
+						'<a class="gallery" href="https://cksource.com" target="_blank" rel="noopener noreferrer" download="download">' +
+							'<img src="sample.jpg" alt="bar">' +
+						'</a>' +
+						'<figcaption>Caption</figcaption>' +
+					'</figure>' +
+					'<p>' +
+						'<a href="https://cksource.com" target="_blank" rel="noopener noreferrer" download="download">' +
+							'https://cksource.com' +
+						'</a>' +
+					'</p>'
+				);
+
+				expect( getModelData( model, { withoutSelection: true } ) ).to.equal(
+					'<image alt="bar" ' +
+						'linkHref="https://cksource.com" ' +
+						'linkIsDownloadable="true" ' +
+						'linkIsExternal="true" ' +
+						'linkIsGallery="true" ' +
+						'src="sample.jpg">' +
+					'</image>' +
+					'<paragraph>' +
+						'<$text linkHref="https://cksource.com" linkIsDownloadable="true" linkIsExternal="true">' +
+							'https://cksource.com' +
+						'</$text>' +
+					'</paragraph>'
+				);
+			} );
+
+			it( 'should upcast the decorators when linked image (a > img)', () => {
+				// (#7975)
+				editor.setData(
+					'<a class="gallery" href="https://cksource.com" target="_blank" rel="noopener noreferrer" download="download">' +
+						'<img src="sample.jpg" alt="bar">' +
+					'</a>' +
+					'<p>' +
+						'<a href="https://cksource.com" target="_blank" rel="noopener noreferrer" download="download">' +
+							'https://cksource.com' +
+						'</a>' +
+					'</p>'
+				);
+
+				expect( getModelData( model, { withoutSelection: true } ) ).to.equal(
+					'<image alt="bar" ' +
+						'linkHref="https://cksource.com" ' +
+						'linkIsDownloadable="true" ' +
+						'linkIsExternal="true" ' +
+						'linkIsGallery="true" ' +
+						'src="sample.jpg">' +
+					'</image>' +
+					'<paragraph>' +
+						'<$text linkHref="https://cksource.com" linkIsDownloadable="true" linkIsExternal="true">' +
+							'https://cksource.com' +
+						'</$text>' +
+					'</paragraph>'
+				);
+			} );
+		} );
+
+		describe( 'downcast converter', () => {
+			let editor, model;
+
+			beforeEach( () => {
+				return VirtualTestEditor
+					.create( {
+						plugins: [ Paragraph, LinkImageEditing ],
+						link: {
+							decorators: {
+								isExternal: {
+									mode: 'manual',
+									label: 'Open in a new tab',
+									attributes: {
+										target: '_blank',
+										rel: 'noopener noreferrer'
+									}
+								},
+								isDownloadable: {
+									mode: 'manual',
+									label: 'Downloadable',
+									attributes: {
+										download: 'download'
+									}
+								},
+								isGallery: {
+									mode: 'manual',
+									label: 'Gallery link',
+									attributes: {
+										class: 'gallery'
+									}
+								}
+							}
+						}
+					} )
+					.then( newEditor => {
+						editor = newEditor;
+						model = editor.model;
+					} );
+			} );
+
+			afterEach( () => {
+				return editor.destroy();
+			} );
+
+			it( 'should downcast the decorators after applying a change', () => {
+				setModelData( model,
+					'[<image alt="bar" src="sample.jpg"></image>]' +
+					'<paragraph>' +
+						'<$text>https://cksource.com</$text>' +
+					'</paragraph>'
+				);
+
+				editor.execute( 'link', 'https://cksource.com', {
+					linkIsDownloadable: true,
+					linkIsExternal: true,
+					linkIsGallery: true
+				} );
+
+				model.change( writer => {
+					writer.setSelection( model.document.getRoot().getChild( 1 ).getChild( 0 ), 'on' );
+				} );
+
+				editor.execute( 'link', 'https://cksource.com', {
+					linkIsDownloadable: true,
+					linkIsExternal: true,
+					linkIsGallery: true
+				} );
+
+				expect( editor.getData() ).to.equal(
+					'<figure class="image">' +
+						'<a class="gallery" href="https://cksource.com" download="download" target="_blank" rel="noopener noreferrer">' +
+							'<img src="sample.jpg" alt="bar">' +
+						'</a>' +
+					'</figure>' +
+					'<p>' +
+						'<a class="gallery" href="https://cksource.com" download="download" target="_blank" rel="noopener noreferrer">' +
+							'https://cksource.com' +
+						'</a>' +
+					'</p>'
+				);
 			} );
 		} );
 	} );

+ 32 - 0
tests/manual/all-features.html

@@ -157,6 +157,38 @@ function bar() {
 			</tbody>
 		</table>
 	</figure>
+
+	<hr>
+
+	<h2>Link images + Link decorators</h2>
+
+	<table>
+		<tr>
+			<td>Linked text</td>
+			<td>Linked image (<code>figure > a > img</code>)</td>
+			<td>Linked image (<code>a > img</code>)</td>
+		</tr>
+		<tr>
+			<td>
+				<p>
+					<a href="https://cksource.com" target="_blank" rel="noopener noreferrer" download="download">https://cksource.com</a>
+				</p>
+			</td>
+			<td>
+				<figure class="image">
+					<a class="gallery" href="https://cksource.com">
+						<img src="sample.jpg" alt="bar">
+					</a>
+					<figcaption>Caption</figcaption>
+				</figure>
+			</td>
+			<td>
+				<a class="gallery" href="https://cksource.com" target="_blank" rel="noopener noreferrer" download="download">
+					<img src="sample.jpg" alt="bar">
+				</a>
+			</td>
+		</tr>
+	</table>
 </div>
 
 <style>

+ 26 - 0
tests/manual/all-features.js

@@ -117,6 +117,32 @@ ClassicEditor
 					minimumCharacters: 1
 				}
 			]
+		},
+		link: {
+			decorators: {
+				isExternal: {
+					mode: 'manual',
+					label: 'Open in a new tab',
+					attributes: {
+						target: '_blank',
+						rel: 'noopener noreferrer'
+					}
+				},
+				isDownloadable: {
+					mode: 'manual',
+					label: 'Downloadable',
+					attributes: {
+						download: 'download'
+					}
+				},
+				isGallery: {
+					mode: 'manual',
+					label: 'Gallery link',
+					attributes: {
+						class: 'gallery'
+					}
+				}
+			}
 		}
 	} )
 	.then( editor => {

+ 15 - 0
tests/manual/all-features.md

@@ -4,6 +4,8 @@
 
 It should contain as many features as we developed. By resizing your viewport you can observe whether grouping toolbar items work.
 
+---
+
 ## Editor
 
 There should be "two pages" with two sections per each page. Pages should be separated by the page break feature.
@@ -20,16 +22,29 @@ There should be "two pages" with two sections per each page. Pages should be sep
 
 **Code blocks in the table** - in the table (4x3), in the second and fourth columns should be visible code snippets.
 
+**Horizontal line** - There is the `<hr>` in the source HTML. It should be displayed in the editor.
+
+**Link images + Link decorators** - in the table (3x2), there are a linked text and two linked images that uses the manual decorators feature:
+  - Left column: the text with two decorators enabled: `Open in a new tab`, `Downloadable`
+  - Middle column: an image with the caption that is a `Gallery link`
+  - Right column: an image without the caption with enabled all decorators (listed above)
+
+---
+
 ## Action buttons
 
 - Clear editor - calls `editor.setData( '' )`
 - Open print preview - opens the print preview window
 - Turn on/off read-only mode - toggle read-only mode
 
+---
+
 ## Console
 
 Wordcount plugin logs into the console number of characters and words in the editor's data.
 
+---
+
 ## Additional
 
 - Empty editor should display a placeholder: `'Type the content here!'`