Merge pull request #107 from ckeditor/t/98

Feature: Introduced support for pasting and loading images in context in which they cannot appear in the editor. For example, if an `<p>foo<img>bar</p>` is pasted, the pasted paragraph will be split (because an image in the editor cannot be contained in a paragraph). Closes #98.

src/image/converters.js

@@ -8,6 +8,7 @@
  */
 
 import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
+import ModelDocumentFragment from '@ckeditor/ckeditor5-engine/src/model/documentfragment';
 import modelWriter from '@ckeditor/ckeditor5-engine/src/model/writer';
 
 /**
@@ -97,3 +98,173 @@ function modelToViewAttributeConverter( evt, data, consumable, conversionApi ) {
 		img.setAttribute( data.attributeKey, data.attributeNewValue );
 	}
 }
+
+// Holds all images that were converted for autohoisting.
+const autohoistedImages = new WeakSet();
+
+/**
+ * Converter which converts `<img>` {@link module:engine/view/element~Element view elements} that can be hoisted.
+ *
+ * If an `<img>` view element has not been converted, this converter checks if that element could be converted in any
+ * context "above". If it could, the converter converts the `<img>` element even though it is not allowed in current
+ * context and marks it to be autohoisted. Then {@link module:image/image/converters~hoistImageThroughElement another converter}
+ * moves the converted element to the correct location.
+ */
+export function convertHoistableImage( evt, data, consumable, conversionApi ) {
+	const img = data.input;
+
+	// If the image has not been consumed (converted)...
+	if ( !consumable.test( img, { name: true, attribute: [ 'src' ] } ) ) {
+		return;
+	}
+	// At this point the image has not been converted because it was not allowed by schema. It might be in wrong
+	// context or missing an attribute, but above we already checked whether the image has mandatory src attribute.
+
+	// If the image would be allowed if it was in one of its ancestors...
+	const allowedContext = _findAllowedContext( { name: 'image', attributes: [ 'src' ] }, data.context, conversionApi.schema );
+
+	if ( !allowedContext ) {
+		return;
+	}
+
+	// Convert it in that context...
+	const newData = Object.assign( {}, data );
+	newData.context = allowedContext;
+
+	data.output = conversionApi.convertItem( img, consumable, newData );
+
+	// And mark that image to be hoisted.
+	autohoistedImages.add( data.output );
+}
+
+// Basing on passed `context`, searches for "closest" context in which model element represented by `modelData`
+// would be allowed by `schema`.
+//
+// @private
+// @param {Object} modelData Object describing model element to check. Has two properties: `name` with model element name
+// and `attributes` with keys of attributes of that model element.
+// @param {Array} context Context in which original conversion was supposed to take place.
+// @param {module:engine/model/schema~Schema} schema Schema to check with.
+// @returns {Array|null} Context in which described model element would be allowed by `schema` or `null` if such context
+// could not been found.
+function _findAllowedContext( modelData, context, schema ) {
+	// Copy context array so we won't modify original array.
+	context = context.slice();
+
+	// Prepare schema query to check with schema.
+	// Since `inside` property is passed as reference to `context` variable, we don't need to modify `schemaQuery`.
+	const schemaQuery = {
+		name: modelData.name,
+		attributes: modelData.attributes,
+		inside: context
+	};
+
+	// Try out all possible contexts.
+	while ( context.length && !schema.check( schemaQuery ) ) {
+		const parent = context.pop();
+		const parentName = typeof parent === 'string' ? parent : parent.name;
+
+		// Do not try to autohoist "above" limiting element.
+		if ( schema.limits.has( parentName ) ) {
+			return null;
+		}
+	}
+
+	// If `context` has any items it means that image is allowed in that context. Return that context.
+	// If `context` has no items it means that image was not allowed in any of possible contexts. Return `null`.
+	return context.length ? context : null;
+}
+
+/**
+ * Converter which hoists `image` {@link module:engine/model/element~Element model elements} to allowed context.
+ *
+ * Looks through all children of converted {@link module:engine/view/element~Element view element} if it
+ * has been converted to model element. Breaks model element if `image` to-be-hoisted is found.
+ *
+ *		<div><paragraph>x<image src="foo.jpg"></image>x</paragraph></div> ->
+ *		<div><paragraph>x</paragraph></div><image src="foo.jpg"></image><div><paragraph>x</paragraph></div>
+ *
+ * This works deeply, as shown in the example. This converter added for `paragraph` element will break `paragraph` element and
+ * pass {@link module:engine/model/documentfragment~DocumentFragment document fragment} in `data.output`. Then,
+ * `div` will be handled by this converter and will be once again broken to hoist `image` up to the root.
+ *
+ * **Note:** This converter should be executed only after the view element has been already converted, meaning that
+ * `data.output` for that view element should be already generated when this converter is fired.
+ */
+export function hoistImageThroughElement( evt, data ) {
+	// If this element has been properly converted...
+	if ( !data.output ) {
+		return;
+	}
+
+	// And it is an element...
+	// (If it is document fragment autohoisting does not have to break anything anyway.)
+	// (And if it is text there are no children here.)
+	if ( !data.output.is( 'element' ) ) {
+		return;
+	}
+
+	// This will hold newly generated output. At the beginning it is only the original element.
+	const newOutput = [];
+
+	// Check if any of its children is to be hoisted...
+	// Start from the last child - it is easier to break that way.
+	for ( let i = data.output.childCount - 1; i >= 0; i-- ) {
+		const child = data.output.getChild( i );
+
+		if ( autohoistedImages.has( child ) ) {
+			// Break autohoisted element's parent:
+			// <parent>{ left-children... }<authoistedElement />{ right-children... }</parent>   --->
+			// <parent>{ left-children... }</parent><autohoistedElement /><parent>{ right-children... }</parent>
+			//
+			// or
+			//
+			// <parent>{ left-children... }<autohoistedElement /></parent> --->
+			// <parent>{ left-children... }</parent><autohoistedElement />
+			//
+			// or
+			//
+			// <parent><autohoistedElement />{ right-children... }</parent> --->
+			// <autohoistedElement /><parent>{ right-children... }</parent>
+			//
+			// or
+			//
+			// <parent><autohoistedElement /></parent> ---> <autohoistedElement />
+
+			// Check how many right-children there are.
+			const rightChildrenCount = data.output.childCount - i - 1;
+			let rightParent = null;
+
+			// If there are any right-children, clone the prent element and insert those children there.
+			if ( rightChildrenCount > 0 ) {
+				rightParent = data.output.clone( false );
+				rightParent.appendChildren( data.output.removeChildren( i + 1, rightChildrenCount ) );
+			}
+
+			// Remove the autohoisted element from its parent.
+			child.remove();
+
+			// Break "leading" `data.output` in `newOutput` into one or more pieces:
+			// Remove "leading" `data.output` (note that `data.output` is always first item in `newOutput`).
+			newOutput.shift();
+
+			// Add the newly created parent of the right-children at the beginning.
+			if ( rightParent ) {
+				newOutput.unshift( rightParent );
+			}
+
+			// Add autohoisted element at the beginning.
+			newOutput.unshift( child );
+
+			// Add `data.output` at the beginning, if there is anything left in it.
+			if ( data.output.childCount > 0 ) {
+				newOutput.unshift( data.output );
+			}
+		}
+	}
+
+	// If the output has changed pass it further.
+	if ( newOutput.length ) {
+		data.output = new ModelDocumentFragment( newOutput );
+	}
+}

src/image/imageengine.js

@@ -10,7 +10,7 @@
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
 import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
-import { viewFigureToModel, createImageAttributeConverter } from './converters';
+import { viewFigureToModel, createImageAttributeConverter, convertHoistableImage, hoistImageThroughElement } from './converters';
 import { toImageWidget } from './utils';
 import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
 import ViewContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
@@ -59,7 +59,12 @@ export default class ImageEngine extends Plugin {
 			.from( { name: 'img', attribute: { src: /./ } } )
 			.toElement( ( viewImage ) => new ModelElement( 'image', { src: viewImage.getAttribute( 'src' ) } ) );
 
+		data.viewToModel.on( 'element:img', convertHoistableImage, { priority: 'low' } );
+		data.viewToModel.on( 'element', hoistImageThroughElement, { priority: 'low' } );
+
 		// Build converter for alt attribute.
+		// Note that by default attribute converters are added with `low` priority.
+		// This converter will be thus fired after `convertHoistableImage` converter.
 		buildViewConverter().for( data.viewToModel )
 			.from( { name: 'img', attribute: { alt: /./ } } )
 			.consuming( { attribute: [ 'alt' ] } )

tests/image/converters.js

@@ -3,13 +3,21 @@
  * For licensing, see LICENSE.md.
  */
 
-import { viewFigureToModel, createImageAttributeConverter } from '../../src/image/converters';
+import {
+	viewFigureToModel,
+	createImageAttributeConverter,
+	convertHoistableImage,
+	hoistImageThroughElement
+} 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 ViewEmptyElement from '@ckeditor/ckeditor5-engine/src/view/emptyelement';
+import ViewContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
+import { convertToModelFragment } from '@ckeditor/ckeditor5-engine/src/conversion/view-to-model-converters';
 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';
 
@@ -187,4 +195,138 @@ describe( 'Image converters', () => {
 			);
 		} );
 	} );
+
+	// More integration tests are in imageengine.js.
+	describe( 'autohoist converters', () => {
+		let dispatcher, schema, imgConverterCalled, viewImg, modelDiv, modelParagraph, modelLimit;
+
+		beforeEach( () => {
+			imgConverterCalled = false;
+
+			schema = document.schema;
+
+			dispatcher = editor.data.viewToModel;
+			dispatcher.on( 'element:img', ( evt, data, consumable ) => {
+				if ( !schema.check( { name: 'image', attributes: [ 'src' ], inside: data.context } ) ) {
+					return;
+				}
+
+				if ( consumable.consume( data.input, { name: true, attribute: 'src' } ) ) {
+					data.output = new ModelElement( 'image', { src: data.input.getAttribute( 'src' ) } );
+
+					imgConverterCalled = true;
+				}
+			} );
+
+			// Make sure to pass document element to the converter and fire this callback after "normal" image callback.
+			dispatcher.on( 'element:img', convertHoistableImage, { priority: 'low' } );
+
+			viewImg = new ViewEmptyElement( 'img', { src: 'foo.jpg' } );
+
+			modelDiv = new ModelElement( 'div' );
+			modelParagraph = new ModelElement( 'paragraph' );
+			modelLimit = new ModelElement( 'limit' );
+		} );
+
+		describe( 'convertHoistableImage', () => {
+			it( 'should convert img element using already added converters if it is allowed in any point of given context #1', () => {
+				const result = dispatcher.convert( viewImg, { context: [ '$root', 'div', 'paragraph' ] } );
+
+				// `result` is a model document fragment.
+				expect( result.childCount ).to.equal( 1 );
+				expect( result.getChild( 0 ).is( 'image' ) ).to.be.true;
+				expect( result.getChild( 0 ).getAttribute( 'src' ) ).to.equal( 'foo.jpg' );
+				expect( imgConverterCalled ).to.be.true;
+			} );
+
+			it( 'should convert img element using already added converters if it is allowed in any point of given context #2', () => {
+				const result = dispatcher.convert( viewImg, { context: [ '$root', modelDiv, modelParagraph ] } );
+
+				// `result` is a model document fragment.
+				expect( result.childCount ).to.equal( 1 );
+				expect( result.getChild( 0 ).is( 'image' ) ).to.be.true;
+				expect( result.getChild( 0 ).getAttribute( 'src' ) ).to.equal( 'foo.jpg' );
+				expect( imgConverterCalled ).to.be.true;
+			} );
+
+			it( 'should not convert img element if there is no allowed context #1', () => {
+				const result = dispatcher.convert( viewImg, { context: [ 'div', 'paragraph' ] } );
+
+				// `result` is an empty model document fragment.
+				expect( result.childCount ).to.equal( 0 );
+				expect( imgConverterCalled ).to.be.false;
+			} );
+
+			it( 'should not convert img element if there is no allowed context #2', () => {
+				const result = dispatcher.convert( viewImg, { context: [ modelDiv, modelParagraph ] } );
+
+				// `result` is an empty model document fragment.
+				expect( result.childCount ).to.equal( 0 );
+				expect( imgConverterCalled ).to.be.false;
+			} );
+
+			it( 'should not convert img element if allowed context is "above" limiting element #1', () => {
+				schema.limits.add( 'limit' );
+
+				const result = dispatcher.convert( viewImg, { context: [ '$root', 'limit', 'div', 'paragraph' ] } );
+
+				// `result` is an empty model document fragment.
+				expect( result.childCount ).to.equal( 0 );
+				expect( imgConverterCalled ).to.be.false;
+			} );
+
+			it( 'should not convert img element if allowed context is "above" limiting element #2', () => {
+				schema.limits.add( 'limit' );
+
+				const result = dispatcher.convert( viewImg, { context: [ '$root', modelLimit, modelDiv, modelParagraph ] } );
+
+				// `result` is an empty model document fragment.
+				expect( result.childCount ).to.equal( 0 );
+				expect( imgConverterCalled ).to.be.false;
+			} );
+		} );
+
+		describe( 'hoistImageThroughElement', () => {
+			it( 'should hoist img element that was converted by convertHoistableImage', () => {
+				schema.registerItem( 'div', '$block' );
+
+				buildModelConverter().for( editor.data.modelToView, editor.editing.modelToView ).fromElement( 'div' ).toElement( 'div' );
+				buildViewConverter().for( editor.data.viewToModel ).fromElement( 'div' ).toElement( 'div' );
+
+				// Make sure to fire this callback after "normal" div callback.
+				dispatcher.on( 'element:div', hoistImageThroughElement, { priority: 'low' } );
+
+				// If img view element is converted it must have been converted thanks to convertHoistableImage,
+				// because image is not allowed in div.
+				const viewDiv = new ViewContainerElement( 'div', null, [ 'foo', viewImg, 'bar' ] );
+
+				const result = dispatcher.convert( viewDiv, { context: [ '$root' ] } );
+
+				// `result` is a model document fragment.
+				expect( result.childCount ).to.equal( 3 );
+				expect( result.getChild( 0 ).is( 'div' ) ).to.be.true;
+				expect( result.getChild( 1 ).is( 'image' ) ).to.be.true;
+				expect( result.getChild( 2 ).is( 'div' ) ).to.be.true;
+			} );
+
+			it( 'should work fine with elements that are converted to document fragments', () => {
+				// The example here is <p> that contains some text and an <img> and is converted in $root > div context.
+				// This way we enable autohoisting for <img> and test how it will work when <p> is converted to model document fragment.
+				schema.registerItem( 'div', '$block' );
+
+				dispatcher.on( 'element:p', convertToModelFragment() );
+				dispatcher.on( 'element:p', hoistImageThroughElement, { priority: 'low' } );
+
+				const viewDiv = new ViewContainerElement( 'p', null, [ 'foo', viewImg, 'bar' ] );
+
+				const result = dispatcher.convert( viewDiv, { context: [ '$root', 'div' ] } );
+
+				// `result` is a model document fragment.
+				expect( result.childCount ).to.equal( 3 );
+				expect( result.getChild( 0 ).is( 'text' ) ).to.be.true;
+				expect( result.getChild( 1 ).is( 'image' ) ).to.be.true;
+				expect( result.getChild( 2 ).is( 'text' ) ).to.be.true;
+			} );
+		} );
+	} );
 } );