From 2593be4ee667dbf8954f8fa7d490f9ea8bd0bbee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 13 Jun 2018 11:01:44 +0200 Subject: [PATCH 1/8] Handle content deletion before composition start to simplify mutations. --- src/input.js | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/src/input.js b/src/input.js index e8dffce..60da40d 100644 --- a/src/input.js +++ b/src/input.js @@ -8,6 +8,7 @@ */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import Selection from '@ckeditor/ckeditor5-engine/src/model/selection'; import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range'; import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position'; import ViewText from '@ckeditor/ckeditor5-engine/src/view/text'; @@ -40,12 +41,22 @@ export default class Input extends Plugin { // TODO The above default configuration value should be defined using editor.config.define() once it's fixed. + this._latestCompositionSelection = null; + editor.commands.add( 'input', inputCommand ); this.listenTo( editingView.document, 'keydown', ( evt, data ) => { this._handleKeydown( data, inputCommand ); }, { priority: 'lowest' } ); + this.listenTo( editingView.document, 'compositionstart', () => { + this._handleCompositionstart( inputCommand ); + }, { priority: 'lowest' } ); + + this.listenTo( editingView.document, 'compositionend', () => { + this._handleCompositionend(); + }, { priority: 'lowest' } ); + this.listenTo( editingView.document, 'mutations', ( evt, mutations, viewSelection ) => { this._handleMutations( mutations, viewSelection ); } ); @@ -71,6 +82,11 @@ export default class Input extends Plugin { const model = this.editor.model; const doc = model.document; const buffer = inputCommand.buffer; + const isComposing = this.editor.editing.view.document.isComposing; + const isSelectionUnchanged = this._latestCompositionSelection && this._latestCompositionSelection.isEqual( doc.selection ); + + // Reset stored composition selection. + this._latestCompositionSelection = null; // By relying on the state of the input command we allow disabling the entire input easily // by just disabling the input command. We could’ve used here the delete command but that @@ -85,6 +101,52 @@ export default class Input extends Plugin { return; } + // If not during composition we should also delete content on '229' key code (composition start). + // While composing, deletion should be prevented as it may remove composed sequence (#83). + if ( isComposing && evtData.keyCode === 229 ) { + return; + } + + // If there is a `keydown` event fired with '229' keycode it might be related to recent composition. + // Check if selection is the same as upon ending recent composition, if so do not remove selected content + // as it will remove composed sequence (#83). + if ( !isComposing && evtData.keyCode === 229 && isSelectionUnchanged ) { + return; + } + + buffer.lock(); + + model.enqueueChange( buffer.batch, () => { + this.editor.model.deleteContent( doc.selection ); + } ); + + buffer.unlock(); + } + + /** + * Handles the `compositionstart` event. It is used only in special cases to remove the contents + * of a non-collapsed selection so composition itself does not result in complex mutations. + * + * The special case mentioned above is a situation in which the `keydown` event is fired after + * `compositionstart` event. In such cases {@link #_handleKeydown} cannot clear current selection + * contents (because it is too late and will break the composition) so the composition handler takes care of it. + * + * @private + * @param {module:typing/inputcommand~InputCommand} inputCommand + */ + _handleCompositionstart( inputCommand ) { + const model = this.editor.model; + const doc = model.document; + const buffer = inputCommand.buffer; + const isFlatSelection = doc.selection.rangeCount === 1 ? doc.selection.getFirstRange().isFlat : true; + + // If on `compositionstart` there is a non-collapsed selection containing more than one element + // it means `_handleKeydown()` method didn't removed its content. It happens usually because of + // events different order (`compositionstart` before `keydown`) - observed only in Safari (#83). + if ( doc.selection.isCollapsed || isFlatSelection ) { + return; + } + buffer.lock(); model.enqueueChange( buffer.batch, () => { @@ -94,6 +156,18 @@ export default class Input extends Plugin { buffer.unlock(); } + /** + * Handles `compositionend` event. It is used to store latest composition selection. + * + * @private + */ + _handleCompositionend() { + // Store current selection on `compositionend`. It is then used in `_handleKeydown()` method + // for browsers (Safari) which fire `keydown` event after (and not before) `compositionend` + // to detect if selection content should be removed (#83). + this._latestCompositionSelection = new Selection( this.editor.model.document.selection ); + } + /** * Handles DOM mutations. * @@ -346,8 +420,7 @@ const safeKeycodes = [ 33, // PageUp 34, // PageDown 35, // Home - 36, // End - 229 // Composition start key + 36 // End ]; // Function keys. From 231f4e31aecf9289f9a1387a3ffb01f0c73891f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 20 Jun 2018 10:33:24 +0200 Subject: [PATCH 2/8] Rewording. --- src/input.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/input.js b/src/input.js index 60da40d..c36dc78 100644 --- a/src/input.js +++ b/src/input.js @@ -140,9 +140,10 @@ export default class Input extends Plugin { const buffer = inputCommand.buffer; const isFlatSelection = doc.selection.rangeCount === 1 ? doc.selection.getFirstRange().isFlat : true; - // If on `compositionstart` there is a non-collapsed selection containing more than one element - // it means `_handleKeydown()` method didn't removed its content. It happens usually because of - // events different order (`compositionstart` before `keydown`) - observed only in Safari (#83). + // If on `compositionstart` there is a non-collapsed selection which start and end have different block + // parents it means `_handleKeydown()` method did not remove its contents. It happens usually because + // of different order of events (`compositionstart` before `keydown` - in Safari). In such cases + // we need to remove selection contents on composition start (#83). if ( doc.selection.isCollapsed || isFlatSelection ) { return; } From a07c7f134a26aeb9d36e38a6a952ef0babb5d259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 20 Jun 2018 10:36:21 +0200 Subject: [PATCH 3/8] Tests: content deletion on 'compositonstart' unit tests. --- tests/input.js | 122 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 109 insertions(+), 13 deletions(-) diff --git a/tests/input.js b/tests/input.js index cd24773..408fd31 100644 --- a/tests/input.js +++ b/tests/input.js @@ -8,11 +8,13 @@ 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/bold'; +import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; import ShiftEnter from '@ckeditor/ckeditor5-enter/src/shiftenter'; import Input from '../src/input'; import Writer from '@ckeditor/ckeditor5-engine/src/model/writer'; import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range'; +import ModelSelection from '@ckeditor/ckeditor5-engine/src/model/selection'; import ViewText from '@ckeditor/ckeditor5-engine/src/view/text'; import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element'; @@ -37,7 +39,7 @@ describe( 'Input feature', () => { const domElement = document.createElement( 'div' ); document.body.appendChild( domElement ); - return ClassicTestEditor.create( domElement, { plugins: [ Input, Paragraph, Bold, List, ShiftEnter ] } ) + return ClassicTestEditor.create( domElement, { plugins: [ Input, Paragraph, Bold, Italic, List, ShiftEnter ] } ) .then( newEditor => { // Mock image feature. newEditor.model.schema.register( 'image', { allowWhere: '$text' } ); @@ -614,18 +616,6 @@ describe( 'Input feature', () => { expect( getModelData( model ) ).to.equal( 'fo[ob]ar' ); } ); - // #82 - it( 'should do nothing on composition start key', () => { - model.change( writer => { - writer.setSelection( - ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 4 ) ); - } ); - - viewDocument.fire( 'keydown', { keyCode: 229 } ); - - expect( getModelData( model ) ).to.equal( 'fo[ob]ar' ); - } ); - it( 'should do nothing if selection is collapsed', () => { viewDocument.fire( 'keydown', { ctrlKey: true, keyCode: getCode( 'c' ) } ); @@ -693,6 +683,112 @@ describe( 'Input feature', () => { expect( getModelData( model ) ).to.equal( 'fo[ob]ar' ); } ); + + describe( '#83', () => { + it( 'should remove contents on composition start key if not during composition', () => { + model.change( writer => { + writer.setSelection( + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 4 ) ); + } ); + + viewDocument.fire( 'keydown', { keyCode: 229 } ); + + expect( getModelData( model ) ).to.equal( 'fo[]ar' ); + } ); + + it( 'should not remove contents on composition start key if during composition', () => { + model.change( writer => { + writer.setSelection( + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 4 ) ); + } ); + + viewDocument.fire( 'compositionstart' ); + viewDocument.fire( 'keydown', { keyCode: 229 } ); + + expect( getModelData( model ) ).to.equal( 'fo[ob]ar' ); + } ); + + it( 'should not remove contents on compositionstart event if selection is flat', () => { + editor.setData( '

foo bar

' ); + + model.change( writer => { + writer.setSelection( + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 5 ) ); + } ); + + viewDocument.fire( 'compositionstart' ); + + expect( getModelData( model ) ).to.equal( + '<$text bold="true">fo[o <$text italic="true">b]ar' ); + } ); + + it( 'should not remove contents on compositionstart event if selection is flat (no selection)', () => { + editor.setData( '

foo bar

' ); + + const documentSelection = model.document.selection; + + // Create empty selection. + model.document.selection = new ModelSelection(); + + viewDocument.fire( 'compositionstart' ); + + expect( getModelData( model ) ).to.equal( + '<$text bold="true">foo <$text italic="true">bar' ); + + // Restore document selection. + model.document.selection = documentSelection; + } ); + + it( 'should remove contents on compositionstart event if selection is not flat', () => { + editor.setData( '

foo

bar

' ); + + model.change( writer => { + writer.setSelection( + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 1 ), 2 ) ); + } ); + + viewDocument.fire( 'compositionstart' ); + + expect( getModelData( model ) ).to.equal( + '<$text bold="true">fo[]<$text italic="true">r' ); + } ); + + it( 'should not remove contents on keydown event after compositionend event if selection did not change', () => { + editor.setData( '

foo

bar

' ); + + model.change( writer => { + writer.setSelection( + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 1 ), 2 ) ); + } ); + + viewDocument.fire( 'compositionend' ); + viewDocument.fire( 'keydown', { keyCode: 229 } ); + + expect( getModelData( model ) ).to.equal( + '<$text bold="true">fo[o<$text italic="true">ba]r' ); + } ); + + it( 'should remove contents on keydown event after compositionend event if selection have changed', () => { + editor.setData( '

foo

bar

' ); + + model.change( writer => { + writer.setSelection( + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 1 ), 2 ) ); + } ); + + viewDocument.fire( 'compositionend' ); + + model.change( writer => { + writer.setSelection( + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 1 ), 1 ) ); + } ); + + viewDocument.fire( 'keydown', { keyCode: 229 } ); + + expect( getModelData( model ) ).to.equal( + '<$text bold="true">fo[]<$text italic="true">ar' ); + } ); + } ); } ); } ); From 4f4addefa16b97246aa00b8073b1a16df8adf065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Thu, 21 Jun 2018 17:49:25 +0200 Subject: [PATCH 4/8] Remove selection contents on '229' keydown only for non-flat selection. --- src/input.js | 11 ++++++----- tests/input.js | 37 ++++++++++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/input.js b/src/input.js index c36dc78..4da2867 100644 --- a/src/input.js +++ b/src/input.js @@ -101,16 +101,17 @@ export default class Input extends Plugin { return; } - // If not during composition we should also delete content on '229' key code (composition start). - // While composing, deletion should be prevented as it may remove composed sequence (#83). + // If during composition, deletion should be prevented as it may remove composed sequence (#83). if ( isComposing && evtData.keyCode === 229 ) { return; } + const isSelectionFlat = doc.selection.rangeCount === 1 ? doc.selection.getFirstRange().isFlat : true; // If there is a `keydown` event fired with '229' keycode it might be related to recent composition. // Check if selection is the same as upon ending recent composition, if so do not remove selected content - // as it will remove composed sequence (#83). - if ( !isComposing && evtData.keyCode === 229 && isSelectionUnchanged ) { + // as it will remove composed sequence. Also do not remove selection contents if selection is flat as it might + // cause some issues when starting composition. Flat selection is correctly handled by mutation handler itself (#83). + if ( !isComposing && evtData.keyCode === 229 && ( isSelectionUnchanged || isSelectionFlat ) ) { return; } @@ -141,7 +142,7 @@ export default class Input extends Plugin { const isFlatSelection = doc.selection.rangeCount === 1 ? doc.selection.getFirstRange().isFlat : true; // If on `compositionstart` there is a non-collapsed selection which start and end have different block - // parents it means `_handleKeydown()` method did not remove its contents. It happens usually because + // parents it means the `_handleKeydown()` method did not remove its contents. It happens usually because // of different order of events (`compositionstart` before `keydown` - in Safari). In such cases // we need to remove selection contents on composition start (#83). if ( doc.selection.isCollapsed || isFlatSelection ) { diff --git a/tests/input.js b/tests/input.js index 408fd31..7cbba4d 100644 --- a/tests/input.js +++ b/tests/input.js @@ -685,7 +685,7 @@ describe( 'Input feature', () => { } ); describe( '#83', () => { - it( 'should remove contents on composition start key if not during composition', () => { + it( 'should not remove contents on composition start key if not during composition', () => { model.change( writer => { writer.setSelection( ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 4 ) ); @@ -693,7 +693,38 @@ describe( 'Input feature', () => { viewDocument.fire( 'keydown', { keyCode: 229 } ); - expect( getModelData( model ) ).to.equal( 'fo[]ar' ); + expect( getModelData( model ) ).to.equal( 'fo[ob]ar' ); + } ); + + it( 'should not remove contents on composition start key if not during composition and no selection', () => { + editor.setData( '

foo bar

' ); + + const documentSelection = model.document.selection; + + // Create empty selection. + model.document.selection = new ModelSelection(); + + viewDocument.fire( 'keydown', { keyCode: 229 } ); + + expect( getModelData( model ) ).to.equal( + '<$text bold="true">foo <$text italic="true">bar' ); + + // Restore document selection. + model.document.selection = documentSelection; + } ); + + it( 'should remove contents on composition start key if not during composition and selection not flat', () => { + editor.setData( '

foo

bar

' ); + + model.change( writer => { + writer.setSelection( + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 1 ), 2 ) ); + } ); + + viewDocument.fire( 'keydown', { keyCode: 229 } ); + + expect( getModelData( model ) ).to.equal( + '<$text bold="true">fo[]<$text italic="true">r' ); } ); it( 'should not remove contents on composition start key if during composition', () => { @@ -722,7 +753,7 @@ describe( 'Input feature', () => { '<$text bold="true">fo[o <$text italic="true">b]ar' ); } ); - it( 'should not remove contents on compositionstart event if selection is flat (no selection)', () => { + it( 'should not remove contents on compositionstart event if no selection', () => { editor.setData( '

foo bar

' ); const documentSelection = model.document.selection; From d7e6fba848ae445db93224fe35afc0a5bf0dfab9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Fri, 22 Jun 2018 13:26:39 +0200 Subject: [PATCH 5/8] Revert latest changes as they break #83 case. --- src/input.js | 10 ++++------ tests/input.js | 35 ++--------------------------------- 2 files changed, 6 insertions(+), 39 deletions(-) diff --git a/src/input.js b/src/input.js index 4da2867..e1742d5 100644 --- a/src/input.js +++ b/src/input.js @@ -106,12 +106,10 @@ export default class Input extends Plugin { return; } - const isSelectionFlat = doc.selection.rangeCount === 1 ? doc.selection.getFirstRange().isFlat : true; - // If there is a `keydown` event fired with '229' keycode it might be related to recent composition. - // Check if selection is the same as upon ending recent composition, if so do not remove selected content - // as it will remove composed sequence. Also do not remove selection contents if selection is flat as it might - // cause some issues when starting composition. Flat selection is correctly handled by mutation handler itself (#83). - if ( !isComposing && evtData.keyCode === 229 && ( isSelectionUnchanged || isSelectionFlat ) ) { + // If there is a `keydown` event fired with '229' keycode it might be related + // to recent composition. Check if selection is the same as upon ending recent composition, + // if so do not remove selected content as it will remove composed sequence (#83). + if ( !isComposing && evtData.keyCode === 229 && isSelectionUnchanged ) { return; } diff --git a/tests/input.js b/tests/input.js index 7cbba4d..28131c0 100644 --- a/tests/input.js +++ b/tests/input.js @@ -685,7 +685,7 @@ describe( 'Input feature', () => { } ); describe( '#83', () => { - it( 'should not remove contents on composition start key if not during composition', () => { + it( 'should remove contents on composition start key if not during composition', () => { model.change( writer => { writer.setSelection( ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 4 ) ); @@ -693,38 +693,7 @@ describe( 'Input feature', () => { viewDocument.fire( 'keydown', { keyCode: 229 } ); - expect( getModelData( model ) ).to.equal( 'fo[ob]ar' ); - } ); - - it( 'should not remove contents on composition start key if not during composition and no selection', () => { - editor.setData( '

foo bar

' ); - - const documentSelection = model.document.selection; - - // Create empty selection. - model.document.selection = new ModelSelection(); - - viewDocument.fire( 'keydown', { keyCode: 229 } ); - - expect( getModelData( model ) ).to.equal( - '<$text bold="true">foo <$text italic="true">bar' ); - - // Restore document selection. - model.document.selection = documentSelection; - } ); - - it( 'should remove contents on composition start key if not during composition and selection not flat', () => { - editor.setData( '

foo

bar

' ); - - model.change( writer => { - writer.setSelection( - ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 1 ), 2 ) ); - } ); - - viewDocument.fire( 'keydown', { keyCode: 229 } ); - - expect( getModelData( model ) ).to.equal( - '<$text bold="true">fo[]<$text italic="true">r' ); + expect( getModelData( model ) ).to.equal( 'fo[]ar' ); } ); it( 'should not remove contents on composition start key if during composition', () => { From 08b9a28696a6729abd8236c359441f61b63db664 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Fri, 22 Jun 2018 14:19:33 +0200 Subject: [PATCH 6/8] Rewording. --- src/input.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/input.js b/src/input.js index e1742d5..b550c40 100644 --- a/src/input.js +++ b/src/input.js @@ -139,8 +139,8 @@ export default class Input extends Plugin { const buffer = inputCommand.buffer; const isFlatSelection = doc.selection.rangeCount === 1 ? doc.selection.getFirstRange().isFlat : true; - // If on `compositionstart` there is a non-collapsed selection which start and end have different block - // parents it means the `_handleKeydown()` method did not remove its contents. It happens usually because + // If on `compositionstart` there is a non-collapsed selection which start and end have different parents + // it means the `_handleKeydown()` method did not remove its contents. It happens usually because // of different order of events (`compositionstart` before `keydown` - in Safari). In such cases // we need to remove selection contents on composition start (#83). if ( doc.selection.isCollapsed || isFlatSelection ) { @@ -162,9 +162,8 @@ export default class Input extends Plugin { * @private */ _handleCompositionend() { - // Store current selection on `compositionend`. It is then used in `_handleKeydown()` method - // for browsers (Safari) which fire `keydown` event after (and not before) `compositionend` - // to detect if selection content should be removed (#83). + // Store current selection on `compositionend`. It is then used in `_handleKeydown()` + // method to detect if selection content should be removed (#83). this._latestCompositionSelection = new Selection( this.editor.model.document.selection ); } From aabacddd2f4f5ad03682a134f6d4dee71c0fb340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 28 Jun 2018 12:30:44 +0200 Subject: [PATCH 7/8] Moved ChangeBuffer to src/utils/. --- src/deletecommand.js | 2 +- src/inputcommand.js | 6 +++--- src/{ => utils}/changebuffer.js | 2 +- tests/inputcommand.js | 2 +- tests/{ => utils}/changebuffer.js | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) rename src/{ => utils}/changebuffer.js (99%) rename tests/{ => utils}/changebuffer.js (99%) diff --git a/src/deletecommand.js b/src/deletecommand.js index 29ddf6f..5b2e0b0 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -11,7 +11,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import Selection from '@ckeditor/ckeditor5-engine/src/model/selection'; import Element from '@ckeditor/ckeditor5-engine/src/model/element'; import Range from '@ckeditor/ckeditor5-engine/src/model/range'; -import ChangeBuffer from './changebuffer'; +import ChangeBuffer from './utils/changebuffer'; import count from '@ckeditor/ckeditor5-utils/src/count'; /** diff --git a/src/inputcommand.js b/src/inputcommand.js index 9bdf3de..aab7557 100644 --- a/src/inputcommand.js +++ b/src/inputcommand.js @@ -8,7 +8,7 @@ */ import Command from '@ckeditor/ckeditor5-core/src/command'; -import ChangeBuffer from './changebuffer'; +import ChangeBuffer from './utils/changebuffer'; /** * The input command. Used by the {@link module:typing/input~Input input feature} to handle typing. @@ -31,7 +31,7 @@ export default class InputCommand extends Command { * * @readonly * @private - * @member {module:typing/changebuffer~ChangeBuffer} #_buffer + * @member {module:typing/utils/changebuffer~ChangeBuffer} #_buffer */ this._buffer = new ChangeBuffer( editor.model, undoStepSize ); } @@ -39,7 +39,7 @@ export default class InputCommand extends Command { /** * The current change buffer. * - * @type {module:typing/changebuffer~ChangeBuffer} + * @type {module:typing/utils/changebuffer~ChangeBuffer} */ get buffer() { return this._buffer; diff --git a/src/changebuffer.js b/src/utils/changebuffer.js similarity index 99% rename from src/changebuffer.js rename to src/utils/changebuffer.js index e9ef97a..acf5eb0 100644 --- a/src/changebuffer.js +++ b/src/utils/changebuffer.js @@ -4,7 +4,7 @@ */ /** - * @module typing/changebuffer + * @module typing/utils/changebuffer */ import Batch from '@ckeditor/ckeditor5-engine/src/model/batch'; diff --git a/tests/inputcommand.js b/tests/inputcommand.js index 59889a2..e3ca8eb 100644 --- a/tests/inputcommand.js +++ b/tests/inputcommand.js @@ -10,7 +10,7 @@ import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model import Range from '@ckeditor/ckeditor5-engine/src/model/range'; import Position from '@ckeditor/ckeditor5-engine/src/model/position'; import InputCommand from '../src/inputcommand'; -import ChangeBuffer from '../src/changebuffer'; +import ChangeBuffer from '../src/utils/changebuffer'; import Input from '../src/input'; describe( 'InputCommand', () => { diff --git a/tests/changebuffer.js b/tests/utils/changebuffer.js similarity index 99% rename from tests/changebuffer.js rename to tests/utils/changebuffer.js index e1daef0..e3dd765 100644 --- a/tests/changebuffer.js +++ b/tests/utils/changebuffer.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md. */ -import ChangeBuffer from '../src/changebuffer'; +import ChangeBuffer from '../../src/utils/changebuffer'; import Model from '@ckeditor/ckeditor5-engine/src/model/model'; import Batch from '@ckeditor/ckeditor5-engine/src/model/batch'; From 093d0376e3e6e42711a0f0710a86e1880dedb72d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 28 Jun 2018 14:11:39 +0200 Subject: [PATCH 8/8] =?UTF-8?q?Major=20refactoring=20of=20the=20Input=20pl?= =?UTF-8?q?ugin=20=E2=80=93=20extracted=20all=20the=20codee=20to=20separat?= =?UTF-8?q?e=20utils.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/input.js | 549 +------------------- src/utils/injecttypingmutationshandling.js | 387 ++++++++++++++ src/utils/injectunsafekeystrokeshandling.js | 153 ++++++ 3 files changed, 546 insertions(+), 543 deletions(-) create mode 100644 src/utils/injecttypingmutationshandling.js create mode 100644 src/utils/injectunsafekeystrokeshandling.js diff --git a/src/input.js b/src/input.js index b550c40..2fa4c9e 100644 --- a/src/input.js +++ b/src/input.js @@ -8,16 +8,11 @@ */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import Selection from '@ckeditor/ckeditor5-engine/src/model/selection'; -import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range'; -import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position'; -import ViewText from '@ckeditor/ckeditor5-engine/src/view/text'; -import diff from '@ckeditor/ckeditor5-utils/src/diff'; -import diffToChanges from '@ckeditor/ckeditor5-utils/src/difftochanges'; -import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard'; -import DomConverter from '@ckeditor/ckeditor5-engine/src/view/domconverter'; import InputCommand from './inputcommand'; +import injectUnsafeKeystrokesHandling from './utils/injectunsafekeystrokeshandling'; +import injectTypingMutationsHandling from './utils/injecttypingmutationshandling'; + /** * Handles text input coming from the keyboard or other input methods. * @@ -36,545 +31,13 @@ export default class Input extends Plugin { */ init() { const editor = this.editor; - const editingView = editor.editing.view; - const inputCommand = new InputCommand( editor, editor.config.get( 'typing.undoStep' ) || 20 ); // TODO The above default configuration value should be defined using editor.config.define() once it's fixed. - - this._latestCompositionSelection = null; + const inputCommand = new InputCommand( editor, editor.config.get( 'typing.undoStep' ) || 20 ); editor.commands.add( 'input', inputCommand ); - this.listenTo( editingView.document, 'keydown', ( evt, data ) => { - this._handleKeydown( data, inputCommand ); - }, { priority: 'lowest' } ); - - this.listenTo( editingView.document, 'compositionstart', () => { - this._handleCompositionstart( inputCommand ); - }, { priority: 'lowest' } ); - - this.listenTo( editingView.document, 'compositionend', () => { - this._handleCompositionend(); - }, { priority: 'lowest' } ); - - this.listenTo( editingView.document, 'mutations', ( evt, mutations, viewSelection ) => { - 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. delete or enter features) - * to handle the event. - * - * @private - * @param {module:engine/view/observer/keyobserver~KeyEventData} evtData - * @param {module:typing/inputcommand~InputCommand} inputCommand - */ - _handleKeydown( evtData, inputCommand ) { - const model = this.editor.model; - const doc = model.document; - const buffer = inputCommand.buffer; - const isComposing = this.editor.editing.view.document.isComposing; - const isSelectionUnchanged = this._latestCompositionSelection && this._latestCompositionSelection.isEqual( doc.selection ); - - // Reset stored composition selection. - this._latestCompositionSelection = null; - - // By relying on the state of the input command we allow disabling the entire input easily - // by just disabling the input command. We could’ve used here the delete command but that - // would mean requiring the delete feature which would block loading one without the other. - // We could also check the editor.isReadOnly property, but that wouldn't allow to block - // the input without blocking other features. - if ( !inputCommand.isEnabled ) { - return; - } - - if ( isSafeKeystroke( evtData ) || doc.selection.isCollapsed ) { - return; - } - - // If during composition, deletion should be prevented as it may remove composed sequence (#83). - if ( isComposing && evtData.keyCode === 229 ) { - return; - } - - // If there is a `keydown` event fired with '229' keycode it might be related - // to recent composition. Check if selection is the same as upon ending recent composition, - // if so do not remove selected content as it will remove composed sequence (#83). - if ( !isComposing && evtData.keyCode === 229 && isSelectionUnchanged ) { - return; - } - - buffer.lock(); - - model.enqueueChange( buffer.batch, () => { - this.editor.model.deleteContent( doc.selection ); - } ); - - buffer.unlock(); - } - - /** - * Handles the `compositionstart` event. It is used only in special cases to remove the contents - * of a non-collapsed selection so composition itself does not result in complex mutations. - * - * The special case mentioned above is a situation in which the `keydown` event is fired after - * `compositionstart` event. In such cases {@link #_handleKeydown} cannot clear current selection - * contents (because it is too late and will break the composition) so the composition handler takes care of it. - * - * @private - * @param {module:typing/inputcommand~InputCommand} inputCommand - */ - _handleCompositionstart( inputCommand ) { - const model = this.editor.model; - const doc = model.document; - const buffer = inputCommand.buffer; - const isFlatSelection = doc.selection.rangeCount === 1 ? doc.selection.getFirstRange().isFlat : true; - - // If on `compositionstart` there is a non-collapsed selection which start and end have different parents - // it means the `_handleKeydown()` method did not remove its contents. It happens usually because - // of different order of events (`compositionstart` before `keydown` - in Safari). In such cases - // we need to remove selection contents on composition start (#83). - if ( doc.selection.isCollapsed || isFlatSelection ) { - return; - } - - buffer.lock(); - - model.enqueueChange( buffer.batch, () => { - this.editor.model.deleteContent( doc.selection ); - } ); - - buffer.unlock(); - } - - /** - * Handles `compositionend` event. It is used to store latest composition selection. - * - * @private - */ - _handleCompositionend() { - // Store current selection on `compositionend`. It is then used in `_handleKeydown()` - // method to detect if selection content should be removed (#83). - this._latestCompositionSelection = new Selection( this.editor.model.document.selection ); - } - - /** - * Handles DOM mutations. - * - * @private - * @param {Array.} mutations - * @param {module:engine/view/selection~Selection|null} viewSelection - */ - _handleMutations( mutations, viewSelection ) { - new MutationHandler( this.editor ).handle( mutations, viewSelection ); - } -} - -/** - * Helper class for translating DOM mutations into model changes. - * - * @private - */ -class MutationHandler { - /** - * Creates an instance of the mutation handler. - * - * @param {module:core/editor/editor~Editor} editor - */ - constructor( editor ) { - /** - * Editor instance for which mutations are handled. - * - * @readonly - * @member {module:core/editor/editor~Editor} #editor - */ - this.editor = editor; - - /** - * The editing controller. - * - * @readonly - * @member {module:engine/controller/editingcontroller~EditingController} #editing - */ - this.editing = this.editor.editing; - } - - /** - * Handles given mutations. - * - * @param {Array.} mutations - * @param {module:engine/view/selection~Selection|null} viewSelection - */ - handle( mutations, viewSelection ) { - if ( containerChildrenMutated( mutations ) ) { - this._handleContainerChildrenMutations( mutations, viewSelection ); - } else { - for ( const mutation of mutations ) { - // Fortunately it will never be both. - this._handleTextMutation( mutation, viewSelection ); - this._handleTextNodeInsertion( mutation ); - } - } - } - - /** - * Handles situations when container's children mutated during input. This can happen when - * the browser is trying to "fix" DOM in certain situations. For example, when the user starts to type - * in `

Link{}

`, the browser might change the order of elements - * to `

Linkx{}

`. A similar situation happens when the spell checker - * replaces a word wrapped with `` with a word wrapped with a `` element. - * - * To handle such situations, the common DOM ancestor of all mutations is converted to the model representation - * and then compared with the current model to calculate the proper text change. - * - * Note: Single text node insertion is handled in {@link #_handleTextNodeInsertion} and text node mutation is handled - * in {@link #_handleTextMutation}). - * - * @private - * @param {Array.} mutations - * @param {module:engine/view/selection~Selection|null} viewSelection - */ - _handleContainerChildrenMutations( mutations, viewSelection ) { - // Get common ancestor of all mutations. - const mutationsCommonAncestor = getMutationsContainer( mutations ); - - // Quit if there is no common ancestor. - if ( !mutationsCommonAncestor ) { - return; - } - - const domConverter = this.editor.editing.view.domConverter; - - // Get common ancestor in DOM. - const domMutationCommonAncestor = domConverter.mapViewToDom( mutationsCommonAncestor ); - - // 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(); - const modelFromCurrentDom = this.editor.data.toModel( - freshDomConverter.domToView( domMutationCommonAncestor ) - ).getChild( 0 ); - - // 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 `
    ` is a common ancestor, `currentModel` is `undefined` - // because `
      ` is not mapped (`
    • `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() ); - - // Remove the last `` from the end of `modelFromDomChildren` if there is no `` in current model. - // If the described scenario happened, it means that this is a bogus `
      ` added by a browser. - const lastDomChild = modelFromDomChildren[ modelFromDomChildren.length - 1 ]; - const lastCurrentChild = currentModelChildren[ currentModelChildren.length - 1 ]; - - if ( lastDomChild && lastDomChild.is( 'softBreak' ) && lastCurrentChild && !lastCurrentChild.is( 'softBreak' ) ) { - modelFromDomChildren.pop(); - } - - // Skip situations when common ancestor has any container elements. - if ( !isSafeForTextMutation( modelFromDomChildren ) || !isSafeForTextMutation( currentModelChildren ) ) { - return; - } - - // Replace   inserted by the browser with normal space. See comment in `_handleTextMutation`. - // Replace non-texts with any character. This is potentially dangerous but passes in manual tests. The thing is - // that we need to take care of proper indexes so we cannot simply remove non-text elements from the content. - // By inserting a character we keep all the real texts on their indexes. - const newText = modelFromDomChildren.map( item => item.is( 'text' ) ? item.data : '@' ).join( '' ).replace( /\u00A0/g, ' ' ); - const oldText = currentModelChildren.map( item => item.is( 'text' ) ? item.data : '@' ).join( '' ); - - // Do nothing if mutations created same text. - if ( oldText === newText ) { - return; - } - - const diffResult = diff( oldText, newText ); - - const { firstChangeAt, insertions, deletions } = calculateChanges( diffResult ); - - // Try setting new model selection according to passed view selection. - let modelSelectionRange = null; - - if ( viewSelection ) { - modelSelectionRange = this.editing.mapper.toModelRange( viewSelection.getFirstRange() ); - } - - const insertText = newText.substr( firstChangeAt, insertions ); - const removeRange = ModelRange.createFromParentsAndOffsets( - currentModel, - firstChangeAt, - currentModel, - firstChangeAt + deletions - ); - - this.editor.execute( 'input', { - text: insertText, - range: removeRange, - resultRange: modelSelectionRange - } ); + injectUnsafeKeystrokesHandling( editor ); + injectTypingMutationsHandling( editor ); } - - /** - * @private - */ - _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 ); - - const { firstChangeAt, insertions, deletions } = calculateChanges( diffResult ); - - // Try setting new model selection according to passed view selection. - let modelSelectionRange = null; - - if ( viewSelection ) { - modelSelectionRange = this.editing.mapper.toModelRange( viewSelection.getFirstRange() ); - } - - // 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 ); - const removeRange = ModelRange.createFromPositionAndShift( modelPos, deletions ); - const insertText = newText.substr( firstChangeAt, insertions ); - - this.editor.execute( 'input', { - text: insertText, - range: removeRange, - resultRange: modelSelectionRange - } ); - } - - /** - * @private - */ - _handleTextNodeInsertion( mutation ) { - if ( mutation.type != 'children' ) { - return; - } - - const change = getSingleTextNodeChange( mutation ); - const viewPos = new ViewPosition( mutation.node, change.index ); - const modelPos = this.editing.mapper.toModelPosition( viewPos ); - const insertedText = change.values[ 0 ].data; - - this.editor.execute( 'input', { - // 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. - text: insertedText.replace( /\u00A0/g, ' ' ), - range: new ModelRange( modelPos ) - } ); - } -} - -const safeKeycodes = [ - getCode( 'arrowUp' ), - getCode( 'arrowRight' ), - getCode( 'arrowDown' ), - getCode( 'arrowLeft' ), - 9, // Tab - 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. -// -// @private -// @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; - } -} - -// Returns change made to a single text node. Returns `undefined` if more than a single text node was changed. -// -// @private -// @param mutation -function getSingleTextNodeChange( mutation ) { - // 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; - } - - return change; -} - -// Returns first common ancestor of all mutations that is either {@link module:engine/view/containerelement~ContainerElement} -// or {@link module:engine/view/rootelement~RootElement}. -// -// @private -// @param {Array.} mutations -// @returns {module:engine/view/containerelement~ContainerElement|engine/view/rootelement~RootElement|undefined} -function getMutationsContainer( mutations ) { - const lca = mutations - .map( mutation => mutation.node ) - .reduce( ( commonAncestor, node ) => { - return commonAncestor.getCommonAncestor( node, { includeSelf: true } ); - } ); - - if ( !lca ) { - return; - } - - // We need to look for container and root elements only, so check all LCA's - // ancestors (starting from itself). - return lca.getAncestors( { includeSelf: true, parentFirst: true } ) - .find( element => element.is( 'containerElement' ) || element.is( 'rootElement' ) ); -} - -// Returns true if container children have mutated or more than a single text node was changed. -// -// Single text node child insertion is handled in {@link module:typing/input~MutationHandler#_handleTextNodeInsertion} -// while text mutation is handled in {@link module:typing/input~MutationHandler#_handleTextMutation}. -// -// @private -// @param {Array.} mutations -// @returns {Boolean} -function containerChildrenMutated( mutations ) { - if ( mutations.length == 0 ) { - return false; - } - - // Check if there is any mutation of `children` type or any mutation that changes more than one text node. - for ( const mutation of mutations ) { - if ( mutation.type === 'children' && !getSingleTextNodeChange( mutation ) ) { - return true; - } - } - - return false; -} - -// Returns true if provided array contains content that won't be problematic during diffing and text mutation handling. -// -// @param {Array.} children -// @returns {Boolean} -function isSafeForTextMutation( children ) { - return children.every( child => child.is( 'text' ) || child.is( 'softBreak' ) ); -} - -// Calculates first change index and number of characters that should be inserted and deleted starting from that index. -// -// @private -// @param diffResult -// @returns {{insertions: number, deletions: number, firstChangeAt: *}} -function calculateChanges( diffResult ) { - // 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. - 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++; - } - } - - return { insertions, deletions, firstChangeAt }; } diff --git a/src/utils/injecttypingmutationshandling.js b/src/utils/injecttypingmutationshandling.js new file mode 100644 index 0000000..ed19e19 --- /dev/null +++ b/src/utils/injecttypingmutationshandling.js @@ -0,0 +1,387 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module typing/utils/injecttypingmutationshandling + */ + +import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range'; +import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position'; +import ViewText from '@ckeditor/ckeditor5-engine/src/view/text'; +import diff from '@ckeditor/ckeditor5-utils/src/diff'; +import diffToChanges from '@ckeditor/ckeditor5-utils/src/difftochanges'; +import DomConverter from '@ckeditor/ckeditor5-engine/src/view/domconverter'; + +/** + * Handles mutations caused by normal typing. + * + * @param {module:core/editor/editor~Editor} editor The editor instance. + */ +export default function injectTypingMutationsHandling( editor ) { + editor.editing.view.document.on( 'mutations', ( evt, mutations, viewSelection ) => { + new MutationHandler( editor ).handle( mutations, viewSelection ); + } ); +} + +/** + * Helper class for translating DOM mutations into model changes. + * + * @private + */ +class MutationHandler { + /** + * Creates an instance of the mutation handler. + * + * @param {module:core/editor/editor~Editor} editor + */ + constructor( editor ) { + /** + * Editor instance for which mutations are handled. + * + * @readonly + * @member {module:core/editor/editor~Editor} #editor + */ + this.editor = editor; + + /** + * The editing controller. + * + * @readonly + * @member {module:engine/controller/editingcontroller~EditingController} #editing + */ + this.editing = this.editor.editing; + } + + /** + * Handles given mutations. + * + * @param {Array.} mutations + * @param {module:engine/view/selection~Selection|null} viewSelection + */ + handle( mutations, viewSelection ) { + if ( containerChildrenMutated( mutations ) ) { + this._handleContainerChildrenMutations( mutations, viewSelection ); + } else { + for ( const mutation of mutations ) { + // Fortunately it will never be both. + this._handleTextMutation( mutation, viewSelection ); + this._handleTextNodeInsertion( mutation ); + } + } + } + + /** + * Handles situations when container's children mutated during input. This can happen when + * the browser is trying to "fix" DOM in certain situations. For example, when the user starts to type + * in `

      Link{}

      `, the browser might change the order of elements + * to `

      Linkx{}

      `. A similar situation happens when the spell checker + * replaces a word wrapped with `` with a word wrapped with a `` element. + * + * To handle such situations, the common DOM ancestor of all mutations is converted to the model representation + * and then compared with the current model to calculate the proper text change. + * + * Note: Single text node insertion is handled in {@link #_handleTextNodeInsertion} and text node mutation is handled + * in {@link #_handleTextMutation}). + * + * @private + * @param {Array.} mutations + * @param {module:engine/view/selection~Selection|null} viewSelection + */ + _handleContainerChildrenMutations( mutations, viewSelection ) { + // Get common ancestor of all mutations. + const mutationsCommonAncestor = getMutationsContainer( mutations ); + + // Quit if there is no common ancestor. + if ( !mutationsCommonAncestor ) { + return; + } + + const domConverter = this.editor.editing.view.domConverter; + + // Get common ancestor in DOM. + const domMutationCommonAncestor = domConverter.mapViewToDom( mutationsCommonAncestor ); + + // 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(); + const modelFromCurrentDom = this.editor.data.toModel( + freshDomConverter.domToView( domMutationCommonAncestor ) + ).getChild( 0 ); + + // 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 `
        ` is a common ancestor, `currentModel` is `undefined` + // because `
          ` is not mapped (`
        • `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() ); + + // Remove the last `` from the end of `modelFromDomChildren` if there is no `` in current model. + // If the described scenario happened, it means that this is a bogus `
          ` added by a browser. + const lastDomChild = modelFromDomChildren[ modelFromDomChildren.length - 1 ]; + const lastCurrentChild = currentModelChildren[ currentModelChildren.length - 1 ]; + + if ( lastDomChild && lastDomChild.is( 'softBreak' ) && lastCurrentChild && !lastCurrentChild.is( 'softBreak' ) ) { + modelFromDomChildren.pop(); + } + + // Skip situations when common ancestor has any container elements. + if ( !isSafeForTextMutation( modelFromDomChildren ) || !isSafeForTextMutation( currentModelChildren ) ) { + return; + } + + // Replace   inserted by the browser with normal space. See comment in `_handleTextMutation`. + // Replace non-texts with any character. This is potentially dangerous but passes in manual tests. The thing is + // that we need to take care of proper indexes so we cannot simply remove non-text elements from the content. + // By inserting a character we keep all the real texts on their indexes. + const newText = modelFromDomChildren.map( item => item.is( 'text' ) ? item.data : '@' ).join( '' ).replace( /\u00A0/g, ' ' ); + const oldText = currentModelChildren.map( item => item.is( 'text' ) ? item.data : '@' ).join( '' ); + + // Do nothing if mutations created same text. + if ( oldText === newText ) { + return; + } + + const diffResult = diff( oldText, newText ); + + const { firstChangeAt, insertions, deletions } = calculateChanges( diffResult ); + + // Try setting new model selection according to passed view selection. + let modelSelectionRange = null; + + if ( viewSelection ) { + modelSelectionRange = this.editing.mapper.toModelRange( viewSelection.getFirstRange() ); + } + + const insertText = newText.substr( firstChangeAt, insertions ); + const removeRange = ModelRange.createFromParentsAndOffsets( + currentModel, + firstChangeAt, + currentModel, + firstChangeAt + deletions + ); + + this.editor.execute( 'input', { + text: insertText, + range: removeRange, + resultRange: modelSelectionRange + } ); + } + + /** + * @private + */ + _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 ); + + const { firstChangeAt, insertions, deletions } = calculateChanges( diffResult ); + + // Try setting new model selection according to passed view selection. + let modelSelectionRange = null; + + if ( viewSelection ) { + modelSelectionRange = this.editing.mapper.toModelRange( viewSelection.getFirstRange() ); + } + + // 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 ); + const removeRange = ModelRange.createFromPositionAndShift( modelPos, deletions ); + const insertText = newText.substr( firstChangeAt, insertions ); + + this.editor.execute( 'input', { + text: insertText, + range: removeRange, + resultRange: modelSelectionRange + } ); + } + + /** + * @private + */ + _handleTextNodeInsertion( mutation ) { + if ( mutation.type != 'children' ) { + return; + } + + const change = getSingleTextNodeChange( mutation ); + const viewPos = new ViewPosition( mutation.node, change.index ); + const modelPos = this.editing.mapper.toModelPosition( viewPos ); + const insertedText = change.values[ 0 ].data; + + this.editor.execute( 'input', { + // 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. + text: insertedText.replace( /\u00A0/g, ' ' ), + range: new ModelRange( modelPos ) + } ); + } +} + +// 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; + } +} + +// Returns change made to a single text node. Returns `undefined` if more than a single text node was changed. +// +// @private +// @param mutation +function getSingleTextNodeChange( mutation ) { + // 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; + } + + return change; +} + +// Returns first common ancestor of all mutations that is either {@link module:engine/view/containerelement~ContainerElement} +// or {@link module:engine/view/rootelement~RootElement}. +// +// @private +// @param {Array.} mutations +// @returns {module:engine/view/containerelement~ContainerElement|engine/view/rootelement~RootElement|undefined} +function getMutationsContainer( mutations ) { + const lca = mutations + .map( mutation => mutation.node ) + .reduce( ( commonAncestor, node ) => { + return commonAncestor.getCommonAncestor( node, { includeSelf: true } ); + } ); + + if ( !lca ) { + return; + } + + // We need to look for container and root elements only, so check all LCA's + // ancestors (starting from itself). + return lca.getAncestors( { includeSelf: true, parentFirst: true } ) + .find( element => element.is( 'containerElement' ) || element.is( 'rootElement' ) ); +} + +// Returns true if container children have mutated or more than a single text node was changed. +// +// Single text node child insertion is handled in {@link module:typing/input~MutationHandler#_handleTextNodeInsertion} +// while text mutation is handled in {@link module:typing/input~MutationHandler#_handleTextMutation}. +// +// @private +// @param {Array.} mutations +// @returns {Boolean} +function containerChildrenMutated( mutations ) { + if ( mutations.length == 0 ) { + return false; + } + + // Check if there is any mutation of `children` type or any mutation that changes more than one text node. + for ( const mutation of mutations ) { + if ( mutation.type === 'children' && !getSingleTextNodeChange( mutation ) ) { + return true; + } + } + + return false; +} + +// Returns true if provided array contains content that won't be problematic during diffing and text mutation handling. +// +// @param {Array.} children +// @returns {Boolean} +function isSafeForTextMutation( children ) { + return children.every( child => child.is( 'text' ) || child.is( 'softBreak' ) ); +} + +// Calculates first change index and number of characters that should be inserted and deleted starting from that index. +// +// @private +// @param diffResult +// @returns {{insertions: number, deletions: number, firstChangeAt: *}} +function calculateChanges( diffResult ) { + // 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. + 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++; + } + } + + return { insertions, deletions, firstChangeAt }; +} diff --git a/src/utils/injectunsafekeystrokeshandling.js b/src/utils/injectunsafekeystrokeshandling.js new file mode 100644 index 0000000..a6f38d1 --- /dev/null +++ b/src/utils/injectunsafekeystrokeshandling.js @@ -0,0 +1,153 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module typing/utils/injectunsafekeystrokeshandling + */ + +import Selection from '@ckeditor/ckeditor5-engine/src/model/selection'; + +import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard'; + +/** + * Handles keystrokes which are unsafe for typing. This handler's logic is explained + * in https://github.com/ckeditor/ckeditor5-typing/issues/83#issuecomment-398690251. + * + * @param {module:core/editor/editor~Editor} editor The editor instance. + */ +export default function injectUnsafeKeystrokesHandling( editor ) { + let latestCompositionSelection = null; + + const model = editor.model; + const view = editor.editing.view; + const inputCommand = editor.commands.get( 'input' ); + + view.document.on( 'keydown', ( evt, evtData ) => handleKeydown( evtData ), { priority: 'lowest' } ); + + view.document.on( 'compositionstart', handleCompositionStart, { priority: 'lowest' } ); + + view.document.on( 'compositionend', () => { + latestCompositionSelection = new Selection( model.document.selection ); + }, { priority: 'lowest' } ); + + // 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. + // + // @param {module:engine/view/observer/keyobserver~KeyEventData} evtData + function handleKeydown( evtData ) { + const doc = model.document; + const isComposing = view.document.isComposing; + const isSelectionUnchanged = latestCompositionSelection && latestCompositionSelection.isEqual( doc.selection ); + + // Reset stored composition selection. + latestCompositionSelection = null; + + // By relying on the state of the input command we allow disabling the entire input easily + // by just disabling the input command. We could’ve used here the delete command but that + // would mean requiring the delete feature which would block loading one without the other. + // We could also check the editor.isReadOnly property, but that wouldn't allow to block + // the input without blocking other features. + if ( !inputCommand.isEnabled ) { + return; + } + + if ( isSafeKeystroke( evtData ) || doc.selection.isCollapsed ) { + return; + } + + // If during composition, deletion should be prevented as it may remove composed sequence (#83). + if ( isComposing && evtData.keyCode === 229 ) { + return; + } + + // If there is a `keydown` event fired with '229' keycode it might be related + // to recent composition. Check if selection is the same as upon ending recent composition, + // if so do not remove selected content as it will remove composed sequence (#83). + if ( !isComposing && evtData.keyCode === 229 && isSelectionUnchanged ) { + return; + } + + deleteSelectionContent(); + } + + // Handles the `compositionstart` event. It is used only in special cases to remove the contents + // of a non-collapsed selection so composition itself does not result in complex mutations. + // + // The special case mentioned above is a situation in which the `keydown` event is fired after + // `compositionstart` event. In such cases {@link #handleKeydown} cannot clear current selection + // contents (because it is too late and will break the composition) so the composition handler takes care of it. + function handleCompositionStart() { + const doc = model.document; + const isFlatSelection = doc.selection.rangeCount === 1 ? doc.selection.getFirstRange().isFlat : true; + + // If on `compositionstart` there is a non-collapsed selection which start and end have different parents + // it means the `handleKeydown()` method did not remove its contents. It happens usually because + // of different order of events (`compositionstart` before `keydown` - in Safari). In such cases + // we need to remove selection contents on composition start (#83). + if ( doc.selection.isCollapsed || isFlatSelection ) { + return; + } + + deleteSelectionContent(); + } + + function deleteSelectionContent() { + const buffer = inputCommand.buffer; + + buffer.lock(); + + model.enqueueChange( buffer.batch, () => { + model.deleteContent( model.document.selection ); + } ); + + buffer.unlock(); + } +} + +const safeKeycodes = [ + getCode( 'arrowUp' ), + getCode( 'arrowRight' ), + getCode( 'arrowDown' ), + getCode( 'arrowLeft' ), + 9, // Tab + 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. +// +// @private +// @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 ); +}