Merge pull request #140 from ckeditor/t/ckeditor5/718

Fix: Editor should not crash in scenarios when mutations' common ancestor could not be mapped to the model. Closes ckeditor/ckeditor5#718.

src/input.js

@@ -187,10 +187,6 @@ class MutationHandler {
  // Get common ancestor in DOM.
  const domMutationCommonAncestor = domConverter.mapViewToDom( mutationsCommonAncestor );
 
- if ( !domMutationCommonAncestor ) {
- return;
- }
-
  // Create fresh DomConverter so it will not use existing mapping and convert current DOM to model.
  // This wouldn't be needed if DomConverter would allow to create fresh view without checking any mappings.
  const freshDomConverter = new DomConverter();
@@ -201,6 +197,16 @@ class MutationHandler {
  // Current model.
  const currentModel = this.editor.editing.mapper.toModelElement( mutationsCommonAncestor );
 
+ // If common ancestor is not mapped, do not do anything. It probably is a parent of another view element.
+ // That means that we would need to diff model elements (see `if` below). Better return early instead of
+ // trying to get a reasonable model ancestor. It will fell into the `if` below anyway.
+ // This situation happens for example for lists. If `<ul>` is a common ancestor, `currentModel` is `undefined`
+ // because `<ul>` is not mapped (`<li>`s are).
+ // See https://github.com/ckeditor/ckeditor5/issues/718.
+ if ( !currentModel ) {
+ return;
+ }
+
  // Get children from both ancestors.
  const modelFromDomChildren = Array.from( modelFromCurrentDom.getChildren() );
  const currentModelChildren = Array.from( currentModel.getChildren() );

tests/input.js

@@ -3,18 +3,16 @@
  * For licensing, see LICENSE.md.
  */
 
-import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
+import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import List from '@ckeditor/ckeditor5-list/src/list';
 import Bold from '@ckeditor/ckeditor5-basic-styles/src/boldengine';
 import Input from '../src/input';
 
 import Writer from '@ckeditor/ckeditor5-engine/src/model/writer';
 import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
 
-import { downcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/downcast-converters';
-import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
-
 import ViewText from '@ckeditor/ckeditor5-engine/src/view/text';
 import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element';
 import ViewSelection from '@ckeditor/ckeditor5-engine/src/view/selection';
@@ -25,31 +23,28 @@ import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard';
 import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
 
+/* global document */
+
 describe( 'Input feature', () => {
  let editor, model, modelRoot, view, viewDocument, viewRoot, listenter;
 
  testUtils.createSinonSandbox();
 
- before( () => {
+ beforeEach( () => {
  listenter = Object.create( EmitterMixin );
 
- return VirtualTestEditor
-  .create( {
- plugins: [ Input, Paragraph, Bold ]
-  } )
+ const domElement = document.createElement( 'div' );
+ document.body.appendChild( domElement );
+
+ return ClassicTestEditor.create( domElement, { plugins: [ Input, Paragraph, Bold, List ] } )
  .then( newEditor => {
  // Mock image feature.
  newEditor.model.schema.register( 'image', { allowWhere: '$text' } );
 
- newEditor.conversion.for( 'downcast' ).add( downcastElementToElement( {
+ newEditor.conversion.elementToElement( {
  model: 'image',
  view: 'img'
- } ) );
-
- newEditor.conversion.for( 'upcast' ).add( upcastElementToElement( {
- view: 'img',
- model: 'image'
- } ) );
+ } );
 
  editor = newEditor;
  model = editor.model;
@@ -450,6 +445,30 @@ describe( 'Input feature', () => {
  expect( getModelData( model ) ).to.equal( '<paragraph>foobar baz[]</paragraph>' );
  expect( getViewData( view ) ).to.equal( '<p>foobar baz{}</p>' );
  } );
+
+ // ckeditor5#718.
+ it( 'should not crash and prevent all changes if view common ancestor of mutations cannot be mapped to model', () => {
+ editor.setData( '<p>Foo</p><ul><li>Bar</li><li>Baz</li></ul>' );
+
+ const ul = viewRoot.getChild( 1 );
+
+ viewDocument.fire( 'mutations', [
+ {
+ type: 'text',
+ oldText: 'Bar',
+ newText: 'Bx',
+ node: ul.getChild( 0 )
+ },
+ {
+ type: 'children',
+ oldChildren: [ ul.getChild( 0 ), ul.getChild( 1 ) ],
+ newChildren: [ ul.getChild( 0 ) ],
+ node: ul
+ }
+ ] );
+
+ expect( getViewData( view ) ).to.equal( '<p>{}Foo</p><ul><li>Bar</li><li>Baz</li></ul>' );
+ } );
  } );
 
  describe( 'keystroke handling', () => {