From 539a0aa6d712c375a68704ce463208213e626bcb Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Mon, 18 Feb 2019 14:50:30 +0100 Subject: [PATCH 1/3] Fix: Fake selection container was not appended to the new fake selection editable when fake selection changes. --- src/view/renderer.js | 5 +++- tests/view/renderer.js | 55 +++++++++++++++++++++++++++--------------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/view/renderer.js b/src/view/renderer.js index 894470fc7..88216d5b7 100644 --- a/src/view/renderer.js +++ b/src/view/renderer.js @@ -707,7 +707,10 @@ export default class Renderer { container.appendChild( domDocument.createTextNode( '\u00A0' ) ); } - // Add fake container if not already added. + if ( container.parentElement && container.parentElement != domRoot ) { + container.remove( container ); + } + if ( !container.parentElement ) { domRoot.appendChild( container ); } diff --git a/tests/view/renderer.js b/tests/view/renderer.js index 8113f0743..c1057d973 100644 --- a/tests/view/renderer.js +++ b/tests/view/renderer.js @@ -7,6 +7,7 @@ import View from '../../src/view/view'; import ViewElement from '../../src/view/element'; +import ViewEditableElement from '../../src/view/editableelement'; import ViewContainerElement from '../../src/view/containerelement'; import ViewAttributeElement from '../../src/view/attributeelement'; import ViewText from '../../src/view/text'; @@ -112,10 +113,12 @@ describe( 'Renderer', () => { } ); describe( 'render', () => { - let viewRoot, domRoot, selectionEditable; + let viewRoot, domRoot; beforeEach( () => { - viewRoot = new ViewElement( 'div' ); + viewRoot = new ViewEditableElement( 'div' ); + viewRoot.getFillerOffset = () => null; + domRoot = document.createElement( 'div' ); document.body.appendChild( domRoot ); @@ -127,16 +130,7 @@ describe( 'Renderer', () => { selection._setTo( null ); - selectionEditable = viewRoot; - renderer.isFocused = true; - - // Fake selection editable - it is needed to render selection properly. - Object.defineProperty( selection, 'editableElement', { - get() { - return selectionEditable; - } - } ); } ); afterEach( () => { @@ -422,8 +416,6 @@ describe( 'Renderer', () => { } ); it( 'should not care about filler if there is no DOM', () => { - selectionEditable = null; - const { view: viewP, selection: newSelection } = parse( 'foo[]bar' ); @@ -1202,11 +1194,10 @@ describe( 'Renderer', () => { domSelection.removeAllRanges(); domSelection.collapse( domDiv, 0 ); - selectionEditable = null; - + const viewDiv = new ViewElement( 'div' ); const { view: viewP, selection: newSelection } = parse( 'fo{o}' ); - viewRoot._appendChild( viewP ); + viewDiv._appendChild( viewP ); selection._setTo( newSelection ); renderer.render(); @@ -1353,13 +1344,13 @@ describe( 'Renderer', () => { } ); it( 'should handle focusing element', () => { + selection._setTo( viewRoot, 0 ); + const domFocusSpy = testUtils.sinon.spy( domRoot, 'focus' ); - const editable = selection.editableElement; renderer.render(); - expect( editable ).to.equal( viewRoot ); - expect( domFocusSpy.calledOnce ).to.be.true; + expect( domFocusSpy.called ).to.be.true; } ); it( 'should not focus editable if isFocues is set to false', () => { @@ -1928,6 +1919,32 @@ describe( 'Renderer', () => { expect( container.style.left ).to.equal( '-9999px' ); } ); + it( 'should move fake selection container between editables', () => { + const viewEditable = new ViewEditableElement( 'div' ); + viewEditable._appendChild( parse( 'abc xyz' ) ); + + const domEditable = document.createElement( 'div' ); + + document.body.appendChild( domEditable ); + + domConverter.bindElements( domEditable, viewEditable ); + + renderer.markToSync( 'children', viewEditable ); + selection._setTo( selection.getRanges(), { fake: true, label: 'fake selection' } ); + renderer.render(); + + let container = document.getSelection().anchorNode; + + expect( container.parentElement ).to.equal( domRoot ); + + selection._setTo( viewEditable, 'in', { fake: true, label: 'fake selection' } ); + renderer.render(); + + container = document.getSelection().anchorNode; + + expect( container.parentElement ).to.equal( domEditable ); + } ); + it( 'should bind fake selection container to view selection', () => { selection._setTo( selection.getRanges(), { fake: true, label: 'fake selection' } ); renderer.render(); From cae80cb0641ae58ca12b003bd97994f2aae79f2e Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Mon, 18 Feb 2019 15:22:51 +0100 Subject: [PATCH 2/3] Tests: Re-written test so it is less prone to browser quirks. --- tests/view/renderer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/view/renderer.js b/tests/view/renderer.js index c1057d973..c505bf206 100644 --- a/tests/view/renderer.js +++ b/tests/view/renderer.js @@ -1935,14 +1935,14 @@ describe( 'Renderer', () => { let container = document.getSelection().anchorNode; - expect( container.parentElement ).to.equal( domRoot ); + expect( domRoot.contains( container ) ).to.be.true; selection._setTo( viewEditable, 'in', { fake: true, label: 'fake selection' } ); renderer.render(); container = document.getSelection().anchorNode; - expect( container.parentElement ).to.equal( domEditable ); + expect( domEditable.contains( container ) ).to.be.true; } ); it( 'should bind fake selection container to view selection', () => { From 4dcbfa30af515e6d4fbb56cdf338e39e29f7a9a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 19 Feb 2019 11:25:52 +0100 Subject: [PATCH 3/3] Simplified the logic. --- src/view/renderer.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/view/renderer.js b/src/view/renderer.js index 88216d5b7..e0b40be3b 100644 --- a/src/view/renderer.js +++ b/src/view/renderer.js @@ -707,11 +707,7 @@ export default class Renderer { container.appendChild( domDocument.createTextNode( '\u00A0' ) ); } - if ( container.parentElement && container.parentElement != domRoot ) { - container.remove( container ); - } - - if ( !container.parentElement ) { + if ( !container.parentElement || container.parentElement != domRoot ) { domRoot.appendChild( container ); }