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

[8.0][FIX][web] Detect image MIME. #12918

Closed
wants to merge 1 commit into from

Conversation

yajo
Copy link
Contributor

@yajo yajo commented Jul 27, 2016

Description of the issue/feature this PR addresses:

We are creating a control to download an image, but the server is not telling the right MIME type in this controller.

Current behavior before PR:

When visiting a URL such as http://localhost/web/binary/image?model=res.users&id=1&field=image_small&t=1469603889889, the controller always says the image is image/png, while sometimes it is JPG.

Desired behavior after PR is merged:

The server can know if it is PNG, GIF or JPEG, and outputs the correct header.

Odoo v8 lacks a MIME autodetect method, but backporting the most relevant parts from v9, we can make it detect the image type and serve the correct header, to allow some client-side manipulations to work fine.

I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@Tecnativa @kutyel

yajo added a commit to Tecnativa/odoo that referenced this pull request Jul 27, 2016
Try to detect the mimetype by binary content before falling back to default.

This covers the same use case as odoo#12918, but using new tools in v9.
Odoo v8 lacks a MIME autodetect method, but backporting the most relevant parts from v9, we can make it detect the image type and serve the correct header, to allow some client-side manipulations to work fine.
@yajo yajo force-pushed the 8.0-web-fix_hardcoded_image_mime branch from 3551b99 to 65dcc7e Compare July 27, 2016 09:38
@pedrobaeza
Copy link
Collaborator

👍 without a doubt

@yajo yajo changed the title [FIX][web] Detect image MIME. [8.0][FIX][web] Detect image MIME. Jul 27, 2016
@lasley
Copy link
Contributor

lasley commented Aug 15, 2016

👍

@mart-e
Copy link
Contributor

mart-e commented Dec 6, 2016

Hello,

We do not wish to change the way the web client handles mimetype in 8.0. It is a very side effect-prone changes with a low ratio value/risk. (We had issues in the past changing this).
I will then refuse this change for 8.0.

We have changed quite a bit the way we handle binaries in 10.0. If there is still issues about it, feel free to adapt and resubmit. We will check it there.

Regards

@mart-e mart-e closed this Dec 6, 2016
@pedrobaeza pedrobaeza deleted the 8.0-web-fix_hardcoded_image_mime branch December 6, 2016 14:53
@pedrobaeza
Copy link
Collaborator

Can you please point to the commit that changes the behavior to try, @mart-e ?

@mart-e
Copy link
Contributor

mart-e commented Dec 6, 2016

You mean the difference in 10.0? The route /web/binary/image has been deprecated and there is an option to specify the mimetype for instance. The new code is quite different and we can not simply apply the 8.0 patch.
But I am not saying the issue is not present in v10.

@pedrobaeza
Copy link
Collaborator

OK, I'll check this new code. Thanks for the pointer.

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.

4 participants