Merge branch 't/12377'

Author: Reinmar

CHANGES.md

@@ -7,6 +7,7 @@ Fixed Issues:
 
 * [#10804](http://dev.ckeditor.com/ticket/10804): Fixed: `CKEDITOR_GETURL` isn't used with some plugins it should be used. Thanks to [Thomas Andraschko](https://github.com/tandraschko)!
 * [#9137](http://dev.ckeditor.com/ticket/9137): Fixed: `base` tag is not created when `head` has an attribute. Thanks to [naoki.fujikawa](https://github.com/naoki-fujikawa)!
+* [#12377](http://dev.ckeditor.com/ticket/12377): Fixed: Errors thrown in the image plugin when removing preview from dialog definition. Thanks to [Axinet](https://github.com/Axinet)!
 * [#12315](http://dev.ckeditor.com/ticket/12315): Fixed: Marked [`config.autoParagraph`](http://docs.ckeditor.com/#!/api/CKEDITOR.config-cfg-autoParagraph) as deprecated.
 * [#12113](http://dev.ckeditor.com/ticket/12113): Fixed: Code snippet should be presented in elements path as "codesnippet".
 * [#12311](http://dev.ckeditor.com/ticket/12311): Fixed: Remove format should also remove `<cite>` elements.

plugins/image/dialogs/image.js

@@ -194,14 +194,17 @@
 
 			var onImgLoadEvent = function() {
 					// Image is ready.
-					var original = this.originalElement;
+					var original = this.originalElement,
+						loader = CKEDITOR.document.getById( imagePreviewLoaderId );
+
 					original.setCustomData( 'isReady', 'true' );
 					original.removeListener( 'load', onImgLoadEvent );
 					original.removeListener( 'error', onImgLoadErrorEvent );
 					original.removeListener( 'abort', onImgLoadErrorEvent );
 
-					// Hide loader
-					CKEDITOR.document.getById( imagePreviewLoaderId ).setStyle( 'display', 'none' );
+					// Hide loader.
+					if ( loader )
+						loader.setStyle( 'display', 'none' );
 
 					// New image -> new domensions
 					if ( !this.dontResetSize )
@@ -218,7 +221,9 @@
 
 			var onImgLoadErrorEvent = function() {
 					// Error. Image is not loaded.
-					var original = this.originalElement;
+					var original = this.originalElement,
+						loader = CKEDITOR.document.getById( imagePreviewLoaderId );
+
 					original.removeListener( 'load', onImgLoadEvent );
 					original.removeListener( 'error', onImgLoadErrorEvent );
 					original.removeListener( 'abort', onImgLoadErrorEvent );
@@ -229,8 +234,10 @@
 					if ( this.preview )
 						this.preview.setAttribute( 'src', noimage );
 
-					// Hide loader
-					CKEDITOR.document.getById( imagePreviewLoaderId ).setStyle( 'display', 'none' );
+					// Hide loader.
+					if ( loader )
+						loader.setStyle( 'display', 'none' );
+
 					switchLockRatio( this, false ); // Unlock.
 				};
 
@@ -264,10 +271,13 @@
 					var editor = this.getParentEditor(),
 						sel = editor.getSelection(),
 						element = sel && sel.getSelectedElement(),
-						link = element && editor.elementPath( element ).contains( 'a', 1 );
+						link = element && editor.elementPath( element ).contains( 'a', 1 ),
+						loader = CKEDITOR.document.getById( imagePreviewLoaderId );
+
+					// Hide loader.
+					if ( loader )
+						loader.setStyle( 'display', 'none' );
 
-					//Hide loader.
-					CKEDITOR.document.getById( imagePreviewLoaderId ).setStyle( 'display', 'none' );
 					// Create the preview before setup the dialog contents.
 					previewPreloader = new CKEDITOR.dom.element( 'img', editor.document );
 					this.preview = CKEDITOR.document.getById( previewImageId );
@@ -464,10 +474,12 @@
 										dialog = this.getDialog();
 										var original = dialog.originalElement;
 
-										dialog.preview.removeStyle( 'display' );
+										if ( dialog.preview ) {
+											dialog.preview.removeStyle( 'display' );
+										}
 
 										original.setCustomData( 'isReady', 'false' );
-										// Show loader
+										// Show loader.
 										var loader = CKEDITOR.document.getById( imagePreviewLoaderId );
 										if ( loader )
 											loader.setStyle( 'display', '' );
@@ -477,10 +489,12 @@
 										original.on( 'abort', onImgLoadErrorEvent, dialog );
 										original.setAttribute( 'src', newUrl );
 
-										// Query the preloader to figure out the url impacted by based href.
-										previewPreloader.setAttribute( 'src', newUrl );
-										dialog.preview.setAttribute( 'src', previewPreloader.$.src );
-										updatePreview( dialog );
+										if ( dialog.preview ) {
+											// Query the preloader to figure out the url impacted by based href.
+											previewPreloader.setAttribute( 'src', newUrl );
+											dialog.preview.setAttribute( 'src', previewPreloader.$.src );
+											updatePreview( dialog );
+										}
 									}
 									// Dont show preview if no URL given.
 									else if ( dialog.preview ) {

tests/tickets/12377/1.js

@@ -0,0 +1,76 @@
+/* bender-tags: editor,unit,image */
+/* bender-ckeditor-plugins: image,toolbar */
+
+(function() {
+	'use strict';
+
+	bender.editor = {};
+
+	bender.test( {
+		'test image dialog with removed preview and basic panel - image loaded': function() {
+			var bot = this.editorBot,
+				editor = this.editor,
+				src = '%BASE_PATH%_assets/img.gif',
+				src2 = '%BASE_PATH%_assets/logo.png';
+
+			bot.setHtmlWithSelection( '<p>[<img alt="foo" src="' + src + '" />]</p>' );
+
+			CKEDITOR.on( 'dialogDefinition', function( evt ) {
+				var dialogName = evt.data.name;
+				var dialogDefinition = evt.data.definition;
+
+				if ( dialogName == 'image' ) {
+					var infoTab = dialogDefinition.getContents( 'info' );
+					infoTab.remove( 'basic' );
+					infoTab.remove( 'htmlPreview' );
+				}
+			} );
+
+			bot.dialog( 'image', function( dialog ) {
+				assert.areSame( src, dialog.getValueOf( 'info', 'txtUrl' ) );
+
+				dialog.setValueOf( 'info', 'txtUrl', src2 );
+				dialog.getContentElement( 'info', 'txtAlt' ).focus();
+
+				// Focus will be moved asynchronously. IE8 might complain too.
+				wait( function() {
+					dialog.getButton( 'ok' ).click();
+					assert.areSame( '<p><img alt="foo" src="' + src2 + '" /></p>', bot.getData() );
+				}, 50 );
+			} );
+		},
+
+		'test image dialog with removed preview and basic panel - image load error': function() {
+			var bot = this.editorBot,
+				editor = this.editor,
+				src = 'img404.gif',
+				src2 = 'anotherimg404.gif';
+
+			bot.setHtmlWithSelection( '<p>[<img alt="foo" src="' + src + '" />]</p>' );
+
+			CKEDITOR.on( 'dialogDefinition', function( evt ) {
+				var dialogName = evt.data.name;
+				var dialogDefinition = evt.data.definition;
+
+				if ( dialogName == 'image' ) {
+					var infoTab = dialogDefinition.getContents( 'info' );
+					infoTab.remove( 'basic' );
+					infoTab.remove( 'htmlPreview' );
+				}
+			} );
+
+			bot.dialog( 'image', function( dialog ) {
+				assert.areSame( src, dialog.getValueOf( 'info', 'txtUrl' ) );
+
+				dialog.setValueOf( 'info', 'txtUrl', src2 );
+				dialog.getContentElement( 'info', 'txtAlt' ).focus();
+
+				// Focus will be moved asynchronously. IE8 might complain too.
+				wait( function() {
+					dialog.getButton( 'ok' ).click();
+					assert.areSame( '<p><img alt="foo" src="' + src2 + '" /></p>', bot.getData() );
+				}, 50 );
+			} );
+		}
+	} );
+})();