Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge 473248a into 17c495f
Browse files Browse the repository at this point in the history
  • Loading branch information
mlewand committed Sep 10, 2019
2 parents 17c495f + 473248a commit c5e040d
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 16 deletions.
68 changes: 52 additions & 16 deletions src/view/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -698,22 +698,14 @@ 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' );
const container = this._fakeSelectionContainer = this._fakeSelectionContainer || createFakeSelectionContainer( domDocument );

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 current selection.
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 ) {
Expand All @@ -730,9 +722,6 @@ export default class Renderer {
domSelection.removeAllRanges();
domRange.selectNodeContents( container );
domSelection.addRange( domRange );

// Bind fake selection container with current selection.
this.domConverter.bindFakeSelection( container, this.selection );
}

/**
Expand Down Expand Up @@ -804,6 +793,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.
*
Expand Down Expand Up @@ -985,3 +999,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;
}
46 changes: 46 additions & 0 deletions tests/view/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit c5e040d

Please sign in to comment.