-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
add config-option for an image's maximum filesize when generating previews #13449
Conversation
works 👍 |
* Default is 50 | ||
* Set to -1 for no limit | ||
*/ | ||
'max_filesize_imagegd_previews' => 50, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
0fa3cac
to
0607ebe
Compare
@@ -944,6 +944,17 @@ | |||
'forwarded_for_headers' => array('HTTP_X_FORWARDED', 'HTTP_FORWARDED_FOR'), | |||
|
|||
/** | |||
* max file size for generating image previews with imagegd (default behaviour) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@carlaschroder please review this documentation -THX
Refer to this link for build results (access rights to CI server needed): |
👍 |
@oparoz @wakeup @Sugaroverdose An easy to review PR ;) Simply use a big image ( https://commons.wikimedia.org/wiki/File:Bath_from_alexandra_park.jpg ) and test if it doesn't has a preview while smaller images still have previews. ;) |
My problem is that it doesn't really fix #13218 I'd like this line to be fixed as part of the overall solution: https://github.com/owncloud/core/blob/master/lib/private/image.php#L470 And I'd like for these defaults to be modified to something reasonable like 2048x2048 so that we can start to generate sane previews for a browser/mobile environment. |
* Default is 50 | ||
* Set to -1 for no limit | ||
*/ | ||
'max_filesize_image_previews' => 50, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized that we have other preview related settings - please move this one to the others and name accordingly
preview_max_filesize_image
Sorry for the late notice
@oparoz we discussed already that fixing OC_Image is too critical for the current dev cycle - we can touch this class for OC8.1. This PR is a workaround for OC8 |
@DeepDiver1975 Line 470 is a logging operation which kills PHP. Surely this can be removed in OC8 or simplified to just output a simple string? The other settings I've mentioned have values of |
Ah - I see what you mean now - sorry - I was on a different track .... 🙈 It's indeed pointless to write the blob content to the log here : facepalm: @georgehrke please submit a second pr where this log line is simply killed @oparoz feel free to submit this pr as well if you are interested - THX |
0607ebe
to
3af8bde
Compare
|
The inspection completed: 4 new issues |
Refer to this link for build results (access rights to CI server needed): |
👍 |
add config-option for an image's maximum filesize when generating previews
@karlitschek backport? Was reported as issue on oc7 |
backport would be good |
stable7 2d2e024 - tested works |
fixes #13218