Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use XHR as possible uploading mechanism in filebrowser plugin #1352

Merged
merged 28 commits into from
Dec 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9791777
Some working solution for using XHR with filebrowser.
Dec 12, 2017
7bf2375
Few improvements to using XHR with uploading images.
Dec 13, 2017
f3449bd
Unit test for adding XHR headers.
Dec 13, 2017
56259ab
Change condition to be independent from clipboard plugin.
Dec 15, 2017
b491787
Unit test for uploading images.
Dec 15, 2017
156fb10
Rename config flag, add docs.
Dec 15, 2017
ef08686
Code optimization, adding feature detection for file uploading.
Dec 15, 2017
ccfffa1
Preserve backward compatybility with returned data formats.
Dec 15, 2017
9642a81
Little changes in unit tests.
Dec 18, 2017
5fcd547
Manual tests for XHR upload.
Dec 18, 2017
9985680
Review fixes: chnage in behaviour to have 'form' as default action, b…
Dec 19, 2017
4cd6c1f
Review fixes: add error handling to loader, provide extra unit and ma…
Dec 20, 2017
f9f54df
Simplified file upload api detection.
mlewand Dec 20, 2017
40a1d06
Test: simplified manual test for xhr upload in filebrowser plugin.
mlewand Dec 20, 2017
d2da180
Tests: adjusted manual tests for 404 errors using XHR in filebrowser …
mlewand Dec 20, 2017
1886055
Docs: corrections for config.filebrowserUploadMethod API docs.
mlewand Dec 20, 2017
f299c36
Docs: adjusted the docs for a config property allowing to customize r…
mlewand Dec 20, 2017
ce4afc2
Renamed xmlHttpRequestHeaders config to fileTools_requestHeaders.
mlewand Dec 20, 2017
c2c7565
Docs: adjusted ticket reference.
mlewand Dec 21, 2017
cfd75e8
Simplified feature detection for file uploading over XHR. Minor other…
mlewand Dec 21, 2017
2c7bf71
Changelog entry.
mlewand Dec 21, 2017
4c95176
Tests: corrected file browser xhr upload manual tests.
mlewand Dec 21, 2017
8276465
Fixed unit tests for Edge, IEs.
mlewand Dec 21, 2017
76cacbe
Extracted common workarounds.
mlewand Dec 21, 2017
1eeab6f
Tests: removed dummy assertion.
mlewand Dec 22, 2017
5a577fe
Tests: corrected language config property.
mlewand Dec 22, 2017
cd36fe8
Tests: corrected manual test expected result.
mlewand Dec 22, 2017
1140dbe
Merge pull request #1369 from ckeditor/t/643
Comandeer Dec 22, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

## CKEditor 4.8.1

New Features:

* [#933](https://github.com/ckeditor/ckeditor-dev/issues/933): [File Browser](https://ckeditor.com/cke4/addon/filetools) plugin can now upload files using XHR requests. This allows for setting custom HTTP headers using [`config.fileTools_requestHeaders`](http://docs.ckeditor.test/#!/api/CKEDITOR.config-cfg-fileTools_requestHeaders) configuration option.

Fixed Issues:

* [#1342](https://github.com/ckeditor/ckeditor-dev/issues/1342): Fixed: [Balloon Toolbar](https://ckeditor.com/cke4/addon/balloontoolbar) should be re-positioned after a [change](https://docs.ckeditor.com/ckeditor4/docs/#!/api/CKEDITOR.editor-event-change) event.
Expand Down
11 changes: 9 additions & 2 deletions plugins/dialogui/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -743,8 +743,15 @@ CKEDITOR.plugins.add( 'dialogui', {
myDefinition.className = ( myDefinition.className ? myDefinition.className + ' ' : '' ) + 'cke_dialog_ui_button';
myDefinition.onClick = function( evt ) {
var target = elementDefinition[ 'for' ]; // [ pageId, elementId ]
if ( !onClick || onClick.call( this, evt ) !== false ) {
dialog.getContentElement( target[ 0 ], target[ 1 ] ).submit();

// If exists onClick in elementDefinition, then it is called and checked response type.
// If it's possible, then XHR is used, what prevents of using submit.
var responseType = onClick ? onClick.call( this, evt ) : false;

if ( responseType !== false ) {
if ( responseType !== 'xhr' ) {
dialog.getContentElement( target[ 0 ], target[ 1 ] ).submit();
}
this.disable();
}
};
Expand Down
75 changes: 63 additions & 12 deletions plugins/filebrowser/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
*/

( function() {
'use strict';
// Default input element name for CSRF protection token.
var TOKEN_INPUT_NAME = 'ckCsrfToken';

Expand Down Expand Up @@ -200,7 +201,7 @@
}
}

// The onlick function assigned to the 'Upload' button. Makes the final
// The onclick function assigned to the 'Upload' button. Makes the final
// decision whether form is really submitted and updates target field when
// file is uploaded.
//
Expand Down Expand Up @@ -296,22 +297,42 @@

if ( url ) {
var onClick = element.onClick;

// "element" here means the definition object, so we need to find the correct
// button to scope the event call
element.onClick = function( evt ) {
// "element" here means the definition object, so we need to find the correct
// button to scope the event call
var sender = evt.sender;
if ( onClick && onClick.call( sender, evt ) === false )
var sender = evt.sender,
fileInput = sender.getDialog().getContentElement( this[ 'for' ][ 0 ], this[ 'for' ][ 1 ] ).getInputElement(),
isFileUploadApiSupported = CKEDITOR.fileTools && CKEDITOR.fileTools.isFileUploadSupported;

if ( onClick && onClick.call( sender, evt ) === false ) {
return false;
}

if ( uploadFile.call( sender, evt ) ) {
var fileInput = sender.getDialog().getContentElement( this[ 'for' ][ 0 ], this[ 'for' ][ 1 ] ).getInputElement();

// Append token preventing CSRF attacks.
appendToken( fileInput );
return true;
// Use one of two upload strategies, either form or XHR based (#643).
if ( editor.config.filebrowserUploadMethod !== 'xhr' || !isFileUploadApiSupported ) {
// Append token preventing CSRF attacks.
appendToken( fileInput );
return true;
} else {
var loader = editor.uploadRepository.create( fileInput.$.files[ 0 ] );

loader.on( 'uploaded', function( evt ) {
var response = evt.sender.responseData;
setUrl.call( evt.sender.editor, response.url, response.message );
} );

// Return non-false value will disable fileButton in dialogui,
// below listeners takes care of such situation and re-enable "send" button.
loader.on( 'error', xhrUploadErrorHandler.bind( this ) );
loader.on( 'abort', xhrUploadErrorHandler.bind( this ) );

loader.loadAndUpload( url );

return 'xhr';
}
}


return false;
};

Expand All @@ -323,6 +344,12 @@
}
}

function xhrUploadErrorHandler( evt ) {
// `this` is a reference to ui.dialog.fileButton.
this.enable();
alert( evt.sender.message ); // jshint ignore:line
}

// Updates the target element with the url of uploaded/selected file.
//
// @param {String}
Expand Down Expand Up @@ -571,3 +598,27 @@
* @cfg {Number/String} [filebrowserWindowHeight='70%']
* @member CKEDITOR.config
*/

/**
* Defines a preferred option for file uploading in the [File Browser](https://ckeditor.com/cke4/addon/filebrowser) plugin.
*
* Available values:
*
* * `'xhr'` - XMLHttpRequest is used to upload file. Using this option allows to set up with Additional XHR headers with
* {@link CKEDITOR.config#fileTools_requestHeaders} option.
* * `'form'` - (default) File is uploaded by submitting a traditional `<form>` element.
* * `null` - The default method is used.
*
* Note: please be aware that `'xhr'` requires the [File Tools](https://ckeditor.com/cke4/addon/filetools) plugin to work
* properly. Without the plugin or using a browser that does not support
* {@link CKEDITOR.fileTools#isFileUploadSupported file uploading}, will fallback to the `'form'` method despite configuration
* option.
*
* // Modern browsers will use XMLHttpRequest to upload files.
* // IE8 and IE9 will use form submit even though the config option is set to 'xhr'.
* config.filebrowserUploadMethod = 'xhr';
*
* @since 4.8.1
* @cfg {String/null} [filebrowserUploadMethod=null]
* @member CKEDITOR.config
*/
44 changes: 42 additions & 2 deletions plugins/filetools/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@
editor.on( 'fileUploadRequest', function( evt ) {
var fileLoader = evt.data.fileLoader,
$formData = new FormData(),
requestData = evt.data.requestData;
requestData = evt.data.requestData,
configXhrHeaders = editor.config.fileTools_requestHeaders,
header;

for ( var name in requestData ) {
var value = requestData[ name ];
Expand All @@ -64,6 +66,12 @@
// Append token preventing CSRF attacks.
$formData.append( 'ckCsrfToken', CKEDITOR.tools.getCsrfToken() );

if ( configXhrHeaders ) {
for ( header in configXhrHeaders ) {
fileLoader.xhr.setRequestHeader( header, configXhrHeaders[ header ] );
}
}

fileLoader.xhr.send( $formData );
}, null, null, 999 );

Expand Down Expand Up @@ -856,7 +864,22 @@
*/
isTypeSupported: function( file, supportedTypes ) {
return !!file.type.match( supportedTypes );
}
},

/**
* Feature detection indicating whether current browser supports methods essential to send files over XHR request.
*
* @since 4.8.1
* @property {Boolean} isFileUploadSupported
*/
isFileUploadSupported: ( function() {
return typeof FileReader === 'function' &&
typeof ( new FileReader() ).readAsDataURL === 'function' &&
typeof FormData === 'function' &&
typeof ( new FormData() ).append === 'function' &&
typeof XMLHttpRequest === 'function' &&
typeof Blob === 'function';
} )()
} );
} )();

Expand Down Expand Up @@ -885,3 +908,20 @@
* @cfg {String} [fileTools_defaultFileName='']
* @member CKEDITOR.config
*/

/**
* Allows to add extra headers for every request made using {@link CKEDITOR.fileTools} API.
*
* Note that headers can still be customized per a single request, using the
* [`fileUploadRequest`](https://docs.ckeditor.com/ckeditor4/docs/#!/api/CKEDITOR.editor-event-fileUploadRequest)
* event.
*
* config.fileTools_requestHeaders = {
* 'X-Requested-With': 'XMLHttpRequest',
* 'Custom-Header': 'header value'
* };
*
* @since 4.8.1
* @cfg {Object} [fileTools_requestHeaders]
* @member CKEDITOR.config
*/
42 changes: 42 additions & 0 deletions tests/plugins/filebrowser/manual/uploadxhr.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<textarea id="classic"></textarea>

<pre id="output" style="background-color:lightgreen;"></pre>

<script>
// Mock XHR
var xhr = sinon.useFakeXMLHttpRequest();
requests = [];

xhr.onCreate = function( xhr ) {
requests.push( xhr );
};
window.onunload = function() {
xhr.restore();
}

var classic = CKEDITOR.replace( 'classic', {
fileTools_requestHeaders: {
'hello': 'world',
'foo': 'bar'
},
filebrowserUploadUrl: 'fake-url',
filebrowserUploadMethod: 'xhr',
on: {
instanceReady: function() {
if ( !CKEDITOR.fileTools.isFileUploadSupported ) {
bender.ignore();
}
}
}
} );

// Display XHR details when CKEDITOR process entire request.
classic.on( 'fileUploadRequest', function( evt ) {
var output = document.getElementById( 'output' ),
outputString = CKEDITOR.tools.array.reduce( requests, function( acc, item ) {
return acc + '<li>' + JSON.stringify( item.requestHeaders, null, 4 ) + '</li>';
}, '' );

output.innerHTML = '<ol>' + outputString + '</ol>';
}, null, null, 1000 )
</script>
24 changes: 24 additions & 0 deletions tests/plugins/filebrowser/manual/uploadxhr.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
@bender-tags: 4.8.1, feature, 643, tp3117
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, filebrowser, filetools, image, link, flash

----
1. Open image dialog
2. Go to upload tab
3. Select some image and send it to server
4. Close dialog (There might appear warning that image url is not set up)

**Expected:** Below editor will appear green div with listed headers attempted to send. You should see following headers for each case:

```
{
"hello": "world",
"foo": "bar"
}
```

**Unexpected:** Headers are not listed below.

Repeat those steps for Link and Flash plugin.

_Note:_ When a new upload request is made, it should be visible as a separate line below.
19 changes: 19 additions & 0 deletions tests/plugins/filebrowser/manual/uploadxhrwitherror.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<textarea id="classic">
</textarea>

<div id="output" style="background-color:lightgreen;"></div>

<script>
CKEDITOR.replace( 'classic', {
filebrowserUploadUrl: 'fake-url',
filebrowserUploadMethod: 'xhr',
on: {
instanceReady: function() {
if ( !CKEDITOR.fileTools.isFileUploadSupported ) {
bender.ignore();
}
}
}
} );

</script>
18 changes: 18 additions & 0 deletions tests/plugins/filebrowser/manual/uploadxhrwitherror.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
@bender-tags: 4.8.1, feature, 643, tp3117
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, filebrowser, filetools, image, link, flash

----
1. Open image dialog
2. Go to upload tab
3. Select an image and send it to server

**Expected:** An alert with 404 message is displayed.

5. Click "Send it to the Server" button again

**Expected:** An alert is shown again.

**Unexpected:** Second and further upload attempts won't trigger upload process, and alert is not shown.

Repeat those steps for Link and Flash plugin.
Loading