diff --git a/src/image/converters.js b/src/image/converters.js
index 8821caba..e4897ae9 100644
--- a/src/image/converters.js
+++ b/src/image/converters.js
@@ -7,7 +7,6 @@
* @module image/image/converters
*/
-import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
import modelWriter from '@ckeditor/ckeditor5-engine/src/model/writer';
@@ -20,52 +19,46 @@ import modelWriter from '@ckeditor/ckeditor5-engine/src/model/writer';
*
*
*
+ * The entire contents of `` except the first `
` is being converted as children
+ * of the `` model element.
+ *
* @returns {Function}
*/
-export function viewToModelImage() {
+export function viewFigureToModel() {
return ( evt, data, consumable, conversionApi ) => {
- const viewFigureElement = data.input;
-
- // *** Step 1: Validate conversion.
- // Check if figure element can be consumed.
- if ( !consumable.test( viewFigureElement, { name: true, class: 'image' } ) ) {
+ // Do not convert if this is not an "image figure".
+ if ( !consumable.test( data.input, { name: true, class: 'image' } ) ) {
return;
}
- // Check if image element can be converted in current context.
+ // Do not convert if image cannot be placed in model at this context.
if ( !conversionApi.schema.check( { name: 'image', inside: data.context, attributes: 'src' } ) ) {
return;
}
- // Check if img element is placed inside figure element and can be consumed with `src` attribute.
- const viewImg = viewFigureElement.getChild( 0 );
+ // Find an image element inside the figure element.
+ const viewImage = Array.from( data.input.getChildren() ).find( viewChild => viewChild.is( 'img' ) );
- if ( !viewImg || viewImg.name != 'img' || !consumable.test( viewImg, { name: true, attribute: 'src' } ) ) {
+ // Do not convert if image element is absent, is missing src attribute or was already converted.
+ if ( !viewImage || !viewImage.hasAttribute( 'src' ) || !consumable.test( viewImage, { name: true } ) ) {
return;
}
- // *** Step2: Convert to model.
- consumable.consume( viewFigureElement, { name: true, class: 'image' } );
- consumable.consume( viewImg, { name: true, attribute: 'src' } );
+ // Convert view image to model image.
+ const modelImage = conversionApi.convertItem( viewImage, consumable, data );
- // Create model element.
- const modelImage = new ModelElement( 'image', {
- src: viewImg.getAttribute( 'src' )
- } );
+ // Convert rest of figure element's children, but in the context of model image, because those converted
+ // children will be added as model image children.
+ data.context.push( modelImage );
- // Convert `alt` attribute if present.
- if ( consumable.consume( viewImg, { attribute: [ 'alt' ] } ) ) {
- modelImage.setAttribute( 'alt', viewImg.getAttribute( 'alt' ) );
- }
+ const modelChildren = conversionApi.convertChildren( data.input, consumable, data );
- // Convert children of converted view element and append them to `modelImage`.
- // TODO https://github.com/ckeditor/ckeditor5-engine/issues/736.
- data.context.push( modelImage );
- const modelChildren = conversionApi.convertChildren( viewFigureElement, consumable, data );
- const insertPosition = ModelPosition.createAt( modelImage, 'end' );
- modelWriter.insert( insertPosition, modelChildren );
data.context.pop();
+ // Add converted children to model image.
+ modelWriter.insert( ModelPosition.createAt( modelImage ), modelChildren );
+
+ // Set model image as conversion result.
data.output = modelImage;
};
}
diff --git a/src/image/imageengine.js b/src/image/imageengine.js
index c88b0a01..8654d38f 100644
--- a/src/image/imageengine.js
+++ b/src/image/imageengine.js
@@ -9,8 +9,10 @@
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
-import { viewToModelImage, createImageAttributeConverter } from './converters';
+import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
+import { viewFigureToModel, createImageAttributeConverter } from './converters';
import { toImageWidget } from './utils';
+import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import ViewContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
import ViewEmptyElement from '@ckeditor/ckeditor5-engine/src/view/emptyelement';
@@ -52,8 +54,19 @@ export default class ImageEngine extends Plugin {
createImageAttributeConverter( [ editing.modelToView, data.modelToView ], 'src' );
createImageAttributeConverter( [ editing.modelToView, data.modelToView ], 'alt' );
+ // Build converter for view img element to model image element.
+ buildViewConverter().for( data.viewToModel )
+ .from( { name: 'img', attribute: { src: /./ } } )
+ .toElement( ( viewImage ) => new ModelElement( 'image', { src: viewImage.getAttribute( 'src' ) } ) );
+
+ // Build converter for alt attribute.
+ buildViewConverter().for( data.viewToModel )
+ .from( { name: 'img', attribute: { alt: /./ } } )
+ .consuming( { attribute: [ 'alt' ] } )
+ .toAttribute( ( viewImage ) => ( { key: 'alt', value: viewImage.getAttribute( 'alt' ) } ) );
+
// Converter for figure element from view to model.
- data.viewToModel.on( 'element:figure', viewToModelImage() );
+ data.viewToModel.on( 'element:figure', viewFigureToModel() );
}
}
diff --git a/tests/image/converters.js b/tests/image/converters.js
index 89e27542..e88f5507 100644
--- a/tests/image/converters.js
+++ b/tests/image/converters.js
@@ -3,11 +3,12 @@
* For licensing, see LICENSE.md.
*/
-import { viewToModelImage, createImageAttributeConverter } from '../../src/image/converters';
+import { viewFigureToModel, createImageAttributeConverter } from '../../src/image/converters';
import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
import { createImageViewElement } from '../../src/image/imageengine';
import { toImageWidget } from '../../src/image/utils';
import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
+import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
@@ -28,10 +29,6 @@ describe( 'Image converters', () => {
schema.allow( { name: 'image', attributes: [ 'alt', 'src' ], inside: '$root' } );
schema.objects.add( 'image' );
- buildModelConverter().for( )
- .fromElement( 'image' )
- .toElement( () => toImageWidget( createImageViewElement() ) );
-
buildModelConverter().for( editor.editing.modelToView )
.fromElement( 'image' )
.toElement( () => toImageWidget( createImageViewElement() ) );
@@ -41,46 +38,87 @@ describe( 'Image converters', () => {
} );
} );
- describe( 'viewToModelImage', () => {
- let dispatcher, schema;
+ describe( 'viewFigureToModel', () => {
+ function expectModel( model ) {
+ expect( getModelData( document, { withoutSelection: true } ) ).to.equal( model );
+ }
+
+ let dispatcher, schema, imgConverterCalled;
beforeEach( () => {
+ imgConverterCalled = false;
+
schema = document.schema;
+ schema.allow( { name: '$text', inside: 'image' } );
+
dispatcher = editor.data.viewToModel;
- dispatcher.on( 'element:figure', viewToModelImage() );
+ dispatcher.on( 'element:figure', viewFigureToModel() );
+ dispatcher.on( 'element:img', ( evt, data, consumable ) => {
+ if ( consumable.consume( data.input, { name: true, attribute: 'src' } ) ) {
+ data.output = new ModelElement( 'image', { src: data.input.getAttribute( 'src' ) } );
+
+ imgConverterCalled = true;
+ }
+ } );
} );
- it( 'should convert view figure element', () => {
- editor.setData( '
' );
- expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' );
+ it( 'should find img element among children and convert it using already defined converters', () => {
+ editor.setData( '
' );
+
+ expectModel( '' );
+ expect( imgConverterCalled ).to.be.true;
} );
- it( 'should convert without alt', () => {
- editor.setData( '
' );
- expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' );
+ it( 'should convert non-img children in image context and append them to model image element', () => {
+ buildViewConverter().for( editor.data.viewToModel ).fromElement( 'foo' ).toElement( 'foo' );
+ buildViewConverter().for( editor.data.viewToModel ).fromElement( 'bar' ).toElement( 'bar' );
+
+ schema.registerItem( 'foo' );
+ schema.registerItem( 'bar' );
+
+ schema.allow( { name: 'foo', inside: 'image' } );
+
+ editor.setData( 'x
y' );
+
+ // Element bar not converted because schema does not allow it.
+ expectModel( 'xy' );
} );
- it( 'should not convert if figure element is already consumed', () => {
+ it( 'should be possible to overwrite', () => {
dispatcher.on( 'element:figure', ( evt, data, consumable ) => {
- consumable.consume( data.input, { name: true, class: 'image' } );
+ consumable.consume( data.input, { name: true } );
+ consumable.consume( data.input.getChild( 0 ), { name: true } );
- data.output = new ModelElement( 'not-image' );
+ data.output = new ModelElement( 'myImage', { data: { src: data.input.getChild( 0 ).getAttribute( 'src' ) } } );
}, { priority: 'high' } );
- editor.setData( '
' );
- expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' );
+ editor.setData( '
xyz' );
+
+ expectModel( '' );
} );
- it( 'should not convert image if schema disallows it', () => {
- schema.disallow( { name: 'image', attributes: [ 'alt', 'src' ], inside: '$root' } );
+ // Test exactly what figure converter does, which is putting it's children element to image element.
+ // If this has not been done, it means that figure converter was not used.
+ it( 'should not convert if figure do not have class="image" attribute', () => {
+ editor.setData( '
xyz' );
- editor.setData( '
' );
- expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' );
+ // Default image converter will be fired.
+ expectModel( '' );
} );
- it( 'should not convert image if there is no img element', () => {
- editor.setData( '' );
- expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' );
+ it( 'should not convert if there is no img element among children', () => {
+ editor.setData( 'xyz' );
+
+ // Figure converter outputs nothing and text is disallowed in root.
+ expectModel( '' );
+ } );
+
+ it( 'should not convert if img element was not converted', () => {
+ // Image element missing src attribute.
+ editor.setData( '
xyz' );
+
+ // Figure converter outputs nothing and text is disallowed in root.
+ expectModel( '' );
} );
} );
diff --git a/tests/image/imageengine.js b/tests/image/imageengine.js
index 745b8fa6..adf38f8f 100644
--- a/tests/image/imageengine.js
+++ b/tests/image/imageengine.js
@@ -58,7 +58,7 @@ describe( 'ImageEngine', () => {
} );
it( 'should not convert if there is no image class', () => {
- editor.setData( '
' );
+ editor.setData( 'My quote' );
expect( getModelData( document, { withoutSelection: true } ) )
.to.equal( '' );
@@ -140,6 +140,36 @@ describe( 'ImageEngine', () => {
sinon.assert.calledOnce( conversionSpy );
} );
+
+ it( 'should convert bare img element', () => {
+ editor.setData( '
' );
+
+ expect( getModelData( document, { withoutSelection: true } ) )
+ .to.equal( '' );
+ } );
+
+ it( 'should not convert alt attribute on non-img element', () => {
+ const data = editor.data;
+ const editing = editor.editing;
+
+ document.schema.registerItem( 'div', '$block' );
+ document.schema.allow( { name: 'div', attributes: 'alt', inside: '$root' } );
+
+ buildModelConverter().for( data.modelToView, editing.modelToView ).fromElement( 'div' ).toElement( 'div' );
+ buildViewConverter().for( data.viewToModel ).fromElement( 'div' ).toElement( 'div' );
+
+ editor.setData( '' );
+
+ expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' );
+ } );
+
+ it( 'should handle figure with two images', () => {
+ document.schema.allow( { name: '$text', inside: 'image' } );
+
+ editor.setData( '![](foo.jpg)
abc' );
+
+ expect( getModelData( document, { withoutSelection: true } ) ).to.equal( 'abc' );
+ } );
} );
} );