From 7ac16b866ce328c2a31f8c96c0a9d84c50b7673e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 29 Mar 2017 11:43:14 +0200 Subject: [PATCH 1/2] Fixed issue with duble inserting caption to image. --- src/imagecaption/imagecaptionengine.js | 6 ++++- tests/imagecaption/imagecaptionengine.js | 28 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/imagecaption/imagecaptionengine.js b/src/imagecaption/imagecaptionengine.js index f0a9b421..92e7e6fa 100644 --- a/src/imagecaption/imagecaptionengine.js +++ b/src/imagecaption/imagecaptionengine.js @@ -139,7 +139,11 @@ function insertMissingModelCaptionElement( evt, changeType, data, batch ) { if ( value.type == 'elementStart' && isImage( item ) && !getCaptionFromImage( item ) ) { batch.document.enqueueChanges( () => { - batch.insert( ModelPosition.createAt( item, 'end' ), new ModelElement( 'caption' ) ); + // Make sure that the image does not have caption already. + // https://github.com/ckeditor/ckeditor5-image/issues/78 + if ( !getCaptionFromImage( item ) ) { + batch.insert( ModelPosition.createAt( item, 'end' ), new ModelElement( 'caption' ) ); + } } ); } } diff --git a/tests/imagecaption/imagecaptionengine.js b/tests/imagecaption/imagecaptionengine.js index 5afb540b..e5482828 100644 --- a/tests/imagecaption/imagecaptionengine.js +++ b/tests/imagecaption/imagecaptionengine.js @@ -189,6 +189,34 @@ describe( 'ImageCaptionEngine', () => { ); } ); + it( 'should not add caption element twice', () => { + const image = new ModelElement( 'image', { src: '', alt: '' } ); + const caption = new ModelElement( 'caption' ); + const batch = document.batch(); + + // When first change to the document will be made - manually add caption to the image element. + document.once( 'change', () => { + document.enqueueChanges( () => { + batch.insert( ModelPosition.createAt( image ), caption ); + } ); + }, { priority: 'high' } ); + + document.enqueueChanges( () => { + batch.insert( ModelPosition.createAt( document.getRoot() ), image ); + } ); + + expect( getModelData( document ) ).to.equal( + '[]' + ); + + expect( getViewData( viewDocument ) ).to.equal( + '[]
' + + '' + + '
' + + '
' + ); + } ); + it( 'should do nothing for other changes than insert', () => { setModelData( document, 'foo bar' ); const image = document.getRoot().getChild( 0 ); From 456bc00fbd7f41bf7575bc2d1ee45bbd8951d62a Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Fri, 31 Mar 2017 09:57:01 +0200 Subject: [PATCH 2/2] Tests: changed test to better reflect original issue. --- tests/imagecaption/imagecaptionengine.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/imagecaption/imagecaptionengine.js b/tests/imagecaption/imagecaptionengine.js index 93697feb..5a6fb913 100644 --- a/tests/imagecaption/imagecaptionengine.js +++ b/tests/imagecaption/imagecaptionengine.js @@ -248,17 +248,15 @@ describe( 'ImageCaptionEngine', () => { const caption = new ModelElement( 'caption' ); const batch = document.batch(); - // When first change to the document will be made - manually add caption to the image element. - document.once( 'change', () => { - document.enqueueChanges( () => { - batch.insert( ModelPosition.createAt( image ), caption ); - } ); - }, { priority: 'high' } ); - document.enqueueChanges( () => { - batch.insert( ModelPosition.createAt( document.getRoot() ), image ); + batch + // Since we are adding an empty image, this should trigger caption fixer. + .insert( ModelPosition.createAt( document.getRoot() ), image ) + // Add caption just after the image is inserted, in same batch and enqueue changes block. + .insert( ModelPosition.createAt( image ), caption ); } ); + // Check whether caption fixer added redundant caption. expect( getModelData( document ) ).to.equal( '[]' );