Przeglądaj źródła

Internal: Improved test coverage, fixed return value if there is no container or container is in a different root.

Marek Lewandowski 6 lat temu
rodzic
commit
74d90d0286

+ 3 - 3
packages/ckeditor5-engine/src/view/renderer.js

@@ -805,10 +805,10 @@ export default class Renderer {
 		const container = this._fakeSelectionContainer;
 		const domSelection = domRoot.ownerDocument.getSelection();
 
-		// Selection fakeselection needs to be updated if there's no fake selection container, or container
-		// currently sits in different root.
+		// 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 false;
+			return true;
 		}
 
 		// Make sure that the selection actually is within the fake selection.

+ 44 - 16
packages/ckeditor5-engine/tests/view/renderer.js

@@ -1804,27 +1804,55 @@ describe( 'Renderer', () => {
 				assertDomSelectionContents( domSelection, container, /^fake selection label$/ );
 			} );
 
-			it( 'doesn\'t render the same selection multiple times', () => {
-				const createRangeSpy = sinon.spy( document, 'createRange' );
-				const label = 'subsequent fake selection calls';
+			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();
+					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 );
-			} );
+					expect( createRangeSpy.callCount ).to.be.equal( 1 );
+				} );
 
-			it( 'different subsequent fake selections sets do change native selection', () => {
-				const createRangeSpy = sinon.spy( document, 'createRange' );
+				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();
+					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 );
+					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( '_fakeSelectionNeedsUpdate returns proper result when container is uninitialized', () => {
+					selection._setTo( selection.getRanges(), { fake: true, label: 'selection 1' } );
+					expect( renderer._fakeSelectionNeedsUpdate( domRoot ) ).to.be.true;
+				} );
 			} );
 
 			it( 'should render   if no selection label is provided', () => {