diff --git a/src/view/renderer.js b/src/view/renderer.js index 763a4fc3a..6ceabe63b 100644 --- a/src/view/renderer.js +++ b/src/view/renderer.js @@ -9,12 +9,14 @@ import ViewText from './text'; import ViewPosition from './position'; +import Selection from './selection'; import { INLINE_FILLER, INLINE_FILLER_LENGTH, startsWithFiller, isInlineFiller, isBlockFiller } from './filler'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; import diff from '@ckeditor/ckeditor5-utils/src/diff'; import insertAt from '@ckeditor/ckeditor5-utils/src/dom/insertat'; import remove from '@ckeditor/ckeditor5-utils/src/dom/remove'; +import log from '@ckeditor/ckeditor5-utils/src/log'; import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; @@ -580,6 +582,17 @@ export default class Renderer { return; } + if ( oldViewSelection && areSimilarSelections( oldViewSelection, this.selection ) ) { + const data = { + oldSelection: oldViewSelection, + currentSelection: this.selection + }; + + log.warn( 'renderer-skipped-selection-rendering: The selection was not rendered due to its similarity to the current one.', data ); + + return; + } + // Multi-range selection is not available in most browsers, and, at least in Chrome, trying to // set such selection, that is not continuous, throws an error. Because of that, we will just use anchor // and focus of view selection. @@ -642,3 +655,35 @@ export default class Renderer { } mix( Renderer, ObservableMixin ); + +// Checks if two given selections are similar. Selections are considered similar if they are non-collapsed +// and their trimmed (see {@link #_trimSelection}) representations are equal. +// +// @private +// @param {module:engine/view/selection~Selection} selection1 +// @param {module:engine/view/selection~Selection} selection2 +// @returns {Boolean} +function areSimilarSelections( selection1, selection2 ) { + return !selection1.isCollapsed && trimSelection( selection1 ).isEqual( trimSelection( selection2 ) ); +} + +// Creates a copy of a given selection with all of its ranges +// trimmed (see {@link module:engine/view/range~Range#getTrimmed getTrimmed}). +// +// @private +// @param {module:engine/view/selection~Selection} selection +// @returns {module:engine/view/selection~Selection} Selection copy with all ranges trimmed. +function trimSelection( selection ) { + const newSelection = Selection.createFromSelection( selection ); + const ranges = newSelection.getRanges(); + + let trimmedRanges = []; + + for ( let range of ranges ) { + trimmedRanges.push( range.getTrimmed() ); + } + + newSelection.setRanges( trimmedRanges, newSelection.isBackward ); + + return newSelection; +} diff --git a/tests/manual/tickets/880/1.html b/tests/manual/tickets/880/1.html new file mode 100644 index 000000000..bf383218e --- /dev/null +++ b/tests/manual/tickets/880/1.html @@ -0,0 +1,3 @@ +
+

This is an editor instance.

+
diff --git a/tests/manual/tickets/880/1.js b/tests/manual/tickets/880/1.js new file mode 100644 index 000000000..d7a64c342 --- /dev/null +++ b/tests/manual/tickets/880/1.js @@ -0,0 +1,27 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals console, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; +import EssentialsPreset from '@ckeditor/ckeditor5-presets/src/essentials'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; + +ClassicEditor.create( document.querySelector( '#editor' ), { + plugins: [ EssentialsPreset, Paragraph, Bold ], + toolbar: [ 'undo', 'redo' ] +} ) +.then( editor => { + window.editor = editor; + + editor.editing.view.on( 'selectionChange', () => { + editor.document.enqueueChanges( () => {} ); + console.log( 'selectionChange', ( new Date() ).getTime() ); + } ); +} ) +.catch( err => { + console.error( err.stack ); +} ); diff --git a/tests/manual/tickets/880/1.md b/tests/manual/tickets/880/1.md new file mode 100644 index 000000000..6603c1c08 --- /dev/null +++ b/tests/manual/tickets/880/1.md @@ -0,0 +1,14 @@ +## Renderer - infinite `selectionChange` event + +Place the selection like this: + +``` +This is {an editor} instance. +``` + +(it must end at the end of the inline style) + +**Expected**: + +* Every time selection is changed, console log with `selectionChange` is printed once. +* While creating selection from **2.**, there might be `renderer-selection-similar` warning visible in the console. diff --git a/tests/manual/tickets/887/1.html b/tests/manual/tickets/887/1.html new file mode 100644 index 000000000..bf383218e --- /dev/null +++ b/tests/manual/tickets/887/1.html @@ -0,0 +1,3 @@ +
+

This is an editor instance.

+
diff --git a/tests/manual/tickets/887/1.js b/tests/manual/tickets/887/1.js new file mode 100644 index 000000000..c18cba015 --- /dev/null +++ b/tests/manual/tickets/887/1.js @@ -0,0 +1,22 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals console, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; +import EssentialsPreset from '@ckeditor/ckeditor5-presets/src/essentials'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; + +ClassicEditor.create( document.querySelector( '#editor' ), { + plugins: [ EssentialsPreset, Paragraph, Bold ], + toolbar: [ 'undo', 'redo' ] +} ) +.then( editor => { + window.editor = editor; +} ) +.catch( err => { + console.error( err.stack ); +} ); diff --git a/tests/manual/tickets/887/1.md b/tests/manual/tickets/887/1.md new file mode 100644 index 000000000..0742fe384 --- /dev/null +++ b/tests/manual/tickets/887/1.md @@ -0,0 +1,10 @@ +## Renderer - MacOS accent balloon on inline element boundary + +1. Place the selection like this: + + `This is an editor{} instance.` +2. Open MacOS accent balloon (e.g. long `a` press). +3. Navigate through the balloon panel using arrow keys. + +**Expected**: It is possible to navigate with arrow keys inside the MacOS balloon panel. While navigating, there +might be `renderer-selection-similar` warning visible in the console. diff --git a/tests/view/renderer.js b/tests/view/renderer.js index 9e37f37e8..8d6fb3568 100644 --- a/tests/view/renderer.js +++ b/tests/view/renderer.js @@ -8,6 +8,7 @@ import ViewElement from '../../src/view/element'; import ViewText from '../../src/view/text'; import ViewRange from '../../src/view/range'; +import ViewPosition from '../../src/view/position'; import Selection from '../../src/view/selection'; import DomConverter from '../../src/view/domconverter'; import Renderer from '../../src/view/renderer'; @@ -16,6 +17,7 @@ import { parse } from '../../src/dev-utils/view'; import { INLINE_FILLER, INLINE_FILLER_LENGTH, isBlockFiller, BR_FILLER } from '../../src/view/filler'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement'; +import log from '@ckeditor/ckeditor5-utils/src/log'; testUtils.createSinonSandbox(); @@ -1349,6 +1351,297 @@ describe( 'Renderer', () => { expect( bindSelection.isEqual( selection ) ).to.be.true; } ); } ); + + // #887 + describe( 'similar selection', () => { + // Use spies to check selection updates. Some selection positions are not achievable in some + // browsers (e.g.

Foo{}Bar

in Chrome) so asserting dom selection after rendering would fail. + let selectionCollapseSpy, selectionExtendSpy, logWarnStub; + + before( () => { + logWarnStub = sinon.stub( log, 'warn' ); + } ); + + afterEach( () => { + if ( selectionCollapseSpy ) { + selectionCollapseSpy.restore(); + selectionCollapseSpy = null; + } + + if ( selectionExtendSpy ) { + selectionExtendSpy.restore(); + selectionExtendSpy = null; + } + logWarnStub.reset(); + } ); + + after( () => { + logWarnStub.restore(); + } ); + + it( 'should always render collapsed selection even if it is similar', () => { + const domSelection = document.getSelection(); + + const { view: viewP, selection: newSelection } = parse( + 'foo{}bar' ); + + viewRoot.appendChildren( viewP ); + selection.setTo( newSelection ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + const domP = domRoot.childNodes[ 0 ]; + const domB = domP.childNodes[ 1 ]; + const viewB = viewRoot.getChild( 0 ).getChild( 1 ); + + expect( domSelection.isCollapsed ).to.true; + expect( domSelection.rangeCount ).to.equal( 1 ); + expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domP.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( 3 ); + expect( domSelection.getRangeAt( 0 ).endContainer ).to.equal( domP.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).endOffset ).to.equal( 3 ); + + selectionCollapseSpy = sinon.spy( window.Selection.prototype, 'collapse' ); + selectionExtendSpy = sinon.spy( window.Selection.prototype, 'extend' ); + + // foo{}bar + selection.setRanges( [ new ViewRange( new ViewPosition( viewB.getChild( 0 ), 0 ), new ViewPosition( viewB.getChild( 0 ), 0 ) ) ] ); + + renderer.markToSync( 'children', viewP ); + renderer.render(); + + expect( selectionCollapseSpy.calledOnce ).to.true; + expect( selectionCollapseSpy.calledWith( domB.childNodes[ 0 ], 0 ) ).to.true; + expect( selectionExtendSpy.calledOnce ).to.true; + expect( selectionExtendSpy.calledWith( domB.childNodes[ 0 ], 0 ) ).to.true; + expect( logWarnStub.notCalled ).to.true; + } ); + + it( 'should always render collapsed selection even if it is similar (with empty element)', () => { + const domSelection = document.getSelection(); + + const { view: viewP, selection: newSelection } = parse( + 'foo[]' ); + + viewRoot.appendChildren( viewP ); + selection.setTo( newSelection ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + const domP = domRoot.childNodes[ 0 ]; + const domB = domP.childNodes[ 1 ]; + + expect( domSelection.isCollapsed ).to.true; + expect( domSelection.rangeCount ).to.equal( 1 ); + expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domB.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( INLINE_FILLER_LENGTH ); + expect( domSelection.getRangeAt( 0 ).endContainer ).to.equal( domB.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).endOffset ).to.equal( INLINE_FILLER_LENGTH ); + + selectionCollapseSpy = sinon.spy( window.Selection.prototype, 'collapse' ); + selectionExtendSpy = sinon.spy( window.Selection.prototype, 'extend' ); + + // foo{} + selection.setRanges( [ new ViewRange( new ViewPosition( viewP.getChild( 0 ), 3 ), new ViewPosition( viewP.getChild( 0 ), 3 ) ) ] ); + + renderer.markToSync( 'children', viewP ); + renderer.render(); + + expect( selectionCollapseSpy.calledOnce ).to.true; + expect( selectionCollapseSpy.calledWith( domP.childNodes[ 0 ], 3 ) ).to.true; + expect( selectionExtendSpy.calledOnce ).to.true; + expect( selectionExtendSpy.calledWith( domP.childNodes[ 0 ], 3 ) ).to.true; + expect( logWarnStub.notCalled ).to.true; + } ); + + it( 'should always render non-collapsed selection if it not is similar', () => { + const domSelection = document.getSelection(); + + const { view: viewP, selection: newSelection } = parse( + 'fo{o}bar' ); + + viewRoot.appendChildren( viewP ); + selection.setTo( newSelection ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + const domP = domRoot.childNodes[ 0 ]; + const domB = domP.childNodes[ 1 ]; + const viewB = viewRoot.getChild( 0 ).getChild( 1 ); + + expect( domSelection.isCollapsed ).to.false; + expect( domSelection.rangeCount ).to.equal( 1 ); + expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domP.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( 2 ); + expect( domSelection.getRangeAt( 0 ).endContainer ).to.equal( domP.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).endOffset ).to.equal( 3 ); + + selectionCollapseSpy = sinon.spy( window.Selection.prototype, 'collapse' ); + selectionExtendSpy = sinon.spy( window.Selection.prototype, 'extend' ); + + // fo{ob}ar + selection.setRanges( [ new ViewRange( new ViewPosition( viewP.getChild( 0 ), 2 ), new ViewPosition( viewB.getChild( 0 ), 1 ) ) ] ); + + renderer.markToSync( 'children', viewP ); + renderer.render(); + + expect( selectionCollapseSpy.calledOnce ).to.true; + expect( selectionCollapseSpy.calledWith( domP.childNodes[ 0 ], 2 ) ).to.true; + expect( selectionExtendSpy.calledOnce ).to.true; + expect( selectionExtendSpy.calledWith( domB.childNodes[ 0 ], 1 ) ).to.true; + expect( logWarnStub.notCalled ).to.true; + } ); + + it( 'should not render non-collapsed selection it is similar (element start)', () => { + const domSelection = document.getSelection(); + + const { view: viewP, selection: newSelection } = parse( + 'foo{ba}r' ); + + viewRoot.appendChildren( viewP ); + selection.setTo( newSelection ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + const domP = domRoot.childNodes[ 0 ]; + const domB = domP.childNodes[ 1 ]; + const viewB = viewRoot.getChild( 0 ).getChild( 1 ); + + expect( domSelection.isCollapsed ).to.false; + expect( domSelection.rangeCount ).to.equal( 1 ); + expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domB.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( 0 ); + expect( domSelection.getRangeAt( 0 ).endContainer ).to.equal( domB.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).endOffset ).to.equal( 2 ); + + selectionCollapseSpy = sinon.spy( window.Selection.prototype, 'collapse' ); + selectionExtendSpy = sinon.spy( window.Selection.prototype, 'extend' ); + + // foo{ba}r + selection.setRanges( [ new ViewRange( new ViewPosition( viewP.getChild( 0 ), 3 ), new ViewPosition( viewB.getChild( 0 ), 2 ) ) ] ); + + renderer.markToSync( 'children', viewP ); + renderer.render(); + + expect( selectionCollapseSpy.notCalled ).to.true; + expect( selectionExtendSpy.notCalled ).to.true; + expect( logWarnStub.called ).to.true; + } ); + + it( 'should not render non-collapsed selection it is similar (element end)', () => { + const domSelection = document.getSelection(); + + const { view: viewP, selection: newSelection } = parse( + 'foob{ar}baz' ); + + viewRoot.appendChildren( viewP ); + selection.setTo( newSelection ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + const domP = domRoot.childNodes[ 0 ]; + const domB = domP.childNodes[ 1 ]; + const viewB = viewRoot.getChild( 0 ).getChild( 1 ); + + expect( domSelection.isCollapsed ).to.false; + expect( domSelection.rangeCount ).to.equal( 1 ); + expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domB.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( 1 ); + expect( domSelection.getRangeAt( 0 ).endContainer ).to.equal( domB.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).endOffset ).to.equal( 3 ); + + selectionCollapseSpy = sinon.spy( window.Selection.prototype, 'collapse' ); + selectionExtendSpy = sinon.spy( window.Selection.prototype, 'extend' ); + + // foob{ar}baz + selection.setRanges( [ new ViewRange( new ViewPosition( viewB.getChild( 0 ), 1 ), new ViewPosition( viewP.getChild( 2 ), 0 ) ) ] ); + + renderer.markToSync( 'children', viewP ); + renderer.render(); + + expect( selectionCollapseSpy.notCalled ).to.true; + expect( selectionExtendSpy.notCalled ).to.true; + expect( logWarnStub.called ).to.true; + } ); + + it( 'should not render non-collapsed selection it is similar (element start - nested)', () => { + const domSelection = document.getSelection(); + + const { view: viewP, selection: newSelection } = parse( + 'foo{ba}r' ); + + viewRoot.appendChildren( viewP ); + selection.setTo( newSelection ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + const domP = domRoot.childNodes[ 0 ]; + const domB = domP.childNodes[ 1 ]; + const viewI = viewRoot.getChild( 0 ).getChild( 1 ).getChild( 0 ); + + expect( domSelection.isCollapsed ).to.false; + expect( domSelection.rangeCount ).to.equal( 1 ); + expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domB.childNodes[ 0 ].childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( 0 ); + expect( domSelection.getRangeAt( 0 ).endContainer ).to.equal( domB.childNodes[ 0 ].childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).endOffset ).to.equal( 2 ); + + selectionCollapseSpy = sinon.spy( window.Selection.prototype, 'collapse' ); + selectionExtendSpy = sinon.spy( window.Selection.prototype, 'extend' ); + + // foo{ba}r + selection.setRanges( [ new ViewRange( new ViewPosition( viewP.getChild( 0 ), 3 ), new ViewPosition( viewI.getChild( 0 ), 2 ) ) ] ); + + renderer.markToSync( 'children', viewP ); + renderer.render(); + + expect( selectionCollapseSpy.notCalled ).to.true; + expect( selectionExtendSpy.notCalled ).to.true; + expect( logWarnStub.called ).to.true; + } ); + + it( 'should not render non-collapsed selection it is similar (element end - nested)', () => { + const domSelection = document.getSelection(); + + const { view: viewP, selection: newSelection } = parse( + 'f{oobar}baz' ); + + viewRoot.appendChildren( viewP ); + selection.setTo( newSelection ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + const domP = domRoot.childNodes[ 0 ]; + const domB = domP.childNodes[ 1 ]; + + expect( domSelection.isCollapsed ).to.false; + expect( domSelection.rangeCount ).to.equal( 1 ); + expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domP.childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( 1 ); + expect( domSelection.getRangeAt( 0 ).endContainer ).to.equal( domB.childNodes[ 0 ].childNodes[ 0 ] ); + expect( domSelection.getRangeAt( 0 ).endOffset ).to.equal( 3 ); + + selectionCollapseSpy = sinon.spy( window.Selection.prototype, 'collapse' ); + selectionExtendSpy = sinon.spy( window.Selection.prototype, 'extend' ); + + // f{oobar}baz + selection.setRanges( [ new ViewRange( new ViewPosition( viewP.getChild( 0 ), 1 ), new ViewPosition( viewP.getChild( 2 ), 0 ) ) ] ); + + renderer.markToSync( 'children', viewP ); + renderer.render(); + + expect( selectionCollapseSpy.notCalled ).to.true; + expect( selectionExtendSpy.notCalled ).to.true; + expect( logWarnStub.called ).to.true; + } ); + } ); } ); } );