Skip to content

Commit 380f253

Browse files
DumluregnDumluregn
committed
Merge branch 'master' into cc/image-merge-fields
2 parents 305ec0f + 6257c78 commit 380f253

File tree

4 files changed

+219
-9
lines changed

4 files changed

+219
-9
lines changed

‎packages/ckeditor5-image/src/image/converters.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -229,13 +229,6 @@ export function downcastSourcesAttribute( imageUtils: ImageUtils ): ( dispatcher
229229
const attributeNewValue = data.attributeNewValue as null | Array<ViewElementAttributes>;
230230

231231
if ( attributeNewValue && attributeNewValue.length ) {
232-
// Make sure <picture> does not break attribute elements, for instance <a> in linked images.
233-
const pictureElement = viewWriter.createContainerElement( 'picture', null,
234-
attributeNewValue.map( sourceAttributes => {
235-
return viewWriter.createEmptyElement( 'source', sourceAttributes );
236-
} )
237-
);
238-
239232
// Collect all wrapping attribute elements.
240233
const attributeElements = [];
241234
let viewElement = imgElement.parent;
@@ -249,8 +242,23 @@ export function downcastSourcesAttribute( imageUtils: ImageUtils ): ( dispatcher
249242
viewElement = parentElement;
250243
}
251244

252-
// Insert the picture and move img into it.
253-
viewWriter.insert( viewWriter.createPositionBefore( imgElement ), pictureElement );
245+
const hasPictureElement = imgElement.parent!.is( 'element', 'picture' );
246+
247+
// Reuse existing <picture> element (ckeditor5#17192) or create a new one.
248+
const pictureElement = hasPictureElement ? imgElement.parent : viewWriter.createContainerElement( 'picture', null );
249+
250+
if ( !hasPictureElement ) {
251+
viewWriter.insert( viewWriter.createPositionBefore( imgElement ), pictureElement );
252+
}
253+
254+
viewWriter.remove( viewWriter.createRangeIn( pictureElement ) );
255+
256+
viewWriter.insert( viewWriter.createPositionAt( pictureElement, 'end' ),
257+
attributeNewValue.map( sourceAttributes => {
258+
return viewWriter.createEmptyElement( 'source', sourceAttributes );
259+
} )
260+
);
261+
254262
viewWriter.move( viewWriter.createRangeOn( imgElement ), viewWriter.createPositionAt( pictureElement, 'end' ) );
255263

256264
// Apply collected attribute elements over the new picture element.

‎packages/ckeditor5-image/tests/pictureediting.js

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1865,6 +1865,158 @@ describe( 'PictureEditing', () => {
18651865

18661866
expect( editor.getData() ).to.equal( '<p>foo<img src="/assets/sample.png">bar</p>' );
18671867
} );
1868+
1869+
it( 'should downcast changed "sources" attribute on an existing picture element', () => {
1870+
editor.setData(
1871+
'<figure class="image">' +
1872+
'<picture>' +
1873+
'<source srcset="">' +
1874+
'<img src="/assets/sample.png">' +
1875+
'</picture>' +
1876+
'<figcaption>Caption</figcaption>' +
1877+
'</figure>'
1878+
);
1879+
1880+
model.change( writer => {
1881+
writer.setAttribute(
1882+
'sources',
1883+
[
1884+
{
1885+
srcset: '/assets/sample2.png'
1886+
}
1887+
],
1888+
modelDocument.getRoot().getChild( 0 )
1889+
);
1890+
} );
1891+
1892+
expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
1893+
'<figure class="ck-widget image" contenteditable="false">' +
1894+
'<picture>' +
1895+
'<source srcset="/assets/sample2.png"></source>' +
1896+
'<img src="/assets/sample.png"></img>' +
1897+
'</picture>' +
1898+
'<figcaption ' +
1899+
'aria-label="Caption for the image" ' +
1900+
'class="ck-editor__editable ck-editor__nested-editable" ' +
1901+
'contenteditable="true" ' +
1902+
'data-placeholder="Enter image caption" ' +
1903+
'role="textbox" ' +
1904+
'tabindex="-1">' +
1905+
'Caption' +
1906+
'</figcaption>' +
1907+
'</figure>'
1908+
);
1909+
} );
1910+
1911+
it( 'should downcast changed "sources" attribute on an existing linked picture element', () => {
1912+
editor.setData(
1913+
'<figure class="image">' +
1914+
'<a href="https://ckeditor.com">' +
1915+
'<picture>' +
1916+
'<source srcset="/assets/sample.png">' +
1917+
'<img src="/assets/sample.png">' +
1918+
'</picture>' +
1919+
'</a>' +
1920+
'<figcaption>Caption</figcaption>' +
1921+
'</figure>'
1922+
);
1923+
1924+
model.change( writer => {
1925+
writer.setAttribute(
1926+
'sources',
1927+
[
1928+
{
1929+
srcset: '/assets/sample2.png'
1930+
}
1931+
],
1932+
modelDocument.getRoot().getChild( 0 )
1933+
);
1934+
} );
1935+
1936+
expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
1937+
'<figure class="ck-widget image" contenteditable="false">' +
1938+
'<a href="https://ckeditor.com">' +
1939+
'<picture>' +
1940+
'<source srcset="/assets/sample2.png"></source>' +
1941+
'<img src="/assets/sample.png"></img>' +
1942+
'</picture>' +
1943+
'</a>' +
1944+
'<figcaption ' +
1945+
'aria-label="Caption for the image" ' +
1946+
'class="ck-editor__editable ck-editor__nested-editable" ' +
1947+
'contenteditable="true" ' +
1948+
'data-placeholder="Enter image caption" ' +
1949+
'role="textbox" ' +
1950+
'tabindex="-1">' +
1951+
'Caption' +
1952+
'</figcaption>' +
1953+
'</figure>'
1954+
);
1955+
} );
1956+
1957+
it( 'should keep existing picture element attributes when downcasting "sources" attribute', () => {
1958+
editor.model.schema.extend( 'imageBlock', {
1959+
allowAttributes: [ 'pictureClass' ]
1960+
} );
1961+
1962+
editor.conversion.for( 'upcast' ).add( dispatcher => {
1963+
dispatcher.on( 'element:picture', ( _evt, data, conversionApi ) => {
1964+
const viewItem = data.viewItem;
1965+
const modelElement = data.modelCursor.parent;
1966+
1967+
conversionApi.writer.setAttribute( 'pictureClass', viewItem.getAttribute( 'class' ), modelElement );
1968+
} );
1969+
} );
1970+
1971+
editor.conversion.for( 'downcast' ).add( dispatcher => {
1972+
dispatcher.on( 'attribute:pictureClass:imageBlock', ( evt, data, conversionApi ) => {
1973+
const element = conversionApi.mapper.toViewElement( data.item );
1974+
const pictureElement = element.getChild( 0 );
1975+
1976+
conversionApi.writer.setAttribute( 'class', data.attributeNewValue, pictureElement );
1977+
} );
1978+
} );
1979+
1980+
editor.setData(
1981+
'<figure class="image">' +
1982+
'<picture class="test-class">' +
1983+
'<source srcset="">' +
1984+
'<img src="/assets/sample.png">' +
1985+
'</picture>' +
1986+
'<figcaption>Caption</figcaption>' +
1987+
'</figure>'
1988+
);
1989+
1990+
model.change( writer => {
1991+
writer.setAttribute(
1992+
'sources',
1993+
[
1994+
{
1995+
srcset: '/assets/sample2.png'
1996+
}
1997+
],
1998+
modelDocument.getRoot().getChild( 0 )
1999+
);
2000+
} );
2001+
2002+
expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
2003+
'<figure class="ck-widget image" contenteditable="false">' +
2004+
'<picture class="test-class">' +
2005+
'<source srcset="/assets/sample2.png"></source>' +
2006+
'<img src="/assets/sample.png"></img>' +
2007+
'</picture>' +
2008+
'<figcaption ' +
2009+
'aria-label="Caption for the image" ' +
2010+
'class="ck-editor__editable ck-editor__nested-editable" ' +
2011+
'contenteditable="true" ' +
2012+
'data-placeholder="Enter image caption" ' +
2013+
'role="textbox" ' +
2014+
'tabindex="-1">' +
2015+
'Caption' +
2016+
'</figcaption>' +
2017+
'</figure>'
2018+
);
2019+
} );
18682020
} );
18692021
} );
18702022
} );

‎packages/ckeditor5-link/src/utils/automaticdecorators.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,13 @@ export default class AutomaticDecorators {
109109
const linkInImage = Array.from( viewFigure.getChildren() )
110110
.find( ( child ): child is ViewElement => child.is( 'element', 'a' ) )!;
111111

112+
// It's not guaranteed that the anchor is present in the image block during execution of this dispatcher.
113+
// It might have been removed during the execution of unlink command that runs the image link downcast dispatcher
114+
// that is executed before this one and removes the anchor from the image block.
115+
if ( !linkInImage ) {
116+
return;
117+
}
118+
112119
for ( const item of this._definitions ) {
113120
const attributes = toMap( item.attributes );
114121

‎packages/ckeditor5-link/tests/unlinkcommand.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@
33
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
44
*/
55

6+
import { global } from '@ckeditor/ckeditor5-utils';
67
import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor.js';
78
import UnlinkCommand from '../src/unlinkcommand.js';
89
import LinkEditing from '../src/linkediting.js';
910
import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js';
1011
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils.js';
12+
import LinkImageEditing from '../src/linkimageediting.js';
13+
import Image from '@ckeditor/ckeditor5-image/src/image.js';
14+
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js';
1115

1216
describe( 'UnlinkCommand', () => {
1317
let editor, model, document, command;
@@ -508,4 +512,43 @@ describe( 'UnlinkCommand', () => {
508512
expect( getData( model ) ).to.equal( '<paragraph>[<linkableInline></linkableInline>]</paragraph>' );
509513
} );
510514
} );
515+
516+
describe( '`Image` plugin integration', () => {
517+
let editorElement;
518+
519+
beforeEach( async () => {
520+
await editor.destroy();
521+
522+
editorElement = global.document.body.appendChild(
523+
global.document.createElement( 'div' )
524+
);
525+
526+
return ClassicTestEditor.create( editorElement, {
527+
extraPlugins: [ LinkEditing, LinkImageEditing, Image ],
528+
link: {
529+
addTargetToExternalLinks: true
530+
}
531+
} )
532+
.then( newEditor => {
533+
editor = newEditor;
534+
model = editor.model;
535+
document = model.document;
536+
} );
537+
} );
538+
539+
afterEach( async () => {
540+
await editor.destroy();
541+
editorElement.remove();
542+
} );
543+
544+
it( 'should not crash during removal of external `linkHref` from `imageBlock` when `Image` plugin is present', () => {
545+
setData( model, '[<imageBlock linkHref="url"></imageBlock>]' );
546+
547+
expect( () => {
548+
editor.execute( 'unlink' );
549+
} ).not.to.throw();
550+
551+
expect( getData( model ) ).to.equal( '[<imageBlock></imageBlock>]' );
552+
} );
553+
} );
511554
} );

0 commit comments

Comments
 (0)