Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #97 from ckeditor/t/8
Browse files Browse the repository at this point in the history 
Fix: Bare `<img>` (not wrapped with `<figure class="image">`) can now be pasted into the editor. Closes #8.
  • Loading branch information
@Reinmar
Reinmar authored Apr 24, 2017
2 parents 5d7bef8 + 3f1db23 commit fb6ab1a
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 57 deletions.
49 changes: 21 additions & 28 deletions src/image/converters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -20,52 +19,46 @@ import modelWriter from '@ckeditor/ckeditor5-engine/src/model/writer';
*
* <image src="..." alt="..."></image>
*
* The entire contents of `<figure>` except the first `<img>` is being converted as children
* of the `<image>` 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;
};
}
Expand Down
17 changes: 15 additions & 2 deletions src/image/imageengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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() );
}
}

Expand Down
90 changes: 64 additions & 26 deletions tests/image/converters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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() ) );
Expand All @@ -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( '<figure class="image"><img src="foo.png" alt="bar baz"></img></figure>' );
expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '<image alt="bar baz" src="foo.png"></image>' );
it( 'should find img element among children and convert it using already defined converters', () => {
editor.setData( '<figure class="image"><img src="foo.png" /></figure>' );

expectModel( '<image src="foo.png"></image>' );
expect( imgConverterCalled ).to.be.true;
} );

it( 'should convert without alt', () => {
editor.setData( '<figure class="image"><img src="foo.png"></img></figure>' );
expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '<image src="foo.png"></image>' );
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( '<figure class="image">x<img src="foo.png" />y<foo></foo><bar></bar></figure>' );

// Element bar not converted because schema does not allow it.
expectModel( '<image src="foo.png">xy<foo></foo></image>' );
} );

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( '<figure class="image"><img src="foo.png" alt="bar baz"></img></figure>' );
expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '<not-image></not-image>' );
editor.setData( '<figure class="image"><img src="foo.png" />xyz</figure>' );

expectModel( '<myImage data="{"src":"foo.png"}"></myImage>' );
} );

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( '<figure><img src="foo.png" />xyz</figure>' );

editor.setData( '<figure class="image"><img src="foo.png"></img></figure>' );
expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' );
// Default image converter will be fired.
expectModel( '<image src="foo.png"></image>' );
} );

it( 'should not convert image if there is no img element', () => {
editor.setData( '<figure class="image"></figure>' );
expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' );
it( 'should not convert if there is no img element among children', () => {
editor.setData( '<figure class="image">xyz</figure>' );

// 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( '<figure class="image"><img alt="abc" />xyz</figure>' );

// Figure converter outputs nothing and text is disallowed in root.
expectModel( '' );
} );
} );

Expand Down
32 changes: 31 additions & 1 deletion tests/image/imageengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe( 'ImageEngine', () => {
} );

it( 'should not convert if there is no image class', () => {
editor.setData( '<figure><img src="foo.png" alt="alt text" /></figure>' );
editor.setData( '<figure class="quote">My quote</figure>' );

expect( getModelData( document, { withoutSelection: true } ) )
.to.equal( '' );
Expand Down Expand Up @@ -140,6 +140,36 @@ describe( 'ImageEngine', () => {

sinon.assert.calledOnce( conversionSpy );
} );

it( 'should convert bare img element', () => {
editor.setData( '<img src="foo.png" alt="alt text" />' );

expect( getModelData( document, { withoutSelection: true } ) )
.to.equal( '<image alt="alt text" src="foo.png"></image>' );
} );

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( '<div alt="foo"></div>' );

expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '<div></div>' );
} );

it( 'should handle figure with two images', () => {
document.schema.allow( { name: '$text', inside: 'image' } );

editor.setData( '<figure class="image"><img src="foo.jpg" /><img src="bar.jpg" />abc</figure>' );

expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '<image src="foo.jpg">abc</image>' );
} );
} );
} );

Expand Down

0 comments on commit fb6ab1a

Please sign in to comment.