diff --git a/src/view/domconverter.js b/src/view/domconverter.js index 66505be86..f7c61b4e6 100644 --- a/src/view/domconverter.js +++ b/src/view/domconverter.js @@ -106,8 +106,9 @@ export default class DomConverter { } /** - * Binds given DOM element that represents fake selection to {@link module:engine/view/documentselection~DocumentSelection - * document selection}. Document selection copy is stored and can be retrieved by + * Binds given DOM element that represents fake selection to a **position** of a + * {@link module:engine/view/documentselection~DocumentSelection document selection}. + * Document selection copy is stored and can be retrieved by * {@link module:engine/view/domconverter~DomConverter#fakeSelectionToView} method. * * @param {HTMLElement} domElement diff --git a/src/view/renderer.js b/src/view/renderer.js index 8606b8b0b..a44fa10c6 100644 --- a/src/view/renderer.js +++ b/src/view/renderer.js @@ -698,41 +698,32 @@ export default class Renderer { */ _updateFakeSelection( domRoot ) { const domDocument = domRoot.ownerDocument; - let container = this._fakeSelectionContainer; - // Create fake selection container if one does not exist. - if ( !container ) { - this._fakeSelectionContainer = container = domDocument.createElement( 'div' ); + if ( !this._fakeSelectionContainer ) { + this._fakeSelectionContainer = createFakeSelectionContainer( domDocument ); + } + + const container = this._fakeSelectionContainer; - Object.assign( container.style, { - position: 'fixed', - top: 0, - left: '-9999px', - // See https://github.com/ckeditor/ckeditor5/issues/752. - width: '42px' - } ); + // Bind fake selection container with the current selection *position*. + this.domConverter.bindFakeSelection( container, this.selection ); - // Fill it with a text node so we can update it later. - container.textContent = '\u00A0'; + if ( !this._fakeSelectionNeedsUpdate( domRoot ) ) { + return; } if ( !container.parentElement || container.parentElement != domRoot ) { domRoot.appendChild( container ); } - // Update contents. container.textContent = this.selection.fakeSelectionLabel || '\u00A0'; - // Update selection. const domSelection = domDocument.getSelection(); const domRange = domDocument.createRange(); domSelection.removeAllRanges(); domRange.selectNodeContents( container ); domSelection.addRange( domRange ); - - // Bind fake selection container with current selection. - this.domConverter.bindFakeSelection( container, this.selection ); } /** @@ -804,6 +795,31 @@ export default class Renderer { return true; } + /** + * Checks whether the fake selection needs to be updated. + * + * @private + * @param {HTMLElement} domRoot A valid DOM root where a new fake selection container should be added. + * @returns {Boolean} + */ + _fakeSelectionNeedsUpdate( domRoot ) { + const container = this._fakeSelectionContainer; + const domSelection = domRoot.ownerDocument.getSelection(); + + // Fake selection needs to be updated if there's no fake selection container, or the container currently sits + // in a different root. + if ( !container || container.parentElement !== domRoot ) { + return true; + } + + // Make sure that the selection actually is within the fake selection. + if ( domSelection.anchorNode !== container && !container.contains( domSelection.anchorNode ) ) { + return true; + } + + return container.textContent !== this.selection.fakeSelectionLabel; + } + /** * Removes the DOM selection. * @@ -985,3 +1001,25 @@ function filterOutFakeSelectionContainer( domChildList, fakeSelectionContainer ) return childList; } + +// Creates a fake selection container for a given document. +// +// @private +// @param {Document} domDocument +// @returns {HTMLElement} +function createFakeSelectionContainer( domDocument ) { + const container = domDocument.createElement( 'div' ); + + Object.assign( container.style, { + position: 'fixed', + top: 0, + left: '-9999px', + // See https://github.com/ckeditor/ckeditor5/issues/752. + width: '42px' + } ); + + // Fill it with a text node so we can update it later. + container.textContent = '\u00A0'; + + return container; +} diff --git a/tests/view/renderer.js b/tests/view/renderer.js index 238589f47..d4daf6af5 100644 --- a/tests/view/renderer.js +++ b/tests/view/renderer.js @@ -1804,6 +1804,52 @@ describe( 'Renderer', () => { assertDomSelectionContents( domSelection, container, /^fake selection label$/ ); } ); + describe( 'subsequent call optimization', () => { + // https://github.com/ckeditor/ckeditor5-engine/issues/1791 + it( 'doesn\'t render the same selection multiple times', () => { + const createRangeSpy = sinon.spy( document, 'createRange' ); + const label = 'subsequent fake selection calls'; + + selection._setTo( selection.getRanges(), { fake: true, label } ); + renderer.render(); + selection._setTo( selection.getRanges(), { fake: true, label } ); + renderer.render(); + + expect( createRangeSpy.callCount ).to.be.equal( 1 ); + } ); + + it( 'different subsequent fake selections sets do change native selection', () => { + const createRangeSpy = sinon.spy( document, 'createRange' ); + + selection._setTo( selection.getRanges(), { fake: true, label: 'selection 1' } ); + renderer.render(); + selection._setTo( selection.getRanges(), { fake: true, label: 'selection 2' } ); + renderer.render(); + + expect( createRangeSpy.callCount ).to.be.equal( 2 ); + } ); + + it( 'rerenders selection if disturbed externally', () => { + const interruptingRange = document.createRange(); + interruptingRange.setStartBefore( domRoot.children[ 0 ] ); + interruptingRange.setEndAfter( domRoot.children[ 0 ] ); + + const createRangeSpy = sinon.spy( document, 'createRange' ); + const label = 'selection 1'; + + selection._setTo( selection.getRanges(), { fake: true, label } ); + renderer.render(); + + document.getSelection().removeAllRanges(); + document.getSelection().addRange( interruptingRange ); + + selection._setTo( selection.getRanges(), { fake: true, label } ); + renderer.render(); + + expect( createRangeSpy.callCount ).to.be.equal( 2 ); + } ); + } ); + it( 'should render   if no selection label is provided', () => { selection._setTo( selection.getRanges(), { fake: true } ); renderer.render();