From 8b2bb126c14094e1905b359da159b5ca112ad55d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Wed, 26 Oct 2016 12:45:59 +0200 Subject: [PATCH 1/3] Added Input Command, updated tests. --- src/input.js | 305 +-------------------------------------- src/inputcommand.js | 339 ++++++++++++++++++++++++++++++++++++++++++++ tests/input.js | 30 ---- 3 files changed, 345 insertions(+), 329 deletions(-) create mode 100644 src/inputcommand.js diff --git a/src/input.js b/src/input.js index e4b6dca..e28f16c 100644 --- a/src/input.js +++ b/src/input.js @@ -4,13 +4,7 @@ */ import Feature from '../core/feature.js'; -import ChangeBuffer from './changebuffer.js'; -import ModelRange from '../engine/model/range.js'; -import ViewPosition from '../engine/view/position.js'; -import ViewText from '../engine/view/text.js'; -import diff from '../utils/diff.js'; -import diffToChanges from '../utils/difftochanges.js'; -import { getCode } from '../utils/keyboard.js'; +import InputCommand from './inputcommand.js'; /** * Handles text input coming from the keyboard or other input methods. @@ -26,302 +20,15 @@ export default class Input extends Feature { const editor = this.editor; const editingView = editor.editing.view; - /** - * Typing's change buffer used to group subsequent changes into batches. - * - * @protected - * @member {typing.ChangeBuffer} typing.Input#_buffer - */ - this._buffer = new ChangeBuffer( editor.document, editor.config.get( 'typing.undoStep' ) || 20 ); + editor.commands.set( 'inputKeydown', new InputCommand( editor, 'keydown' ) ); + editor.commands.set( 'inputMutation', new InputCommand( editor, 'mutation' ) ); - // TODO The above default configuration value should be defined using editor.config.define() once it's fixed. - - this.listenTo( editingView, 'keydown', ( evt, data ) => { - this._handleKeydown( data ); + this.listenTo( editingView, 'keydown', ( evt, evtData ) => { + editor.execute( 'inputKeydown', { evtData } ); }, { priority: 'lowest' } ); this.listenTo( editingView, 'mutations', ( evt, mutations, viewSelection ) => { - this._handleMutations( mutations, viewSelection ); - } ); - } - - /** - * @inheritDoc - */ - destroy() { - super.destroy(); - - this._buffer.destroy(); - this._buffer = null; - } - - /** - * Handles the keydown event. We need to guess whether such keystroke is going to result - * in typing. If so, then before character insertion happens, any selected content needs - * to be deleted. Otherwise the default browser deletion mechanism would be - * triggered, resulting in: - * - * * Hundreds of mutations which could not be handled. - * * But most importantly, loss of control over how the content is being deleted. - * - * The method is used in a low-priority listener, hence allowing other listeners (e.g. delete or enter features) - * to handle the event. - * - * @private - * @param {engine.view.observer.keyObserver.KeyEventData} evtData - */ - _handleKeydown( evtData ) { - const doc = this.editor.document; - - if ( isSafeKeystroke( evtData ) || doc.selection.isCollapsed ) { - return; - } - - doc.enqueueChanges( () => { - doc.composer.deleteContents( this._buffer.batch, doc.selection ); + editor.execute( 'inputMutation', { mutations, viewSelection } ); } ); } - - /** - * Handles DOM mutations. - * - * @param {Array.} mutations - */ - _handleMutations( mutations, viewSelection ) { - const doc = this.editor.document; - const handler = new MutationHandler( this.editor.editing, this._buffer ); - - doc.enqueueChanges( () => handler.handle( mutations, viewSelection ) ); - } -} - -/** - * Helper class for translating DOM mutations into model changes. - * - * @private - * @member typing.Input - */ -class MutationHandler { - /** - * Creates an instance of the mutation handler. - * - * @param {engine.EditingController} editing - * @param {typing.ChangeBuffer} buffer - */ - constructor( editing, buffer ) { - /** - * The editing controller. - * - * @member {engine.EditingController} typing.Input.MutationHandler#editing - */ - this.editing = editing; - - /** - * The change buffer. - * - * @member {engine.EditingController} typing.Input.MutationHandler#buffer - */ - this.buffer = buffer; - - /** - * The number of inserted characters which need to be fed to the {@link #buffer change buffer} - * on {@link #commit}. - * - * @member {Number} typing.Input.MutationHandler#insertedCharacterCount - */ - this.insertedCharacterCount = 0; - } - - /** - * Handles given mutations. - * - * @param {Array.} mutations - */ - handle( mutations, viewSelection ) { - for ( let mutation of mutations ) { - // Fortunately it will never be both. - this._handleTextMutation( mutation, viewSelection ); - this._handleTextNodeInsertion( mutation ); - } - - this.buffer.input( Math.max( this.insertedCharacterCount, 0 ) ); - } - - _handleTextMutation( mutation, viewSelection ) { - if ( mutation.type != 'text' ) { - return; - } - - // Replace   inserted by the browser with normal space. - // We want only normal spaces in the model and in the view. Renderer and DOM Converter will be then responsible - // for rendering consecutive spaces using  , but the model and the view has to be clear. - // Other feature may introduce inserting non-breakable space on specific key stroke (for example shift + space). - // However then it will be handled outside of mutations, like enter key is. - // The replacing is here because it has to be done before `diff` and `diffToChanges` functions, as they - // take `newText` and compare it to (cleaned up) view. - // It could also be done in mutation observer too, however if any outside plugin would like to - // introduce additional events for mutations, they would get already cleaned up version (this may be good or not). - const newText = mutation.newText.replace( /\u00A0/g, ' ' ); - // To have correct `diffResult`, we also compare view node text data with   replaced by space. - const oldText = mutation.oldText.replace( /\u00A0/g, ' ' ); - - const diffResult = diff( oldText, newText ); - - // Index where the first change happens. Used to set the position from which nodes will be removed and where will be inserted. - let firstChangeAt = null; - // Index where the last change happens. Used to properly count how many characters have to be removed and inserted. - let lastChangeAt = null; - - // Get `firstChangeAt` and `lastChangeAt`. - for ( let i = 0; i < diffResult.length; i++ ) { - const change = diffResult[ i ]; - - if ( change != 'equal' ) { - firstChangeAt = firstChangeAt === null ? i : firstChangeAt; - lastChangeAt = i; - } - } - - // How many characters, starting from `firstChangeAt`, should be removed. - let deletions = 0; - // How many characters, starting from `firstChangeAt`, should be inserted (basing on mutation.newText). - let insertions = 0; - - for ( let i = firstChangeAt; i <= lastChangeAt; i++ ) { - // If there is no change (equal) or delete, the character is existing in `oldText`. We count it for removing. - if ( diffResult[ i ] != 'insert' ) { - deletions++; - } - - // If there is no change (equal) or insert, the character is existing in `newText`. We count it for inserting. - if ( diffResult[ i ] != 'delete' ) { - insertions++; - } - } - - // Try setting new model selection according to passed view selection. - let modelSelectionPosition = null; - - if ( viewSelection ) { - modelSelectionPosition = this.editing.mapper.toModelPosition( viewSelection.anchor ); - } - - // Get the position in view and model where the changes will happen. - const viewPos = new ViewPosition( mutation.node, firstChangeAt ); - const modelPos = this.editing.mapper.toModelPosition( viewPos ); - - // Remove appropriate number of characters from the model text node. - if ( deletions > 0 ) { - const removeRange = ModelRange.createFromPositionAndShift( modelPos, deletions ); - this._remove( removeRange, deletions ); - } - - // Insert appropriate characters basing on `mutation.text`. - const insertedText = mutation.newText.substr( firstChangeAt, insertions ); - this._insert( modelPos, insertedText ); - - // If there was `viewSelection` and it got correctly mapped, collapse selection at found model position. - if ( modelSelectionPosition ) { - this.editing.model.selection.collapse( modelSelectionPosition ); - } - } - - _handleTextNodeInsertion( mutation ) { - if ( mutation.type != 'children' ) { - return; - } - - // One new node. - if ( mutation.newChildren.length - mutation.oldChildren.length != 1 ) { - return; - } - - // Which is text. - const diffResult = diff( mutation.oldChildren, mutation.newChildren, compareChildNodes ); - const changes = diffToChanges( diffResult, mutation.newChildren ); - - // In case of [ delete, insert, insert ] the previous check will not exit. - if ( changes.length > 1 ) { - return; - } - - const change = changes[ 0 ]; - - // Which is text. - if ( !( change.values[ 0 ] instanceof ViewText ) ) { - return; - } - - const viewPos = new ViewPosition( mutation.node, change.index ); - const modelPos = this.editing.mapper.toModelPosition( viewPos ); - let insertedText = change.values[ 0 ].data; - - // Replace   inserted by the browser with normal space. - // See comment in `_handleTextMutation`. - // In this case we don't need to do this before `diff` because we diff whole nodes. - // Just change   in case there are some. - insertedText = insertedText.replace( /\u00A0/g, ' ' ); - - this._insert( modelPos, insertedText ); - - this.editing.model.selection.collapse( modelPos.parent, 'end' ); - } - - _insert( position, text ) { - this.buffer.batch.weakInsert( position, text ); - - this.insertedCharacterCount += text.length; - } - - _remove( range, length ) { - this.buffer.batch.remove( range ); - - this.insertedCharacterCount -= length; - } -} - -const safeKeycodes = [ - getCode( 'arrowUp' ), - getCode( 'arrowRight' ), - getCode( 'arrowDown' ), - getCode( 'arrowLeft' ), - 16, // Shift - 17, // Ctrl - 18, // Alt - 20, // CapsLock - 27, // Escape - 33, // PageUp - 34, // PageDown - 35, // Home - 36, // End -]; - -// Function keys. -for ( let code = 112; code <= 135; code++ ) { - safeKeycodes.push( code ); -} - -// Returns `true` if a keystroke should not cause any content change caused by "typing". -// -// Note: This implementation is very simple and will need to be refined with time. -// -// @param {engine.view.observer.keyObserver.KeyEventData} keyData -// @returns {Boolean} -function isSafeKeystroke( keyData ) { - // Keystrokes which contain Ctrl don't represent typing. - if ( keyData.ctrlKey ) { - return true; - } - - return safeKeycodes.includes( keyData.keyCode ); -} - -// Helper function that compares whether two given view nodes are same. It is used in `diff` when it's passed an array -// with child nodes. -function compareChildNodes( oldChild, newChild ) { - if ( oldChild instanceof ViewText && newChild instanceof ViewText ) { - return oldChild.data === newChild.data; - } else { - return oldChild === newChild; - } } diff --git a/src/inputcommand.js b/src/inputcommand.js new file mode 100644 index 0000000..504422d --- /dev/null +++ b/src/inputcommand.js @@ -0,0 +1,339 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import ChangeBuffer from './changebuffer.js'; + +import Command from '../core/command/command.js'; +import ModelRange from '../engine/model/range.js'; +import ViewPosition from '../engine/view/position.js'; +import ViewText from '../engine/view/text.js'; + +import diff from '../utils/diff.js'; +import diffToChanges from '../utils/difftochanges.js'; +import { getCode } from '../utils/keyboard.js'; + +/** + * The input command. Used by the {@link typing.Input input feature} to handle inserting new characters. + * + * @member typing + * @extends core.command.Command + */ +export default class InputCommand extends Command { + /** + * Creates an instance of the command. + * + * @param {core.editor.Editor} editor + * @param {'keydown'|'mutation'} inputType Type of input operation to handle. + */ + constructor( editor, inputType ) { + super( editor ); + + this.inputType = inputType; + + /** + * Input's change buffer used to group subsequent changes into batches. + * + * @readonly + * @private + * @member {typing.ChangeBuffer} typing.InputCommand#buffer + */ + this._buffer = new ChangeBuffer( editor.document, editor.config.get( 'undo.step' ) || 20 ); + + /** + * @readonly + * @private + */ + this._handler = new MutationHandler( this.editor.editing, this._buffer ); + } + + /** + * Executes the input command. Depending on the `inputType` it handles `keydown` or `mutation` input. + * + * @param {Object} [options] The command options. + * @param {engine.view.observer.DomEventData} [options.evtData] Event data. + * @param {Array.} [options.mutations] List of view mutations. + * @param {engine.view.Selection} [options.viewSelection] Selection object of view. + */ + _doExecute( options = {} ) { + const { evtData, mutations, viewSelection } = options; + + if ( this.inputType === 'keydown' ) { + this._handleKeydown( evtData ); + } + + if ( this.inputType === 'mutation' ) { + this._handleMutations( mutations, viewSelection ); + } + } + + /** + * Handles the keydown event. We need to guess whether such keystroke is going to result + * in typing. If so, then before character insertion happens, any selected content needs + * to be deleted. Otherwise the default browser deletion mechanism would be + * triggered, resulting in: + * + * * Hundreds of mutations which could not be handled. + * * But most importantly, loss of control over how the content is being deleted. + * + * The method is used in a low-priority listener, hence allowing other listeners (e.g. input or enter features) + * to handle the event. + * + * @private + * @param {engine.view.observer.keyObserver.KeyEventData} evtData + */ + _handleKeydown( evtData ) { + const doc = this.editor.document; + + if ( isSafeKeystroke( evtData ) || doc.selection.isCollapsed ) { + return; + } + + doc.enqueueChanges( () => { + doc.composer.deleteContents( this._buffer.batch, doc.selection ); + } ); + } + + /** + * Handles DOM mutations. + * + * @param {Array.} mutations + */ + _handleMutations( mutations, viewSelection ) { + const doc = this.editor.document; + + doc.enqueueChanges( () => this._handler.handle( mutations, viewSelection ) ); + } +} + +/** + * Helper class for translating DOM mutations into model changes. + * + * @private + * @member typing.Input + */ +class MutationHandler { + /** + * Creates an instance of the mutation handler. + * + * @param {engine.EditingController} editing + * @param {typing.ChangeBuffer} buffer + */ + constructor( editing, buffer ) { + /** + * The editing controller. + * + * @member {engine.EditingController} typing.Input.MutationHandler#editing + */ + this.editing = editing; + + /** + * The change buffer. + * + * @member {engine.EditingController} typing.Input.MutationHandler#buffer + */ + this.buffer = buffer; + + /** + * The number of inserted characters which need to be fed to the {@link #buffer change buffer} + * on {@link #commit}. + * + * @member {Number} typing.Input.MutationHandler#insertedCharacterCount + */ + this.insertedCharacterCount = 0; + } + + /** + * Handles given mutations. + * + * @param {Array.} mutations + */ + handle( mutations, viewSelection ) { + for ( let mutation of mutations ) { + // Fortunately it will never be both. + this._handleTextMutation( mutation, viewSelection ); + this._handleTextNodeInsertion( mutation ); + } + + this.buffer.input( Math.max( this.insertedCharacterCount, 0 ) ); + } + + _handleTextMutation( mutation, viewSelection ) { + if ( mutation.type != 'text' ) { + return; + } + + // Replace   inserted by the browser with normal space. + // We want only normal spaces in the model and in the view. Renderer and DOM Converter will be then responsible + // for rendering consecutive spaces using  , but the model and the view has to be clear. + // Other feature may introduce inserting non-breakable space on specific key stroke (for example shift + space). + // However then it will be handled outside of mutations, like enter key is. + // The replacing is here because it has to be done before `diff` and `diffToChanges` functions, as they + // take `newText` and compare it to (cleaned up) view. + // It could also be done in mutation observer too, however if any outside plugin would like to + // introduce additional events for mutations, they would get already cleaned up version (this may be good or not). + const newText = mutation.newText.replace( /\u00A0/g, ' ' ); + // To have correct `diffResult`, we also compare view node text data with   replaced by space. + const oldText = mutation.oldText.replace( /\u00A0/g, ' ' ); + + const diffResult = diff( oldText, newText ); + + // Index where the first change happens. Used to set the position from which nodes will be removed and where will be inserted. + let firstChangeAt = null; + // Index where the last change happens. Used to properly count how many characters have to be removed and inserted. + let lastChangeAt = null; + + // Get `firstChangeAt` and `lastChangeAt`. + for ( let i = 0; i < diffResult.length; i++ ) { + const change = diffResult[ i ]; + + if ( change != 'equal' ) { + firstChangeAt = firstChangeAt === null ? i : firstChangeAt; + lastChangeAt = i; + } + } + + // How many characters, starting from `firstChangeAt`, should be removed. + let deletions = 0; + // How many characters, starting from `firstChangeAt`, should be inserted (basing on mutation.newText). + let insertions = 0; + + for ( let i = firstChangeAt; i <= lastChangeAt; i++ ) { + // If there is no change (equal) or input, the character is existing in `oldText`. We count it for removing. + if ( diffResult[ i ] != 'insert' ) { + deletions++; + } + + // If there is no change (equal) or insert, the character is existing in `newText`. We count it for inserting. + if ( diffResult[ i ] != 'delete' ) { + insertions++; + } + } + + // Try setting new model selection according to passed view selection. + let modelSelectionPosition = null; + + if ( viewSelection ) { + modelSelectionPosition = this.editing.mapper.toModelPosition( viewSelection.anchor ); + } + + // Get the position in view and model where the changes will happen. + const viewPos = new ViewPosition( mutation.node, firstChangeAt ); + const modelPos = this.editing.mapper.toModelPosition( viewPos ); + + // Remove appropriate number of characters from the model text node. + if ( deletions > 0 ) { + const removeRange = ModelRange.createFromPositionAndShift( modelPos, deletions ); + this._remove( removeRange, deletions ); + } + + // Insert appropriate characters basing on `mutation.text`. + const insertedText = mutation.newText.substr( firstChangeAt, insertions ); + this._insert( modelPos, insertedText ); + + // If there was `viewSelection` and it got correctly mapped, collapse selection at found model position. + if ( modelSelectionPosition ) { + this.editing.model.selection.collapse( modelSelectionPosition ); + } + } + + _handleTextNodeInsertion( mutation ) { + if ( mutation.type != 'children' ) { + return; + } + + // One new node. + if ( mutation.newChildren.length - mutation.oldChildren.length != 1 ) { + return; + } + + // Which is text. + const diffResult = diff( mutation.oldChildren, mutation.newChildren, compareChildNodes ); + const changes = diffToChanges( diffResult, mutation.newChildren ); + + // In case of [ input, insert, insert ] the previous check will not exit. + if ( changes.length > 1 ) { + return; + } + + const change = changes[ 0 ]; + + // Which is text. + if ( !( change.values[ 0 ] instanceof ViewText ) ) { + return; + } + + const viewPos = new ViewPosition( mutation.node, change.index ); + const modelPos = this.editing.mapper.toModelPosition( viewPos ); + let insertedText = change.values[ 0 ].data; + + // Replace   inserted by the browser with normal space. + // See comment in `_handleTextMutation`. + // In this case we don't need to do this before `diff` because we diff whole nodes. + // Just change   in case there are some. + insertedText = insertedText.replace( /\u00A0/g, ' ' ); + + this._insert( modelPos, insertedText ); + + this.editing.model.selection.collapse( modelPos.parent, 'end' ); + } + + _insert( position, text ) { + this.buffer.batch.weakInsert( position, text ); + + this.insertedCharacterCount += text.length; + } + + _remove( range, length ) { + this.buffer.batch.remove( range ); + + this.insertedCharacterCount -= length; + } +} + +const safeKeycodes = [ + getCode( 'arrowUp' ), + getCode( 'arrowRight' ), + getCode( 'arrowDown' ), + getCode( 'arrowLeft' ), + 16, // Shift + 17, // Ctrl + 18, // Alt + 20, // CapsLock + 27, // Escape + 33, // PageUp + 34, // PageDown + 35, // Home + 36, // End +]; + +// Function keys. +for ( let code = 112; code <= 135; code++ ) { + safeKeycodes.push( code ); +} + +// Returns `true` if a keystroke should not cause any content change caused by "typing". +// +// Note: This implementation is very simple and will need to be refined with time. +// +// @param {engine.view.observer.keyObserver.KeyEventData} keyData +// @returns {Boolean} +function isSafeKeystroke( keyData ) { + // Keystrokes which contain Ctrl don't represent typing. + if ( keyData.ctrlKey ) { + return true; + } + + return safeKeycodes.includes( keyData.keyCode ); +} + +// Helper function that compares whether two given view nodes are same. It is used in `diff` when it's passed an array +// with child nodes. +function compareChildNodes( oldChild, newChild ) { + if ( oldChild instanceof ViewText && newChild instanceof ViewText ) { + return oldChild.data === newChild.data; + } else { + return oldChild === newChild; + } +} diff --git a/tests/input.js b/tests/input.js index 0e803ac..9b2c6c5 100644 --- a/tests/input.js +++ b/tests/input.js @@ -65,22 +65,6 @@ describe( 'Input feature', () => { listenter.stopListening(); } ); - it( 'has a buffer configured to default value of config.typing.undoStep', () => { - expect( editor.plugins.get( Input )._buffer ).to.have.property( 'limit', 20 ); - } ); - - it( 'has a buffer configured to config.typing.undoStep', () => { - return VirtualTestEditor.create( { - features: [ Input ], - typing: { - undoStep: 5 - } - } ) - .then( editor => { - expect( editor.plugins.get( Input )._buffer ).to.have.property( 'limit', 5 ); - } ); - } ); - describe( 'mutations handling', () => { it( 'should handle text mutation', () => { view.fire( 'mutations', [ @@ -313,19 +297,5 @@ describe( 'Input feature', () => { expect( getModelData( model ) ).to.equal( 'foo[]bar' ); } ); } ); - - describe( 'destroy', () => { - it( 'should destroy change buffer', () => { - const typing = new Input( new VirtualTestEditor() ); - typing.init(); - - const destroy = typing._buffer.destroy = sinon.spy(); - - typing.destroy(); - - expect( destroy.calledOnce ).to.be.true; - expect( typing._buffer ).to.be.null; - } ); - } ); } ); From b99d01dcdc66571d95290f7c5ec52ed784b71923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 27 Oct 2016 10:14:16 +0200 Subject: [PATCH 2/3] Moved _handleKeydown to Input feature. --- src/input.js | 69 +++++++++++++++++++++++++++++++++++-- src/inputcommand.js | 84 +++------------------------------------------ 2 files changed, 71 insertions(+), 82 deletions(-) diff --git a/src/input.js b/src/input.js index e28f16c..bd9d660 100644 --- a/src/input.js +++ b/src/input.js @@ -5,6 +5,7 @@ import Feature from '../core/feature.js'; import InputCommand from './inputcommand.js'; +import { getCode } from '../utils/keyboard.js'; /** * Handles text input coming from the keyboard or other input methods. @@ -20,15 +21,77 @@ export default class Input extends Feature { const editor = this.editor; const editingView = editor.editing.view; - editor.commands.set( 'inputKeydown', new InputCommand( editor, 'keydown' ) ); - editor.commands.set( 'inputMutation', new InputCommand( editor, 'mutation' ) ); + editor.commands.set( 'inputMutation', new InputCommand( editor ) ); this.listenTo( editingView, 'keydown', ( evt, evtData ) => { - editor.execute( 'inputKeydown', { evtData } ); + this._handleKeydown( evtData ); }, { priority: 'lowest' } ); this.listenTo( editingView, 'mutations', ( evt, mutations, viewSelection ) => { editor.execute( 'inputMutation', { mutations, viewSelection } ); } ); } + + /** + * Handles the keydown event. We need to guess whether such keystroke is going to result + * in typing. If so, then before character insertion happens, any selected content needs + * to be deleted. Otherwise the default browser deletion mechanism would be + * triggered, resulting in: + * + * * Hundreds of mutations which could not be handled. + * * But most importantly, loss of control over how the content is being deleted. + * + * The method is used in a low-priority listener, hence allowing other listeners (e.g. input or enter features) + * to handle the event. + * + * @private + * @param {engine.view.observer.keyObserver.KeyEventData} evtData + */ + _handleKeydown( evtData ) { + const doc = this.editor.document; + + if ( isSafeKeystroke( evtData ) || doc.selection.isCollapsed ) { + return; + } + + doc.enqueueChanges( () => { + doc.composer.deleteContents( doc.batch(), doc.selection ); + } ); + } +} + +const safeKeycodes = [ + getCode( 'arrowUp' ), + getCode( 'arrowRight' ), + getCode( 'arrowDown' ), + getCode( 'arrowLeft' ), + 16, // Shift + 17, // Ctrl + 18, // Alt + 20, // CapsLock + 27, // Escape + 33, // PageUp + 34, // PageDown + 35, // Home + 36, // End +]; + +// Function keys. +for ( let code = 112; code <= 135; code++ ) { + safeKeycodes.push( code ); +} + +// Returns `true` if a keystroke should not cause any content change caused by "typing". +// +// Note: This implementation is very simple and will need to be refined with time. +// +// @param {engine.view.observer.keyObserver.KeyEventData} keyData +// @returns {Boolean} +function isSafeKeystroke( keyData ) { + // Keystrokes which contain Ctrl don't represent typing. + if ( keyData.ctrlKey ) { + return true; + } + + return safeKeycodes.includes( keyData.keyCode ); } diff --git a/src/inputcommand.js b/src/inputcommand.js index 504422d..78feed5 100644 --- a/src/inputcommand.js +++ b/src/inputcommand.js @@ -12,7 +12,6 @@ import ViewText from '../engine/view/text.js'; import diff from '../utils/diff.js'; import diffToChanges from '../utils/difftochanges.js'; -import { getCode } from '../utils/keyboard.js'; /** * The input command. Used by the {@link typing.Input input feature} to handle inserting new characters. @@ -25,13 +24,10 @@ export default class InputCommand extends Command { * Creates an instance of the command. * * @param {core.editor.Editor} editor - * @param {'keydown'|'mutation'} inputType Type of input operation to handle. */ - constructor( editor, inputType ) { + constructor( editor ) { super( editor ); - this.inputType = inputType; - /** * Input's change buffer used to group subsequent changes into batches. * @@ -51,48 +47,14 @@ export default class InputCommand extends Command { /** * Executes the input command. Depending on the `inputType` it handles `keydown` or `mutation` input. * - * @param {Object} [options] The command options. - * @param {engine.view.observer.DomEventData} [options.evtData] Event data. - * @param {Array.} [options.mutations] List of view mutations. + * @param {Object} options The command options. + * @param {Array.} options.mutations List of view mutations. * @param {engine.view.Selection} [options.viewSelection] Selection object of view. */ _doExecute( options = {} ) { - const { evtData, mutations, viewSelection } = options; - - if ( this.inputType === 'keydown' ) { - this._handleKeydown( evtData ); - } - - if ( this.inputType === 'mutation' ) { - this._handleMutations( mutations, viewSelection ); - } - } - - /** - * Handles the keydown event. We need to guess whether such keystroke is going to result - * in typing. If so, then before character insertion happens, any selected content needs - * to be deleted. Otherwise the default browser deletion mechanism would be - * triggered, resulting in: - * - * * Hundreds of mutations which could not be handled. - * * But most importantly, loss of control over how the content is being deleted. - * - * The method is used in a low-priority listener, hence allowing other listeners (e.g. input or enter features) - * to handle the event. - * - * @private - * @param {engine.view.observer.keyObserver.KeyEventData} evtData - */ - _handleKeydown( evtData ) { - const doc = this.editor.document; - - if ( isSafeKeystroke( evtData ) || doc.selection.isCollapsed ) { - return; - } + const { mutations, viewSelection } = options; - doc.enqueueChanges( () => { - doc.composer.deleteContents( this._buffer.batch, doc.selection ); - } ); + this._handleMutations( mutations, viewSelection ); } /** @@ -292,42 +254,6 @@ class MutationHandler { } } -const safeKeycodes = [ - getCode( 'arrowUp' ), - getCode( 'arrowRight' ), - getCode( 'arrowDown' ), - getCode( 'arrowLeft' ), - 16, // Shift - 17, // Ctrl - 18, // Alt - 20, // CapsLock - 27, // Escape - 33, // PageUp - 34, // PageDown - 35, // Home - 36, // End -]; - -// Function keys. -for ( let code = 112; code <= 135; code++ ) { - safeKeycodes.push( code ); -} - -// Returns `true` if a keystroke should not cause any content change caused by "typing". -// -// Note: This implementation is very simple and will need to be refined with time. -// -// @param {engine.view.observer.keyObserver.KeyEventData} keyData -// @returns {Boolean} -function isSafeKeystroke( keyData ) { - // Keystrokes which contain Ctrl don't represent typing. - if ( keyData.ctrlKey ) { - return true; - } - - return safeKeycodes.includes( keyData.keyCode ); -} - // Helper function that compares whether two given view nodes are same. It is used in `diff` when it's passed an array // with child nodes. function compareChildNodes( oldChild, newChild ) { From 65c975140e21f93f412c309696c85f561a06d36d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 27 Oct 2016 13:47:34 +0200 Subject: [PATCH 3/3] Updated tests for Input feature and InputCommand. --- src/input.js | 3 +- src/inputcommand.js | 7 +- tests/input.js | 187 ++--------------------------- tests/inputcommand.js | 266 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 283 insertions(+), 180 deletions(-) create mode 100644 tests/inputcommand.js diff --git a/src/input.js b/src/input.js index bd9d660..8441a54 100644 --- a/src/input.js +++ b/src/input.js @@ -5,6 +5,7 @@ import Feature from '../core/feature.js'; import InputCommand from './inputcommand.js'; + import { getCode } from '../utils/keyboard.js'; /** @@ -41,7 +42,7 @@ export default class Input extends Feature { * * Hundreds of mutations which could not be handled. * * But most importantly, loss of control over how the content is being deleted. * - * The method is used in a low-priority listener, hence allowing other listeners (e.g. input or enter features) + * The method is used in a low-priority listener, hence allowing other listeners (e.g. delete or enter features) * to handle the event. * * @private diff --git a/src/inputcommand.js b/src/inputcommand.js index 78feed5..a07ecdb 100644 --- a/src/inputcommand.js +++ b/src/inputcommand.js @@ -35,7 +35,7 @@ export default class InputCommand extends Command { * @private * @member {typing.ChangeBuffer} typing.InputCommand#buffer */ - this._buffer = new ChangeBuffer( editor.document, editor.config.get( 'undo.step' ) || 20 ); + this._buffer = new ChangeBuffer( editor.document, editor.config.get( 'undo.step' ) ); /** * @readonly @@ -45,7 +45,7 @@ export default class InputCommand extends Command { } /** - * Executes the input command. Depending on the `inputType` it handles `keydown` or `mutation` input. + * Executes the input command. * * @param {Object} options The command options. * @param {Array.} options.mutations List of view mutations. @@ -119,6 +119,9 @@ class MutationHandler { } this.buffer.input( Math.max( this.insertedCharacterCount, 0 ) ); + + // Reset the counter after reaching buffer's limit. + this.insertedCharacterCount = this.insertedCharacterCount % this.buffer.limit; } _handleTextMutation( mutation, viewSelection ) { diff --git a/tests/input.js b/tests/input.js index 9b2c6c5..b7f6ee4 100644 --- a/tests/input.js +++ b/tests/input.js @@ -7,14 +7,7 @@ import VirtualTestEditor from '/tests/core/_utils/virtualtesteditor.js'; import Input from '/ckeditor5/typing/input.js'; import Paragraph from '/ckeditor5/paragraph/paragraph.js'; -import Batch from '/ckeditor5/engine/model/batch.js'; import ModelRange from '/ckeditor5/engine/model/range.js'; -import buildModelConverter from '/ckeditor5/engine/conversion/buildmodelconverter.js'; -import buildViewConverter from '/ckeditor5/engine/conversion/buildviewconverter.js'; - -import ViewText from '/ckeditor5/engine/view/text.js'; -import ViewElement from '/ckeditor5/engine/view/element.js'; -import ViewSelection from '/ckeditor5/engine/view/selection.js'; import EmitterMixin from '/ckeditor5/utils/emittermixin.js'; import { getCode } from '/ckeditor5/utils/keyboard.js'; @@ -32,17 +25,6 @@ describe( 'Input feature', () => { features: [ Input, Paragraph ] } ) .then( newEditor => { - // Mock image feature. - newEditor.document.schema.registerItem( 'image', '$inline' ); - - buildModelConverter().for( newEditor.data.modelToView, newEditor.editing.modelToView ) - .fromElement( 'image' ) - .toElement( 'img' ); - - buildViewConverter().for( newEditor.data.viewToModel ) - .fromElement( 'img' ) - .toElement( 'image' ); - editor = newEditor; model = editor.editing.model; modelRoot = model.getRoot(); @@ -65,168 +47,19 @@ describe( 'Input feature', () => { listenter.stopListening(); } ); - describe( 'mutations handling', () => { - it( 'should handle text mutation', () => { - view.fire( 'mutations', [ - { - type: 'text', - oldText: 'foobar', - newText: 'fooxbar', - node: viewRoot.getChild( 0 ).getChild( 0 ) - } - ] ); - - expect( getModelData( model ) ).to.equal( 'foox[]bar' ); - expect( getViewData( view ) ).to.equal( '

foox{}bar

' ); - } ); - - it( 'should handle text mutation change', () => { - view.fire( 'mutations', [ - { - type: 'text', - oldText: 'foobar', - newText: 'foodar', - node: viewRoot.getChild( 0 ).getChild( 0 ) - } - ] ); - - expect( getModelData( model ) ).to.equal( 'food[]ar' ); - expect( getViewData( view ) ).to.equal( '

food{}ar

' ); - } ); - - it( 'should handle text node insertion', () => { - editor.setData( '

' ); - - view.fire( 'mutations', [ - { - type: 'children', - oldChildren: [], - newChildren: [ new ViewText( 'x' ) ], - node: viewRoot.getChild( 0 ) - } - ] ); - - expect( getModelData( model ) ).to.equal( 'x[]' ); - expect( getViewData( view ) ).to.equal( '

x{}

' ); - } ); - - it( 'should do nothing when two nodes were inserted', () => { - editor.setData( '

' ); - - view.fire( 'mutations', [ - { - type: 'children', - oldChildren: [], - newChildren: [ new ViewText( 'x' ), new ViewElement( 'img' ) ], - node: viewRoot.getChild( 0 ) - } - ] ); - - expect( getModelData( model ) ).to.equal( '[]' ); - expect( getViewData( view ) ).to.equal( '

[]

' ); - } ); - - it( 'should do nothing when two nodes were inserted and one removed', () => { - view.fire( 'mutations', [ - { - type: 'children', - oldChildren: [ new ViewText( 'foobar' ) ], - newChildren: [ new ViewText( 'x' ), new ViewElement( 'img' ) ], - node: viewRoot.getChild( 0 ) - } - ] ); - - expect( getModelData( model ) ).to.equal( 'foo[]bar' ); - expect( getViewData( view ) ).to.equal( '

foo{}bar

' ); - } ); - - it( 'should handle multiple children in the node', () => { - editor.setData( '

foo

' ); - - view.fire( 'mutations', [ - { - type: 'children', - oldChildren: [ new ViewText( 'foo' ), viewRoot.getChild( 0 ).getChild( 1 ) ], - newChildren: [ new ViewText( 'foo' ), viewRoot.getChild( 0 ).getChild( 1 ), new ViewText( 'x' ) ], - node: viewRoot.getChild( 0 ) - } - ] ); - - expect( getModelData( model ) ).to.equal( 'foox[]' ); - expect( getViewData( view ) ).to.equal( '

foox{}

' ); - } ); - - it( 'should do nothing when node was removed', () => { - view.fire( 'mutations', [ - { - type: 'children', - oldChildren: [ new ViewText( 'foobar' ) ], - newChildren: [], - node: viewRoot.getChild( 0 ) - } - ] ); - - expect( getModelData( model ) ).to.equal( 'foo[]bar' ); - expect( getViewData( view ) ).to.equal( '

foo{}bar

' ); - } ); - - it( 'should do nothing when element was inserted', () => { - editor.setData( '

' ); - - view.fire( 'mutations', [ - { - type: 'children', - oldChildren: [], - newChildren: [ new ViewElement( 'img' ) ], - node: viewRoot.getChild( 0 ) - } - ] ); - - expect( getModelData( model ) ).to.equal( '[]' ); - expect( getViewData( view ) ).to.equal( '

[]

' ); - } ); - - it( 'should set model selection appropriately to view selection passed in mutations event', () => { - // This test case emulates spellchecker correction. - - const viewSelection = new ViewSelection(); - viewSelection.collapse( viewRoot.getChild( 0 ).getChild( 0 ), 6 ); - - view.fire( 'mutations', - [ { - type: 'text', - oldText: 'foobar', - newText: 'foodar', - node: viewRoot.getChild( 0 ).getChild( 0 ) - } ], - viewSelection - ); - - expect( getModelData( model ) ).to.equal( 'foodar[]' ); - expect( getViewData( view ) ).to.equal( '

foodar{}

' ); - } ); - - it( 'should use up to one insert and remove operations', () => { - // This test case emulates spellchecker correction. - - const viewSelection = new ViewSelection(); - viewSelection.collapse( viewRoot.getChild( 0 ).getChild( 0 ), 6 ); + describe( 'mutation handling', () => { + it( 'should execute inputMutation command when mutation event is fired', () => { + const command = editor.commands.get( 'inputMutation' ); + const spy = sinon.spy( command, '_execute' ); + const param = []; - sinon.spy( Batch.prototype, 'weakInsert' ); - sinon.spy( Batch.prototype, 'remove' ); + view.fire( 'mutations', param ); - view.fire( 'mutations', - [ { - type: 'text', - oldText: 'foobar', - newText: 'fxobxr', - node: viewRoot.getChild( 0 ).getChild( 0 ) - } ], - viewSelection - ); + expect( spy.calledOnce ).to.be.true; + expect( spy.getCall( 0 ).args[0].mutations === param ).to.be.true; - expect( Batch.prototype.weakInsert.calledOnce ).to.be.true; - expect( Batch.prototype.remove.calledOnce ).to.be.true; + // Revert to non-spied method. + command._execute.restore(); } ); } ); diff --git a/tests/inputcommand.js b/tests/inputcommand.js new file mode 100644 index 0000000..8e9f0c3 --- /dev/null +++ b/tests/inputcommand.js @@ -0,0 +1,266 @@ +/* + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import VirtualTestEditor from '/tests/core/_utils/virtualtesteditor.js'; +import InputCommand from '/ckeditor5/typing/inputcommand.js'; +import Paragraph from '/ckeditor5/paragraph/paragraph.js'; + +import Batch from '/ckeditor5/engine/model/batch.js'; +import ModelRange from '/ckeditor5/engine/model/range.js'; +import buildModelConverter from '/ckeditor5/engine/conversion/buildmodelconverter.js'; +import buildViewConverter from '/ckeditor5/engine/conversion/buildviewconverter.js'; + +import ViewText from '/ckeditor5/engine/view/text.js'; +import ViewElement from '/ckeditor5/engine/view/element.js'; +import ViewSelection from '/ckeditor5/engine/view/selection.js'; + +import { getData as getModelData } from '/ckeditor5/engine/dev-utils/model.js'; +import { getData as getViewData } from '/ckeditor5/engine/dev-utils/view.js'; + +describe( 'InputCommand', () => { + let editor, doc, model, modelRoot, view, viewRoot; + + before( () => { + return VirtualTestEditor.create( { + features: [ Paragraph ] + } ) + .then( newEditor => { + // Mock image feature. + newEditor.document.schema.registerItem( 'image', '$inline' ); + + buildModelConverter().for( newEditor.data.modelToView, newEditor.editing.modelToView ) + .fromElement( 'image' ) + .toElement( 'img' ); + + buildViewConverter().for( newEditor.data.viewToModel ) + .fromElement( 'img' ) + .toElement( 'image' ); + + editor = newEditor; + model = editor.editing.model; + modelRoot = model.getRoot(); + view = editor.editing.view; + viewRoot = view.getRoot(); + doc = editor.document; + + editor.commands.set( 'inputMutation', new InputCommand( editor ) ); + } ); + } ); + + beforeEach( () => { + editor.setData( '

foobar

' ); + + model.enqueueChanges( () => { + model.selection.setRanges( [ + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 3, modelRoot.getChild( 0 ), 3 ) + ] ); + } ); + } ); + + describe( 'execute', () => { + it( 'uses enqueueChanges', () => { + const spy = sinon.spy( doc, 'enqueueChanges' ); + + editor.execute( 'inputMutation', { mutations: [] } ); + + expect( spy.calledOnce ).to.be.true; + } ); + + it( 'should handle mutations', () => { + editor.setData( '

foo[]bar

' ); + + const mutations = [ { + type: 'text', + oldText: 'foobar', + newText: 'fooxbar', + node: viewRoot.getChild( 0 ).getChild( 0 ) + } ]; + + editor.execute( 'inputMutation', { mutations } ); + + expect( editor.getData( { selection: true } ) ).to.equal( '

foox[]bar

' ); + } ); + } ); + + describe( 'mutations handling', () => { + it( 'should handle text mutation', () => { + const mutations = [ + { + type: 'text', + oldText: 'foobar', + newText: 'fooxbar', + node: viewRoot.getChild( 0 ).getChild( 0 ) + } + ]; + + editor.execute( 'inputMutation', { mutations } ); + + expect( getModelData( model ) ).to.equal( 'foox[]bar' ); + expect( getViewData( view ) ).to.equal( '

foox{}bar

' ); + } ); + + it( 'should handle text mutation change', () => { + const mutations = [ + { + type: 'text', + oldText: 'foobar', + newText: 'foodar', + node: viewRoot.getChild( 0 ).getChild( 0 ) + } + ]; + + editor.execute( 'inputMutation', { mutations } ); + + expect( getModelData( model ) ).to.equal( 'food[]ar' ); + expect( getViewData( view ) ).to.equal( '

food{}ar

' ); + } ); + + it( 'should handle text node insertion', () => { + editor.setData( '

' ); + + const mutations = [ + { + type: 'children', + oldChildren: [], + newChildren: [ new ViewText( 'x' ) ], + node: viewRoot.getChild( 0 ) + } + ]; + + editor.execute( 'inputMutation', { mutations } ); + + expect( getModelData( model ) ).to.equal( 'x[]' ); + expect( getViewData( view ) ).to.equal( '

x{}

' ); + } ); + + it( 'should do nothing when two nodes were inserted', () => { + editor.setData( '

' ); + + const mutations = [ + { + type: 'children', + oldChildren: [], + newChildren: [ new ViewText( 'x' ), new ViewElement( 'img' ) ], + node: viewRoot.getChild( 0 ) + } + ]; + + editor.execute( 'inputMutation', { mutations } ); + + expect( getModelData( model ) ).to.equal( '[]' ); + expect( getViewData( view ) ).to.equal( '

[]

' ); + } ); + + it( 'should do nothing when two nodes were inserted and one removed', () => { + const mutations = [ + { + type: 'children', + oldChildren: [ new ViewText( 'foobar' ) ], + newChildren: [ new ViewText( 'x' ), new ViewElement( 'img' ) ], + node: viewRoot.getChild( 0 ) + } + ]; + + editor.execute( 'inputMutation', { mutations } ); + + expect( getModelData( model ) ).to.equal( 'foo[]bar' ); + expect( getViewData( view ) ).to.equal( '

foo{}bar

' ); + } ); + + it( 'should handle multiple children in the node', () => { + editor.setData( '

foo

' ); + + const mutations = [ + { + type: 'children', + oldChildren: [ new ViewText( 'foo' ), viewRoot.getChild( 0 ).getChild( 1 ) ], + newChildren: [ new ViewText( 'foo' ), viewRoot.getChild( 0 ).getChild( 1 ), new ViewText( 'x' ) ], + node: viewRoot.getChild( 0 ) + } + ]; + + editor.execute( 'inputMutation', { mutations } ); + + expect( getModelData( model ) ).to.equal( 'foox[]' ); + expect( getViewData( view ) ).to.equal( '

foox{}

' ); + } ); + + it( 'should do nothing when node was removed', () => { + const mutations = [ + { + type: 'children', + oldChildren: [ new ViewText( 'foobar' ) ], + newChildren: [], + node: viewRoot.getChild( 0 ) + } + ]; + + editor.execute( 'inputMutation', { mutations } ); + + expect( getModelData( model ) ).to.equal( 'foo[]bar' ); + expect( getViewData( view ) ).to.equal( '

foo{}bar

' ); + } ); + + it( 'should do nothing when element was inserted', () => { + editor.setData( '

' ); + + const mutations = [ + { + type: 'children', + oldChildren: [], + newChildren: [ new ViewElement( 'img' ) ], + node: viewRoot.getChild( 0 ) + } + ]; + + editor.execute( 'inputMutation', { mutations } ); + + expect( getModelData( model ) ).to.equal( '[]' ); + expect( getViewData( view ) ).to.equal( '

[]

' ); + } ); + + it( 'should set model selection appropriately to view selection passed in mutations event', () => { + // This test case emulates spellchecker correction. + + const viewSelection = new ViewSelection(); + viewSelection.collapse( viewRoot.getChild( 0 ).getChild( 0 ), 6 ); + + const mutations = [ { + type: 'text', + oldText: 'foobar', + newText: 'foodar', + node: viewRoot.getChild( 0 ).getChild( 0 ) + } ]; + + editor.execute( 'inputMutation', { mutations, viewSelection } ); + + expect( getModelData( model ) ).to.equal( 'foodar[]' ); + expect( getViewData( view ) ).to.equal( '

foodar{}

' ); + } ); + + it( 'should use up to one insert and remove operations', () => { + // This test case emulates spellchecker correction. + + const viewSelection = new ViewSelection(); + viewSelection.collapse( viewRoot.getChild( 0 ).getChild( 0 ), 6 ); + + sinon.spy( Batch.prototype, 'weakInsert' ); + sinon.spy( Batch.prototype, 'remove' ); + + const mutations = [ { + type: 'text', + oldText: 'foobar', + newText: 'fxobxr', + node: viewRoot.getChild( 0 ).getChild( 0 ) + } ]; + + editor.execute( 'inputMutation', { mutations, viewSelection } ); + + expect( Batch.prototype.weakInsert.calledOnce ).to.be.true; + expect( Batch.prototype.remove.calledOnce ).to.be.true; + } ); + } ); +} ); +