Updated: Comment #33.
- Right now
ImageInterface::getExtension()is used to check if an image is a supported image type or if the file is an image file. This is a bad pattern because:
- The extension is a filesystem component that can lie about the file's nature,
- A file having
.PNGextension on disk may have a JPEG or even a non-image content,
- Image type is not the same thing as the image extension.
- Being a filesystem component, the extension is not something that is linked to the toolkit (
ImageToolkitInterface) but to the image (
ImageInterface). Toolkits should have image types, images should have image type (talking about content nature) and extension (talking about external, filesystem file extension).
- Image types are determined (badly, by extension) in
GDToolkit::getInfo()based on a hardcoded list of extensions.
- If a custom module wants to add a new image type (extension), he normally has to extend
- The list of possible image types is something that should be provided by the toolkit. Some toolkits may provide more types than the other.
- Extension should be decoupled from image type. Images created from image resources (images not on disk) or temporary images have no extension while they still have an image type.
supportedTypesshould return a list of
IMAGETYPE_*constants. Note that
IMAGETYPE_*constants are PHP constants and are not bundled to GD library.
- Declare a simple
ImageInterfacewith implementation in
Imagethat will be used to test if a image type is supported by the current toolkit or if a file is a valid image.
ImageInterface::getExtension()should ALWAYS return the filesystem extension and will not be used for other purposes anymore. What would be the logic of returning an extension different from the real filesystem extension as long we will not use the extension as a way to check the image type or if a file is image?.
ImageInterface::getExtension()will fallback to
image_type_to_extension()for images created from resources or images without any extension on filesystem (like temporary files).
ImageInterface::getType()that returns the corresponding
User interface changes
No changes in UI.
ImageTookitInterface::supportedTypes()static method. Implementations must declare the supported image types.
ImageInterface::isSupported()method. This is used if a image type is supported OR if a file is a valid image.
ImageInterface::getType()method. Used to test if an image is of a specified type.
Original report by claudiu.cristea
It turned out in
Image::getExtension() needs to check if the image extension is a valid one (.png, .gif, .jpg). It's not the only place. Extension are already checked in
GDToolkit::getInfo(). This is a code duplication and, worst, a hardcoded list of extensions.
If a custom module wants to add a new image extension, he normally has to extend
GDToolkit and fork
GDToolkit::getInfo(). This is really ugly! But, if will get in, then will have to extend
Image too and fork the
::getExtension(). In fact is wrong because the list of possible extensions is something that should be provided by the toolkit. Some toolkits may provide more extensions than the other.
|PASSED: [[SimpleTest]]: [MySQL] 58,847 pass(es).|
|PASSED: [[SimpleTest]]: [MySQL] 59,239 pass(es).|
|PASSED: [[SimpleTest]]: [MySQL] 58,830 pass(es).|