Media upload needs to respect/hint at image limits

[yellowled]

Scenario: certain limits are set for images in configuration (max. width, height, filesize); an image is uploaded which overruns any of these limits. Result: error 500, the image is not uploaded.

There should be some kind of better error message/error handling, maybe even the possibility to resize said image to the limit set in the config?

See http://board.s9y.org/viewtopic.php?p=10441429 (German) for the original bug report.

[garvinhicking]

This is not really solvable, only by using a html5-based javascript file uploader that can check local limits. Modifying the images to adhere to the limits is not possible Afaik because it would need to be performed clientside.

On 15.01.2015, at 21:35, Matthias Mees notifications@github.com wrote:

Scenario: certain limits are set for images in configuration (max. width, height, filesize); an image is uploaded which overruns any of these limits. Result: error 500.

There should be some kind of better error message/error handling, maybe even the possibility to resize said image to the limit set in the config?

See http://board.s9y.org/viewtopic.php?p=10441429 (German) for the original bug report.

— Reply to this email directly or view it on GitHub.

[garvinhicking]

Also if a file exceeds the upload limit, apache itself will give the 500 errory There's nothing s9y/any php app could do about that...

On 15.01.2015, at 21:35, Matthias Mees notifications@github.com wrote:

Scenario: certain limits are set for images in configuration (max. width, height, filesize); an image is uploaded which overruns any of these limits. Result: error 500.

There should be some kind of better error message/error handling, maybe even the possibility to resize said image to the limit set in the config?

See http://board.s9y.org/viewtopic.php?p=10441429 (German) for the original bug report.

— Reply to this email directly or view it on GitHub.

[yellowled]

Well, in that case, aren't the configuration limits kind of pointless?

[garvinhicking]

They are meant to lie below the server-side limits, ie if your server allows 256M but in your blog you'd only want to allow 16M.

On 15.01.2015, at 22:16, Matthias Mees notifications@github.com wrote:

Well, in that case, aren't the configuration limits kind of pointless?

— Reply to this email directly or view it on GitHub.

[onli]

Are we sure the 500 is only generated when we go over server limits, not already when we go over s9y limits below those?

Apache limits being the culprit is not how I understood the bug report, would be a lot of bad luck if all those fell together.

[garvinhicking]

I'm pretty sure; there's no code in s9y to generate a HTTP/500 error.

[garvinhicking]

OK, I just tested - when I use the option "automatic size reduction" with the javascript, and configure a limit of 100kb but upload a 105kb file, then I get an error: https://www.dropbox.com/s/wkiiowyowbtilab/Screenshot%202015-01-16%2009.42.04.png?dl=0

I'm not so sure what the automatic size reduction thing should do, I believe it only applies to the image dimensions, not the file size, yes? So the error one gets there is right, because the filesize is exceeded? But I don't get an 500 error...

[yellowled]

That's not the error I meant – that one is indeed a server-side error 500. Did I by accident choose a file/server/limits which trigger an Apache error? But Hagen reported the same thing in the original bug report, I don't think he meant what you just screenshotted …

I'm not so sure what the automatic size reduction thing should do

It should look for the width/height/filesize limits in uploaded files and report back if a file is too wide/high/large with a sensible error message. Or, but that was just my idea (because I'm used to it in Processwire), it should shrink the image to fit the limits set in the config using ImageMagick or GD, of course still honoring dimensions etc.

However, we have talked about wanting to extend the media library and upload before, and I think we opted to postpone that because it's a large task, didn't we?

[ophian]

I cannot reproduce it for width 100 or even with+height 100. It looks fine for me.

[ophian]

If I use width 100 and use "Resize before Upload" true, the image is converted to ie 100x66. Which is fine also and would be the expected behaviour.

[ophian]

btw using "Resize before Upload" with not set max width and/or height does not work, say just uses the normal image upload. Maybe this should be communicated in the info box, which is: Resize images before the upload using Javascript. This will also change the uploader to use Ajax and thus remove the Property-Button

[yellowled]

Interestingly, renaming the file on upload also does not work if “Resize before upload” is used. It keeps the original file name.

This seems broken.

[ophian]

Interestingly, renaming the file on upload also does not work if “Resize before upload” is used. It keeps the original file name.

This works for me... but if I use Go & enter properties I have a strange look, since there is something making trouble...(can't post that as a screenshot here)

[yellowled]

Erm … there is no “Go & enter properties” if “Resize before upload” is active?

[ophian]

There is! If you do not have any values set in the max inputs, which I said already.

[ophian]

I have a strange look, since there is something making trouble...(can't post that as a screenshot here)

<section class="media_file_metadata clearfix" role="region">
                <h4>EXIF/IPTC/XMP</h4>
                            <h5>EXIF</h5>
                                <dl class="clearfix">
                                        <dt>CameraMaker</dt>
                    <dd>Canon</dd>
                                        <dt>CameraModel</dt>
                    <dd>Canon EOS 20D</dd>
                                        <dt>Orientation</dt>
                    <dd>Portrait</dd>
                                        <dt>XResolution</dt>
                    <dd>0.0072</dd>
                                        <dt>YResolution</dt>
                    <dd>0.0072</dd>
                                        <dt>Software</dt>
                    <dd>Adobe Photoshop CS3 Windows</dd>
                                        <dt>DateCreated</dt>
                    <dd>2008-03-24 13:40</dd>
                                        <dt>ExposureTime</dt>
                    <dd>0.005</dd>
                                        <dt>ApertureValue</dt>
                    <dd>7.40087890625</dd>
                                        <dt>ISOSpeedRatings</dt>
                    <dd>100</dd>
                                        <dt>MeteringMode</dt>
                    <dd>5</dd>
                                        <dt>FNumber</dt>
                    <dd>13</dd>
                                        <dt>ExposureProgram</dt>
                    <dd>4</dd>
                                        <dt>FocalLength</dt>
                    <dd>50</dd>
                                        <dt>WhiteBalance</dt>
                    <dd>0</dd>
                                        <dt>Flash</dt>
                    <dd>16</dd>
                                    </dl>
                                            <h5>XMP</h5>
                                <dl class="clearfix">
                                        <dt>ISOSpeedRatings</dt>
                    <dd><rdf:seq> <rdf:li>100</rdf:li> </rdf:seq></dd>
                                        <dt>Title</dt>
                    <dd>Snow Texture</dd>
                                        <dt>Creator</dt>
                    <dd>Matt Kunz</dd>
                                    </dl>
                                        </section>

Its the <dl> ... </dl> <h5>XMP</h5> not being cleared.

[yellowled]

That should fix the clearing at least.

[ophian]

That should fix the clearing at least.

yepp

but underneath the ml is shown broken too

[yellowled]

I saw that, and I was wondering why the hell we actually emit the ML there. It doesn't make much sense, especially since htting “Go” there will take the user to the “real” ML.

[ophian]

Hey, I just saw my small image jpg upload produced an authors title, which is https://github.com/s9y/Serendipity/blob/2.0/templates/2k11/admin/media_items.tpl#L59 and should not happen! So there must be a missing or wrong authors id around to make this happen...

In the serendipity_image table it is placed as authorid 1. Is that intended to happen here? I assume, Not!

Apart from this is a bug, this becomes important when you need a static sized clearfix equal_heights media_file_wrap block. while then you have images with and without the authors name span, which expands the grey media box.

[garvinhicking]

oops I just accidentally removed three or four comments right now while scrolling, my keyboard stuck the enter key. That is funny. :-D

Anyhow. @ophian Please post this as a seperate issue.

[ophian]

I saw that, and I was wondering why the hell we actually emit the ML there. It doesn't make much sense, especially since htting “Go” there will take the user to the “real” ML.

Did we change this behaviour already?

[yellowled]

No. And we forgot to discuss it, too. But it's assigned to Future anyway …

[yellowled]

I don't understand this issue anymore. Last inactivity over 20 months ago, kind of unreadable.

@onli @garvinhicking We should either close this if it's not an issue any more of reformulate it as a readable issue.

[onli]

20 month is too long: I'll close. Besides, by now we have an hmtl5/js-uploader with which we can do automatic resizing before the upload, which means we also could use that to give better warnings beforehand. But for that we have to restructure the ML upload workflow.