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

Resize images on client-side before uploading #7734

Merged
merged 1 commit into from Apr 2, 2018

Conversation

Fensterbank
Copy link
Contributor

@Fensterbank Fensterbank commented Mar 7, 2018

This pull requests implements client-side resizing of images before uploading them.
While FineUploader provides scaling of selected images, its internal image resize code delegates to the drawImage method on the browser's native CanvasRenderingContext2D object which means linear interpolation in most browser.

So for quality and performance reasons I added the Pica-Library which is makeing a much better resizing and uses a Webworker for it.
Since we support Internet Explorer 11 and Pica / FineUploader are working with promises here I added Lie as a small, performant promise library.
I'm not so sure about this or if there is a better way to just make a Promise polyfill for IE11 without affect the modern browsers.

Each image is resized to a max width or max height of 3072 pixels before uploading, so in the future the original images stored on the server will never exceed this size.
In my tests I successfully selected big photos (> 12 MB) to upload. The biggest file sent to the server was 2,4 MiB.

Scaling is disabled when running in a PhantomJS instance because it didn't seem to work at all.

This fixes #5621 and makes #4997 less important.

@Flaburgan
Copy link
Member

Flaburgan commented Mar 7, 2018

Good news: we don't especially support IE11. If users are able to upload images without resizing them with IE11, we're good enough. We would even tend to not support it at all. FTR on https://framasphere.org from the 1st of January to the 7th of March 2018, IE11 represented 1674 hits = 4.1% of the visitors.

@Fensterbank
Copy link
Contributor Author

Okay. I'm not sure, if we want to have that it's possible to upload the original image without any limitation.
It could easily happen that huge files are saved on the server which the 4 MB limit prevented.

But yes, only for IE11 we don't need another Promise library. In fact the resizing based on linear interpolation brought with FineUploader is enough. Technically it's not the best quality but in my test it was much faster than resizing in IE11 with Pica, so we just use what FineUploader brings.

@@ -542,7 +542,7 @@ describe("app.views.Publisher", function() {
var upload_view = this.view.viewUploader;
this.uploader = {
onProgress: _.bind(upload_view.progressHandler, upload_view),
onSubmit: _.bind(upload_view.submitHandler, upload_view),
onUpload: _.bind(upload_view.uploadHandler, upload_view),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Extra space before value for key 'onUpload'.
  • Identifier 'upload_view' is not in camel case.

@@ -560,7 +560,7 @@ describe("app.views.Publisher", function() {

context('submitting', function() {
beforeEach(function() {
this.uploader.onSubmit(null, 'test.jpg');
this.uploader.onUpload(null, 'test.jpg');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use doublequote.

@denschub
Copy link
Member

denschub commented Mar 7, 2018

Good news: we don't especially support IE11. If users are able to upload images without resizing them with IE11, we're good enough

I mean... if we only support the latest of the major browser, it's fine by me. And that would be Edge! And since Edge has Promise-support, that's fine. At least for me.

However, I'm not sure about your approach. Is there an option to only resize when the image is larger than 4MB and not touch the image at all if it's below the limit?

@Fensterbank
Copy link
Contributor Author

However, I'm not sure about your approach. Is there an option to only resize when the image is larger than 4MB and not touch the image at all if it's below the limit?

The options can only be set at initialization. But it seems we could achieve this by calling the scaleImage method if necessary in the submit handler and then trigger the upload.

Since scaleImage returns a promise, we will definitely lose any scaling in IE11. But we can give the IE11 users the 4 MB validation as it has been before and the moderm browser will scale if necessary.

I'll take a look at it!

@Fensterbank
Copy link
Contributor Author

As suggested by @denschub the picture is now only scaled if its size is above the 4 MB size limit.
Smaller images will be uploaded untouched at it has been before.

If the browser does not support Promises or canvas scaling the logic is the same as always.
This means an IE11 user trying to upload a huge image will see the file too big error message while the modern browsers will scale.

Unfortunately we have a bug in the FineUploader library. Firefox for Android is considered to be a stock browser for which the scaling feature is disabled.
This means uploading pictures directly from the camera of an Android smartphone does currently only work in Chrome for Android.

* @param {param} - The object to check
* @returns {boolean}
*/
this.func = param => (typeof(param) === 'function');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Unary word operator 'typeof' must be followed by whitespace.
  • Strings must use doublequote.

});
}


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More than 1 blank line not allowed.

@SuperTux88 SuperTux88 added this to the 0.8.0.0 milestone Mar 13, 2018
@denschub denschub merged commit 6d55b15 into diaspora:develop Apr 2, 2018
denschub added a commit that referenced this pull request Apr 2, 2018
Resize images on client-side before uploading
@denschub
Copy link
Member

denschub commented Apr 2, 2018

Sorry it took me a bit to review, but it looks good to me. Merged to develop, and we'll see if we break things. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resize images on client-side before uploading
5 participants