Merge pull request #1840 from ckeditor/t/1835

Fix integration with CKFinder
ckeditor · Mar 26, 2018 · 72a83a5 · 72a83a5
2 parents 7eecac9 + 1ae994a
commit 72a83a5
Show file tree

Hide file tree

Showing 5 changed files with 196 additions and 2 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -3,6 +3,10 @@
 
 ## CKEditor 4.9.1
 
+Fixed Issues:
+
+* [#1835](https://github.com/ckeditor/ckeditor-dev/issues/1835): Fixed: Integration between [CKFinder](https://ckeditor.com/ckeditor-4/ckfinder/) and [File Browser](https://ckeditor.com/cke4/addon/filebrowser) plugin does not work.
+
 ## CKEditor 4.9
 
 New Features:

diff --git a/plugins/filebrowser/plugin.js b/plugins/filebrowser/plugin.js
@@ -137,6 +137,19 @@
 		return url + ( ( url.indexOf( '?' ) != -1 ) ? '&' : '?' ) + queryString.join( '&' );
 	}
 
+	// Function sniffs for CKFinder URLs, and adds required parameters if needed (#1835).
+	//
+	// @since 4.9.1
+	// @param {String} url CKFinder's URL.
+	// @returns {String} Decorated URL.
+	function addMissingParams( url ) {
+		if ( !url.match( /command=QuickUpload/ ) || url.match( /(\?|&)responseType=json/ ) ) {
+			return url;
+		}
+
+		return addQueryString( url, { responseType: 'json' } );
+	}
+
 	// Make a string's first character uppercase.
 	//
 	// @param {String}
@@ -328,7 +341,7 @@
 								loader.on( 'error', xhrUploadErrorHandler.bind( this ) );
 								loader.on( 'abort', xhrUploadErrorHandler.bind( this ) );
 
-								loader.loadAndUpload( url );
+								loader.loadAndUpload( addMissingParams( url ) );
 
 								return 'xhr';
 							}
@@ -345,9 +358,15 @@
 	}
 
 	function xhrUploadErrorHandler( evt ) {
+		var response = {};
+
+		try {
+			response = JSON.parse( evt.sender.xhr.response ) || {};
+		} catch ( e ) {}
+
 		// `this` is a reference to ui.dialog.fileButton.
 		this.enable();
-		alert( evt.sender.message ); // jshint ignore:line
+		alert( response.error ? response.error.message : evt.sender.message ); // jshint ignore:line
 	}
 
 	// Updates the target element with the url of uploaded/selected file.

diff --git a/tests/plugins/filebrowser/manual/uploadckfinder.html b/tests/plugins/filebrowser/manual/uploadckfinder.html
@@ -0,0 +1,29 @@
+<h2>Classic editor</h2>
+
+<textarea id="classic"></textarea>
+
+<h2>Inline editor</h2>
+
+<div id="inline" contenteditable="true">
+	<p>Click me</p>
+</div>
+
+<script>
+	var ignored = false,
+		config = {
+			filebrowserBrowseUrl: '//ckeditor.test/ckfinder/ckfinder.html',
+			filebrowserUploadUrl: '//ckeditor.test/ckfinder/core/connector/php/connector.php?command=QuickUpload&type=Files',
+			on: {
+				instanceReady: function() {
+					if ( !CKEDITOR.fileTools.isFileUploadSupported && !ignored ) {
+						bender.ignore();
+						ignored = true;
+					}
+				}
+			}
+		};
+
+	CKEDITOR.replace( 'classic', config );
+	CKEDITOR.inline( 'inline', config );
+
+</script>
diff --git a/tests/plugins/filebrowser/manual/uploadckfinder.md b/tests/plugins/filebrowser/manual/uploadckfinder.md
@@ -0,0 +1,23 @@
+@bender-tags: 4.9.1, bug, 1835
+@bender-ui: collapsed
+@bender-ckeditor-plugins: wysiwygarea, toolbar, filebrowser, filetools, image, link, flash, floatingspace
+
+# CKFinder upload
+
+1. Open Image dialog using the Image button.
+2. Go to the Upload tab.
+3. Choose an image to be uploaded.
+4. Click the "Send it to the Server" button.
+
+## Expected
+
+No errors are displayed.
+
+CKFinder _might_ report an alert like: "A file with the same name already exists. The uploaded file was renamed to "city-wallpaper-47(8).jpg"." - that's fine it only means that the file was renamed.
+
+## Unexpected
+
+There should be no errors, e.g. like (but not limited to):
+
+* "Network error occurred during file upload."
+* "Incorrect server response."
diff --git a/tests/plugins/filebrowser/uploadbutton.js b/tests/plugins/filebrowser/uploadbutton.js
@@ -44,6 +44,26 @@
 				language: 'en'
 			}
 		},
+
+		xhrCkf: {
+			config: {
+				filebrowserUploadUrl: 'upload/?command=QuickUpload'
+			}
+		},
+
+		xhrCkfJson: {
+			config: {
+				filebrowserUploadUrl: 'upload/?command=QuickUpload&responseType=json'
+			}
+		},
+
+		submitCkf: {
+			config: {
+				filebrowserUploadUrl: 'upload/?command=QuickUpload',
+				filebrowserUploadMethod: 'form'
+			}
+		},
+
 		submit: {
 			config: {
 				filebrowserUploadUrl: 'foo',
@@ -126,6 +146,91 @@
 			} );
 		},
 
+		// (#1835)
+		'test for XHR request to CKFinder (no responseType parameter)': function() {
+			if ( !CKEDITOR.fileTools.isFileUploadSupported ) {
+				assert.ignore();
+			}
+
+			var editor = this.editors.xhrCkf,
+				bot = this.editorBots.xhrCkf;
+
+			editor.addCommand( 'testDialog', new CKEDITOR.dialogCommand( 'testDialog' ) );
+			bot.dialog( 'testDialog', function( dialog ) {
+				var sendButton = dialog.getContentElement( 'Upload', 'uploadButton' ),
+					inputStub = mockInput( dialog );
+
+				// Execute just after XHR request is generated;
+				editor.once( 'fileUploadRequest', function() {
+					resume( function() {
+						assert.isMatching( /responseType=json/g, this.requests[ 0 ].url, 'responseType parameter was added' );
+						dialog.hide();
+						inputStub.restore();
+					} );
+				}, null, null, 1000 );
+
+				sendButton.click();
+				wait();
+			} );
+		},
+
+		// (#1835)
+		'test for XHR request to CKFinder (responseType parameter present)': function() {
+			if ( !CKEDITOR.fileTools.isFileUploadSupported ) {
+				assert.ignore();
+			}
+
+			var editor = this.editors.xhrCkfJson,
+				bot = this.editorBots.xhrCkfJson;
+
+			editor.addCommand( 'testDialog', new CKEDITOR.dialogCommand( 'testDialog' ) );
+			bot.dialog( 'testDialog', function( dialog ) {
+				var sendButton = dialog.getContentElement( 'Upload', 'uploadButton' ),
+					inputStub = mockInput( dialog );
+
+				// Execute just after XHR request is generated;
+				editor.once( 'fileUploadRequest', function() {
+					resume( function() {
+						assert.areSame( 1, this.requests[ 0 ].url.match( /responseType=json/g ).length,
+							'responseType parameter count' );
+						dialog.hide();
+						inputStub.restore();
+					} );
+				}, null, null, 1000 );
+
+				sendButton.click();
+				wait();
+			} );
+		},
+
+		// (#1835)
+		'test for XHR request not to CKFinder (responseType parameter)': function() {
+			if ( !CKEDITOR.fileTools.isFileUploadSupported ) {
+				assert.ignore();
+			}
+
+			var editor = this.editors.xhr,
+				bot = this.editorBots.xhr;
+
+			editor.addCommand( 'testDialog', new CKEDITOR.dialogCommand( 'testDialog' ) );
+			bot.dialog( 'testDialog', function( dialog ) {
+				var sendButton = dialog.getContentElement( 'Upload', 'uploadButton' ),
+					inputStub = mockInput( dialog );
+
+				// Execute just after XHR request is generated;
+				editor.once( 'fileUploadRequest', function() {
+					resume( function() {
+						assert.isNotMatching( /responseType=json/g, this.requests[ 0 ].url, 'responseType parameter is not added' );
+						dialog.hide();
+						inputStub.restore();
+					} );
+				}, null, null, 1000 );
+
+				sendButton.click();
+				wait();
+			} );
+		},
+
 		'test for submit form': function() {
 			var editor = this.editors.submit,
 				bot = this.editorBots.submit;
@@ -143,6 +248,20 @@
 			} );
 		},
 
+		// (#1835)
+		'test form action with CKFinder': function() {
+			var editor = this.editors.submitCkf,
+				bot = this.editorBots.submitCkf;
+
+			editor.addCommand( 'testDialog', new CKEDITOR.dialogCommand( 'testDialog' ) );
+			bot.dialog( 'testDialog', function( dialog ) {
+				var input = dialog.getContentElement( 'Upload', 'upload' );
+
+				assert.isNotMatching( /responseType=json/g, input.action, 'responseType parameter is not added' );
+				dialog.hide();
+			} );
+		},
+
 		'test for xhr loader error': function() {
 			if ( !CKEDITOR.fileTools.isFileUploadSupported ) {
 				assert.ignore();