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();