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

Commit 6591f87

Browse files
jodatorjodator
authored
Merge pull request #1687 from ckeditor/t/ckeditor5/1578b
Fix: Filter out fake selection container before comparing DOM view root children in view renderer. Closes ckeditor/ckeditor5#1578.
2 parents 2f2b42e + 4227f1d commit 6591f87

File tree

2 files changed

+278
-2
lines changed

2 files changed

+278
-2
lines changed

‎src/view/renderer.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,8 @@ export default class Renderer {
581581
* @returns {Array.<String>} The list of actions based on the {@link module:utils/diff~diff} function.
582582
*/
583583
_diffNodeLists( actualDomChildren, expectedDomChildren ) {
584+
actualDomChildren = filterOutFakeSelectionContainer( actualDomChildren, this._fakeSelectionContainer );
585+
584586
return diff( actualDomChildren, expectedDomChildren, sameNodes.bind( null, this.domConverter.blockFiller ) );
585587
}
586588

@@ -955,3 +957,19 @@ function fixGeckoSelectionAfterBr( focus, domSelection ) {
955957
domSelection.addRange( domSelection.getRangeAt( 0 ) );
956958
}
957959
}
960+
961+
function filterOutFakeSelectionContainer( domChildList, fakeSelectionContainer ) {
962+
const childList = Array.from( domChildList );
963+
964+
if ( childList.length == 0 || !fakeSelectionContainer ) {
965+
return childList;
966+
}
967+
968+
const last = childList[ childList.length - 1 ];
969+
970+
if ( last == fakeSelectionContainer ) {
971+
childList.pop();
972+
}
973+
974+
return childList;
975+
}

‎tests/view/renderer.js‎

Lines changed: 260 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* For licensing, see LICENSE.md.
44
*/
55

6-
/* globals document, window, NodeFilter */
6+
/* globals document, window, NodeFilter, MutationObserver */
77

88
import View from '../../src/view/view';
99
import ViewElement from '../../src/view/element';
@@ -2302,7 +2302,7 @@ describe( 'Renderer', () => {
23022302
} );
23032303

23042304
// #1417
2305-
describe( 'optimal rendering', () => {
2305+
describe( 'optimal rendering – reusing existing nodes', () => {
23062306
it( 'should render inline element replacement (before text)', () => {
23072307
viewRoot._appendChild( parse( '<container:p><attribute:i>A</attribute:i>1</container:p>' ) );
23082308

@@ -3126,6 +3126,264 @@ describe( 'Renderer', () => {
31263126
} );
31273127
} );
31283128

3129+
describe( 'optimal (minimal) rendering – minimal children changes', () => {
3130+
let observer;
3131+
3132+
beforeEach( () => {
3133+
observer = new MutationObserver( () => {} );
3134+
3135+
observer.observe( domRoot, {
3136+
childList: true,
3137+
attributes: false,
3138+
subtree: false
3139+
} );
3140+
} );
3141+
3142+
afterEach( () => {
3143+
observer.disconnect();
3144+
} );
3145+
3146+
it( 'should add only one child (at the beginning)', () => {
3147+
viewRoot._appendChild( parse( '<container:p>1</container:p>' ) );
3148+
3149+
renderer.markToSync( 'children', viewRoot );
3150+
renderer.render();
3151+
cleanObserver( observer );
3152+
3153+
viewRoot._insertChild( 0, parse( '<container:p>2</container:p>' ) );
3154+
3155+
renderer.markToSync( 'children', viewRoot );
3156+
renderer.render();
3157+
3158+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3159+
'added: 1, removed: 0'
3160+
] );
3161+
} );
3162+
3163+
it( 'should add only one child (at the end)', () => {
3164+
viewRoot._appendChild( parse( '<container:p>1</container:p>' ) );
3165+
3166+
renderer.markToSync( 'children', viewRoot );
3167+
renderer.render();
3168+
cleanObserver( observer );
3169+
3170+
viewRoot._appendChild( parse( '<container:p>2</container:p>' ) );
3171+
3172+
renderer.markToSync( 'children', viewRoot );
3173+
renderer.render();
3174+
3175+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3176+
'added: 1, removed: 0'
3177+
] );
3178+
} );
3179+
3180+
it( 'should add only one child (in the middle)', () => {
3181+
viewRoot._appendChild( parse( '<container:p>1</container:p><container:p>2</container:p>' ) );
3182+
3183+
renderer.markToSync( 'children', viewRoot );
3184+
renderer.render();
3185+
cleanObserver( observer );
3186+
3187+
viewRoot._insertChild( 1, parse( '<container:p>3</container:p>' ) );
3188+
3189+
renderer.markToSync( 'children', viewRoot );
3190+
renderer.render();
3191+
3192+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3193+
'added: 1, removed: 0'
3194+
] );
3195+
} );
3196+
3197+
it( 'should not touch elements at all (rendering texts is enough)', () => {
3198+
viewRoot._appendChild( parse( '<container:p>1</container:p><container:p>2</container:p>' ) );
3199+
3200+
renderer.markToSync( 'children', viewRoot );
3201+
renderer.render();
3202+
cleanObserver( observer );
3203+
3204+
viewRoot._insertChild( 1, parse( '<container:p>3</container:p>' ) );
3205+
viewRoot._removeChildren( 0, 1 );
3206+
3207+
renderer.markToSync( 'children', viewRoot );
3208+
renderer.render();
3209+
3210+
expect( getMutationStats( observer.takeRecords() ) ).to.be.empty;
3211+
} );
3212+
3213+
it( 'should add and remove one', () => {
3214+
viewRoot._appendChild( parse( '<container:p>1</container:p><container:p>2</container:p>' ) );
3215+
3216+
renderer.markToSync( 'children', viewRoot );
3217+
renderer.render();
3218+
cleanObserver( observer );
3219+
3220+
viewRoot._insertChild( 1, parse( '<container:h1>3</container:h1>' ) );
3221+
viewRoot._removeChildren( 0, 1 );
3222+
3223+
renderer.markToSync( 'children', viewRoot );
3224+
renderer.render();
3225+
3226+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3227+
'added: 1, removed: 0',
3228+
'added: 0, removed: 1'
3229+
] );
3230+
} );
3231+
3232+
it( 'should not touch the FSC when rendering children', () => {
3233+
viewRoot._appendChild( parse( '<container:p>1</container:p><container:p>2</container:p>' ) );
3234+
3235+
// Set fake selection on the second paragraph.
3236+
selection._setTo( viewRoot.getChild( 1 ), 'on', { fake: true } );
3237+
3238+
renderer.markToSync( 'children', viewRoot );
3239+
renderer.render();
3240+
cleanObserver( observer );
3241+
3242+
// Remove the second paragraph.
3243+
viewRoot._removeChildren( 1, 1 );
3244+
// And set the fake selection on the first one.
3245+
selection._setTo( viewRoot.getChild( 0 ), 'on', { fake: true } );
3246+
3247+
renderer.markToSync( 'children', viewRoot );
3248+
renderer.render();
3249+
3250+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3251+
'added: 0, removed: 1'
3252+
] );
3253+
} );
3254+
3255+
describe( 'using fastDiff() - significant number of nodes in the editor', () => {
3256+
it( 'should add only one child (at the beginning)', () => {
3257+
viewRoot._appendChild( parse( makeContainers( 151 ) ) );
3258+
3259+
renderer.markToSync( 'children', viewRoot );
3260+
renderer.render();
3261+
cleanObserver( observer );
3262+
3263+
viewRoot._insertChild( 0, parse( '<container:p>x</container:p>' ) );
3264+
3265+
renderer.markToSync( 'children', viewRoot );
3266+
renderer.render();
3267+
3268+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3269+
'added: 1, removed: 0'
3270+
] );
3271+
} );
3272+
3273+
it( 'should add only one child (at the end)', () => {
3274+
viewRoot._appendChild( parse( makeContainers( 151 ) ) );
3275+
3276+
renderer.markToSync( 'children', viewRoot );
3277+
renderer.render();
3278+
cleanObserver( observer );
3279+
3280+
viewRoot._appendChild( parse( '<container:p>x</container:p>' ) );
3281+
3282+
renderer.markToSync( 'children', viewRoot );
3283+
renderer.render();
3284+
3285+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3286+
'added: 1, removed: 0'
3287+
] );
3288+
} );
3289+
3290+
it( 'should add only one child (in the middle)', () => {
3291+
viewRoot._appendChild( parse( makeContainers( 151 ) ) );
3292+
3293+
renderer.markToSync( 'children', viewRoot );
3294+
renderer.render();
3295+
cleanObserver( observer );
3296+
3297+
viewRoot._insertChild( 75, parse( '<container:p>x</container:p>' ) );
3298+
3299+
renderer.markToSync( 'children', viewRoot );
3300+
renderer.render();
3301+
3302+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3303+
'added: 1, removed: 0'
3304+
] );
3305+
} );
3306+
3307+
it( 'should not touch elements at all (rendering texts is enough)', () => {
3308+
viewRoot._appendChild( parse( makeContainers( 151 ) ) );
3309+
3310+
renderer.markToSync( 'children', viewRoot );
3311+
renderer.render();
3312+
cleanObserver( observer );
3313+
3314+
viewRoot._insertChild( 1, parse( '<container:p>x</container:p>' ) );
3315+
viewRoot._removeChildren( 0, 1 );
3316+
3317+
renderer.markToSync( 'children', viewRoot );
3318+
renderer.render();
3319+
3320+
expect( getMutationStats( observer.takeRecords() ) ).to.be.empty;
3321+
} );
3322+
3323+
it( 'should add and remove one', () => {
3324+
viewRoot._appendChild( parse( makeContainers( 151 ) ) );
3325+
3326+
renderer.markToSync( 'children', viewRoot );
3327+
renderer.render();
3328+
cleanObserver( observer );
3329+
3330+
viewRoot._insertChild( 1, parse( '<container:h1>x</container:h1>' ) );
3331+
viewRoot._removeChildren( 0, 1 );
3332+
3333+
renderer.markToSync( 'children', viewRoot );
3334+
renderer.render();
3335+
3336+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3337+
'added: 1, removed: 0',
3338+
'added: 0, removed: 1'
3339+
] );
3340+
} );
3341+
3342+
it( 'should not touch the FSC when rendering children', () => {
3343+
viewRoot._appendChild( parse( makeContainers( 151 ) ) );
3344+
3345+
// Set fake selection on the second paragraph.
3346+
selection._setTo( viewRoot.getChild( 1 ), 'on', { fake: true } );
3347+
3348+
renderer.markToSync( 'children', viewRoot );
3349+
renderer.render();
3350+
cleanObserver( observer );
3351+
3352+
// Remove the second paragraph.
3353+
viewRoot._removeChildren( 1, 1 );
3354+
// And set the fake selection on the first one.
3355+
selection._setTo( viewRoot.getChild( 0 ), 'on', { fake: true } );
3356+
3357+
renderer.markToSync( 'children', viewRoot );
3358+
renderer.render();
3359+
3360+
expect( getMutationStats( observer.takeRecords() ) ).to.deep.equal( [
3361+
'added: 0, removed: 1'
3362+
] );
3363+
} );
3364+
} );
3365+
3366+
function getMutationStats( mutationList ) {
3367+
return mutationList.map( mutation => {
3368+
return `added: ${ mutation.addedNodes.length }, removed: ${ mutation.removedNodes.length }`;
3369+
} );
3370+
}
3371+
3372+
function cleanObserver( observer ) {
3373+
observer.takeRecords();
3374+
}
3375+
3376+
function makeContainers( howMany ) {
3377+
const containers = [];
3378+
3379+
for ( let i = 1; i <= howMany; i++ ) {
3380+
containers.push( `<container:p>${ i }</container:p>` );
3381+
}
3382+
3383+
return containers.join( '' );
3384+
}
3385+
} );
3386+
31293387
// #1560
31303388
describe( 'attributes manipulation on replaced element', () => {
31313389
it( 'should rerender element if it was removed after having its attributes removed (attribute)', () => {

0 commit comments

Comments
 (0)