Only fetch the width and height when it is a file #116
Conversation
laoneo
changed the title
| $props = JImage::getImageFileProperties($path); | ||
| $obj->width = $props->width; | ||
| $obj->height = $props->height; | ||
| } |
There was a problem hiding this comment.
This will fix the error for directories but what happens in case of non-image files like html, mp4, pdf? Perhaps we need an isImage() check.
There was a problem hiding this comment.
Most information you find on the net do say that getimagesize is the function to check if it is an image or not. That is what the JImage lib is doing. I'm not sure if it would be enough to rely on the filetyope or mime type only.
|
Then an exception is thrown, which we catch. |
|
Hm, is that what we want? Imho exceptions should be thrown (and catched) only in case of serious errors (when the app is unable to recover) and not as part of an expected and valid application flow. |
laoneo
changed the title
|
|
||
| try | ||
| if (strpos($obj->mime_type, 'image/') === 0 && in_array(strtolower($obj->extension), array('jpg', 'jpeg', 'png', 'gif', 'bmp'))) |
There was a problem hiding this comment.
Can we put this in an isImage method to be more readable?
| { | ||
| // Get the image properties | ||
| $props = JImage::getImageFileProperties($path); | ||
| $obj->width = $props->width; | ||
| $obj->height = $props->height; | ||
| } | ||
| catch (Exception $e) |
There was a problem hiding this comment.
What kind of exception do we catch here?
There was a problem hiding this comment.
I'v removed that and opened issue #117 as we don't have a defined way of exception handling.
|
thanks @laoneo |
When error reporting is set to maximum, then an error is thrown because the image size can't be determined on a folder. This pr fixes that.
The error is:
Notice: getimagesize(): Read error! in /mm/libraries/vendor/joomla/image/src/Image.php on line 215