Skip to content

Commit

Permalink
Fix (link): Manual decorators will no longer be corrupted by the link…
Browse files Browse the repository at this point in the history 
… image plugin. Closes #7975.
  • Loading branch information
@mlewand
mlewand authored Sep 23, 2020
2 parents 07ba48e + 58fc76d commit 73eacd6
Show file tree
Hide file tree
Showing 5 changed files with 294 additions and 85 deletions.
27 changes: 23 additions & 4 deletions packages/ckeditor5-link/src/linkimageediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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' );
}
279 changes: 198 additions & 81 deletions packages/ckeditor5-link/tests/linkimageediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>'
Expand All @@ -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 ) => {
Expand All @@ -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>'
);
} );
} );
} );
Expand Down
Loading

0 comments on commit 73eacd6

Please sign in to comment.