Przeglądaj źródła

Merge branch 'master' into t/ckeditor5/488

# Conflicts:
#	tests/input.js
Szymon Cofalik 7 lat temu
rodzic
commit
18c3883e82

+ 10 - 4
packages/ckeditor5-typing/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() );

+ 36 - 17
packages/ckeditor5-typing/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 BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting';
+import List from '@ckeditor/ckeditor5-list/src/list';
+import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
 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, BoldEditing ]
-			} )
+		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', () => {