-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
Drop getimagesize()[3] (width and height for img) #17707
Comments
This is potentially quite big BC break so it will surely require RFC. I would probably just leave it and document accordingly. And we want to add extra dimension for SVG so it might not be good idea to just drop it. |
Instead of dropping this, how hard would it be to fix the orientation issue? |
It's in exif so it would probably make sense to get that using exif ext but exif is not always enabled and it's a different ext so we would need some hooking around that. I wonder if there would be also some perf impact of always getting the exif data. |
ext/exif only parses JFIF/TIFF, but other formats also support Exif (at least PNG); and there may be other metadata formats with similar information. I don't think it's worth to fix that (for now; GD ignores such meta information, too). |
I think we should just note in the docs that it doesn't take into account orieantation but maybe we could introduce another param so users can provide orientation themeself so it would get swapped potentially. |
Yeah, that makes sense to me. See php/doc-en#4450.
In my opinion, that would not really be helpful. Users can just swap the first and second element, if they already care to read the Orientation flag (or whatever else may apply in the metadata; I don't know if there are alternatives). And if we provide such a param, the next feature request will be for ext/gd to also support such a param (especially since I think this ticket can be closed. Having it documented is good enough for me. |
I tend to agree that documenting this is good enough, at least for now. |
Description
From the getimagesize() docs:
Besides that this is not really a convenience feature, it may easily give outright wrong information, since e.g. the EXIF Orientation tag is ignored. To wit:
Using these dimension actually defeats the whole point of using these attributes (namely to inform the browser of the expected dimensions upfront for layout purposes).
I suggest to drop this index from the return value. While that is clearly a BC break (and cannot be mitigated by a deprecation), it is not that bad, since index 3 is the last numerical index; further elements have string keys.
PS: a less drastic measure could be to just provide an empty string. That is less likely to break existing code.
The text was updated successfully, but these errors were encountered: