Merge pull request #198 from ckeditor/t/167

Feature: Improved typing on Android devices by handling `beforeinpnut` event. Introduced `options.selection` in `DeleteCommand` params. Introduced `selectionToRemove` parameter in `view.Document#event:delete` data. Closes #167.

src/delete.js

@@ -10,8 +10,7 @@
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import DeleteCommand from './deletecommand';
 import DeleteObserver from './deleteobserver';
-
-import injectAndroidBackspaceMutationsHandling from './utils/injectandroidbackspacemutationshandling';
+import env from '@ckeditor/ckeditor5-utils/src/env';
 
 /**
  * The delete and backspace feature. Handles the <kbd>Delete</kbd> and <kbd>Backspace</kbd> keys in the editor.
@@ -37,11 +36,62 @@ export default class Delete extends Plugin {
 		editor.commands.add( 'delete', new DeleteCommand( editor, 'backward' ) );
 
 		this.listenTo( viewDocument, 'delete', ( evt, data ) => {
-			editor.execute( data.direction == 'forward' ? 'forwardDelete' : 'delete', { unit: data.unit, sequence: data.sequence } );
+			const deleteCommandParams = { unit: data.unit, sequence: data.sequence };
+
+			// If a specific (view) selection to remove was set, convert it to a model selection and set as a parameter for `DeleteCommand`.
+			if ( data.selectionToRemove ) {
+				const modelSelection = editor.model.createSelection();
+				const ranges = [];
+
+				for ( const viewRange of data.selectionToRemove.getRanges() ) {
+					ranges.push( editor.editing.mapper.toModelRange( viewRange ) );
+				}
+
+				modelSelection.setTo( ranges );
+
+				deleteCommandParams.selection = modelSelection;
+			}
+
+			editor.execute( data.direction == 'forward' ? 'forwardDelete' : 'delete', deleteCommandParams );
+
 			data.preventDefault();
+
 			view.scrollToTheSelection();
 		} );
 
-		injectAndroidBackspaceMutationsHandling( editor );
+		// Android IMEs have a quirk - they change DOM selection after the input changes were performed by the browser.
+		// This happens on `keyup` event. Android doesn't know anything about our deletion and selection handling. Even if the selection
+		// was changed during input events, IME remembers the position where the selection "should" be placed and moves it there.
+		//
+		// To prevent incorrect selection, we save the selection after deleting here and then re-set it on `keyup`. This has to be done
+		// on DOM selection level, because on `keyup` the model selection is still the same as it was just after deletion, so it
+		// wouldn't be changed and the fix would do nothing.
+		//
+		/* istanbul ignore if */
+		if ( env.isAndroid ) {
+			let domSelectionAfterDeletion = null;
+
+			this.listenTo( viewDocument, 'delete', ( evt, data ) => {
+				const domSelection = data.domTarget.ownerDocument.defaultView.getSelection();
+
+				domSelectionAfterDeletion = {
+					anchorNode: domSelection.anchorNode,
+					anchorOffset: domSelection.anchorOffset,
+					focusNode: domSelection.focusNode,
+					focusOffset: domSelection.focusOffset
+				};
+			}, { priority: 'lowest' } );
+
+			this.listenTo( viewDocument, 'keyup', ( evt, data ) => {
+				if ( domSelectionAfterDeletion ) {
+					const domSelection = data.domTarget.ownerDocument.defaultView.getSelection();
+
+					domSelection.collapse( domSelectionAfterDeletion.anchorNode, domSelectionAfterDeletion.anchorOffset );
+					domSelection.extend( domSelectionAfterDeletion.focusNode, domSelectionAfterDeletion.focusOffset );
+
+					domSelectionAfterDeletion = null;
+				}
+			} );
+		}
 	}
 }

src/deletecommand.js

@@ -66,6 +66,8 @@ export default class DeleteCommand extends Command {
 	 * @param {'character'} [options.unit='character'] See {@link module:engine/model/utils/modifyselection~modifySelection}'s options.
 	 * @param {Number} [options.sequence=1] A number describing which subsequent delete event it is without the key being released.
 	 * See the {@link module:engine/view/document~Document#event:delete} event data.
+	 * @param {module:engine/model/selection~Selection} [options.selection] Selection to remove. If not set, current model selection
+	 * will be used.
 	 */
 	execute( options = {} ) {
 		const model = this.editor.model;
@@ -74,7 +76,7 @@ export default class DeleteCommand extends Command {
 		model.enqueueChange( this._buffer.batch, writer => {
 			this._buffer.lock();
 
-			const selection = writer.createSelection( doc.selection );
+			const selection = writer.createSelection( options.selection || doc.selection );
 
 			// Do not replace the whole selected content if selection was collapsed.
 			// This prevents such situation:

src/deleteobserver.js

@@ -47,19 +47,52 @@ export default class DeleteObserver extends Observer {
 			deleteData.unit = hasWordModifier ? 'word' : deleteData.unit;
 			deleteData.sequence = ++sequence;
 
+			fireViewDeleteEvent( evt, data.domEvent, deleteData );
+		} );
+
+		// `beforeinput` is handled only for Android devices. Desktop Chrome and iOS are skipped because they are working fine now.
+		/* istanbul ignore if */
+		if ( env.isAndroid ) {
+			document.on( 'beforeinput', ( evt, data ) => {
+				// If event type is other than `deleteContentBackward` then this is not deleting.
+				if ( data.domEvent.inputType != 'deleteContentBackward' ) {
+					return;
+				}
+
+				const deleteData = {
+					unit: 'codepoint',
+					direction: 'backward',
+					sequence: 1
+				};
+
+				// Android IMEs may change the DOM selection on `beforeinput` event so that the selection contains all the text
+				// that the IME wants to remove. We will pass this information to `delete` event so proper part of the content is removed.
+				//
+				// Sometimes it is only expanding by a one character (in case of collapsed selection). In this case we don't need to
+				// set a different selection to remove, it will work just fine.
+				const domSelection = data.domTarget.ownerDocument.defaultView.getSelection();
+
+				if ( domSelection.anchorNode == domSelection.focusNode && domSelection.anchorOffset + 1 != domSelection.focusOffset ) {
+					deleteData.selectionToRemove = view.domConverter.domSelectionToView( domSelection );
+				}
+
+				fireViewDeleteEvent( evt, data.domEvent, deleteData );
+			} );
+		}
+
+		function fireViewDeleteEvent( originalEvent, domEvent, deleteData ) {
 			// Save the event object to check later if it was stopped or not.
 			let event;
 			document.once( 'delete', evt => ( event = evt ), { priority: Number.POSITIVE_INFINITY } );
 
-			const domEvtData = new DomEventData( document, data.domEvent, deleteData );
-			document.fire( 'delete', domEvtData );
+			document.fire( 'delete', new DomEventData( document, domEvent, deleteData ) );
 
-			// Stop `keydown` event if `delete` event was stopped.
+			// Stop the original event if `delete` event was stopped.
 			// https://github.com/ckeditor/ckeditor5/issues/753
 			if ( event && event.stop.called ) {
-				evt.stop();
+				originalEvent.stop();
 			}
-		} );
+		}
 	}
 
 	/**
@@ -80,4 +113,6 @@ export default class DeleteObserver extends Observer {
  * @param {'character'|'word'} data.unit The "amount" of content that should be deleted.
  * @param {Number} data.sequence A number describing which subsequent delete event it is without the key being released.
  * If it's 2 or more it means that the key was pressed and hold.
+ * @param {module:engine/view/selection~Selection} [data.selectionToRemove] View selection which content should be removed. If not set,
+ * current selection should be used.
  */

src/utils/injectunsafekeystrokeshandling.js

@@ -8,6 +8,7 @@
  */
 
 import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard';
+import env from '@ckeditor/ckeditor5-utils/src/env';
 
 /**
  * Handles keystrokes which are unsafe for typing. This handler's logic is explained
@@ -22,7 +23,13 @@ export default function injectUnsafeKeystrokesHandling( editor ) {
 	const view = editor.editing.view;
 	const inputCommand = editor.commands.get( 'input' );
 
-	view.document.on( 'keydown', ( evt, evtData ) => handleKeydown( evtData ), { priority: 'lowest' } );
+	// For Android, we want to handle keystrokes on `beforeinput` to be sure that code in `DeleteObserver` already had a chance to be fired.
+	/* istanbul ignore if */
+	if ( env.isAndroid ) {
+		view.document.on( 'beforeinput', ( evt, evtData ) => handleUnsafeKeystroke( evtData ), { priority: 'lowest' } );
+	} else {
+		view.document.on( 'keydown', ( evt, evtData ) => handleUnsafeKeystroke( evtData ), { priority: 'lowest' } );
+	}
 
 	view.document.on( 'compositionstart', handleCompositionStart, { priority: 'lowest' } );
 
@@ -42,7 +49,7 @@ export default function injectUnsafeKeystrokesHandling( editor ) {
 	// to handle the event.
 	//
 	// @param {module:engine/view/observer/keyobserver~KeyEventData} evtData
-	function handleKeydown( evtData ) {
+	function handleUnsafeKeystroke( evtData ) {
 		const doc = model.document;
 		const isComposing = view.document.isComposing;
 		const isSelectionUnchanged = latestCompositionSelection && latestCompositionSelection.isEqual( doc.selection );

tests/delete.js

@@ -60,6 +60,33 @@ describe( 'Delete feature', () => {
 		expect( spy.calledWithMatch( 'delete', { unit: 'character', sequence: 5 } ) ).to.be.true;
 	} );
 
+	it( 'passes options.selection parameter to delete command if selection to remove was specified', () => {
+		editor.setData( '<p>Foobar</p>' );
+
+		const spy = editor.execute = sinon.spy();
+		const view = editor.editing.view;
+		const viewDocument = view.document;
+		const domEvt = getDomEvent();
+
+		const viewSelection = view.createSelection( view.createRangeIn( viewDocument.getRoot() ) );
+
+		viewDocument.fire( 'delete', new DomEventData( viewDocument, domEvt, {
+			direction: 'backward',
+			unit: 'character',
+			sequence: 1,
+			selectionToRemove: viewSelection
+		} ) );
+
+		expect( spy.calledOnce ).to.be.true;
+
+		const commandName = spy.args[ 0 ][ 0 ];
+		const options = spy.args[ 0 ][ 1 ];
+		const expectedSelection = editor.model.createSelection( editor.model.createRangeIn( editor.model.document.getRoot() ) );
+
+		expect( commandName ).to.equal( 'delete' );
+		expect( options.selection.isEqual( expectedSelection ) ).to.be.true;
+	} );
+
 	it( 'scrolls the editing document to the selection after executing the command', () => {
 		const scrollSpy = sinon.stub( editor.editing.view, 'scrollToTheSelection' );
 		const executeSpy = editor.execute = sinon.spy();