Implement #71517: Implement SVG support for getimagesize() and friends #16670
Conversation
nielsdos
force-pushed
the
req-71517
branch
2 times, most recently
from
69e135f to
93884c5
Compare
ext/standard/image.c
Outdated
| if (result->width_str) { | ||
| add_index_str(return_value, 0, result->width_str); | ||
| add_index_str(return_value, 1, result->height_str); | ||
| } else { | ||
| add_index_long(return_value, 0, result->width); | ||
| add_index_long(return_value, 1, result->height); | ||
| } |
There was a problem hiding this comment.
Is this the only case where width and height in the result are not long? If so, I guess this could be potentially problematic for some PHP user applications relaying on the result being int type.
There was a problem hiding this comment.
This does not affect existing uses of getimagesize(), this is unique to SVG. The reason being that in SVG you can define the width and height with a unit like "px" or "cm" etc. So in SVG I just always return it as a string.
There was a problem hiding this comment.
Yeah that's what I thought and that's actually my concern. I think it should work the same for all images and not have a different type just for SVG as it might lead to bugs if the function is used in a generic way. But loosing units is also not good. I guess it might be best to add another indexes for units.
And also how is this going to work for index 3 that should be height="yyy" width="xxx" that is save to add to IMG tag. I guess no units should be there either, right?
There was a problem hiding this comment.
Good points. I think adding a separate index makes sense.
And also how is this going to work for index 3 that should be height="yyy" width="xxx" that is save to add to IMG tag. I guess no units should be there either, right?
Right... Width and height attributes are always specified in pixels, so indeed it shouldn't actually output it if we have units; it should be fine if we have the "px" unit or no units (no units defaults to px).
|
Very welcome addition! Thank you! If you have some time, could you please fix the merge conflicts? |
This is necessary for future commits, when we extend the image handling to support extensions adding their own handlers. Also extend the struct with fields for when the width and height are not numbers but strings (e.g. for SVG).
This is modelled similarly to the password registry API. We have an array to which new handlers can be added, and when a built-in handler cannot handle the image, we try the handlers in the array. The standard module is in control of registering a new constant for the image file type so that no clashes can occur. It also updates the image file type count constant. As such, the registration may only happen during module startup.
This implements an SVG handler using the libxml reader API. This does not parse the entire document but instead uses a pull parser to locate the root element, check whether it's an svg root, do some extra sanity checks on the attribute, and fill in the php_gfxinfo structure.
| - Standard: | ||
| . getimagesize() now supports SVG images when ext-libxml is also loaded. | ||
| Similarly, image_type_to_extension() and image_type_to_extension() | ||
| now also handle IMAGETYPE_SVG. |
There was a problem hiding this comment.
Maybe add a note about the new array element width_unit and height_unit (for the doc people).
This implements feature request #71517
Expose gfxinfo as php_gfxinfo
This is necessary for future commits, when we extend the image handling
to support extensions adding their own handlers.
Also extend the struct with fields for when the width and height are not
numbers but strings (e.g. for SVG).
Add API to register custom image handlers
This is modelled similarly to the password registry API.
We have an array to which new handlers can be added, and when a built-in
handler cannot handle the image, we try the handlers in the array.
The standard module is in control of registering a new constant for the
image file type so that no clashes can occur. It also updates the image
file type count constant. As such, the registration may only happen
during module startup.
Implement SVG image handler
This implements an SVG handler using the libxml reader API. This does
not parse the entire document but instead uses a pull parser to locate
the root element, check whether it's an svg root, do some extra sanity
checks on the attribute, and fill in the php_gfxinfo structure.