Updated: Comment #N
Follow-up from #2217783: Remove $extension and $mimeType from Image, use image_type_to_xxx function in the getters instead.
In Image, we are currently using the IMAGETYPE_* constants to indicate the format of image being processed. While this allows for uniformity within PHP, it also creates some problems:
1) the default file extensions that are retrievable from the IMAGETYPE_* constants via image_type_to_extension() are sometimes questionable --> e.g. IMAGETYPE_JPEG will return a 'JPEG' extension (uppercase), while common practice is to use mainly 'jpg' as a file extension (lowercase), or 'jpeg' (lowercase too).
2) it strictly links manageable image formats to those supported by PHP (mainly via GD extension) --> other toolkits may be able to manage more image formats, e.g. Imagemagick supports pdf.
Conversely, in core we have in file.mimetypes.inc a full list of file extensions and corresponding mime types, retrievable via file_mimetype_mapping(). This list is maintained as part of Drupal core, and also allows alterability via hook_file_mimetype_mapping_alter(), so that modules can freely extend/adapt it.
An alternative to current status could be;
file_mimetype_mapping (i.e. 'png', 'gif', 'jpeg', 'jpg', etc.)getInfo() will return the 'format' of the image to the Image class processInfo(), compliant to point 1 (this is simply Unicode::strtolower(image_type_to_extension($type, FALSE));)file_mimetype_mappingNone.
TBD
Comments
Comment #1
mondrakeComment #2
fietserwinIMO, as spin-off of the parent issue,m this issue should be about 2 things:
- Representation of image type/format in a way that allows contrib modules to go further than those defined by PHP (with only GD in mind).
- The image system is about image manipulation not about file handling. If information about an image file is needed, think of extension or mime type, it should be approached as a file or stream (wrapper) not as an image object.
The latter would indicate to simply remove getMimeType() from Image, even if it would be used internally for representing the image type/format. Though a method should remain to query the image format/type, that should not be used in file serving.
For some history regarding storing the type/format of an image see: #2066219: Decouple image type from image extension.
regarding the proposed solution, I'm not happy with the fact that multiple values can be used to denote the same (jpeg, jpe, jpg). This is not acceptable and suggests to use the mime type (from that same hook), not the extension. The internal numeric keys are better not used, they should stay internal.
Comment #3
mondrake@fietserwin re #2
So you're proposing to use mime type internally within Image, right? Interesting... I need to think about it.
In any case, directly or indirectly, we need to get a list of supported file extensions via the toolkit. That's needed by file validators and only the toolkit can define which ones it supports.
Comment #4
fietserwinYes, Image internally needs some property to store the current image format.
IMAGETYPE constants have some drawbacks, see your remarks in the issue summary. We could replace that with another way of uniquely identifying the type of the image at hand. So, yes mime type comes to my mind (see the issue I mentioned in #1) and is preferable over extension (not unique, though we could restrict our allowed values to a set of preferred extensions) and over the list of known/supported by Drupal extensions from file_mimetype_mapping() (as proposed solution, also not unique). However, in the other issue it was also decided to not use mime type and I must say that the IMAGETYPE constants do have some nice features, namely the image_type_to_... lookup functions.
If we decide to keep the IMAGETYPE constants we must keep in mind that if contrib wants to extend support to not known image types (e.g. webp, pdf), it will probably do so by defining its own constants. This will require overriding all places where image_type_to_... functions are used, because they will fail with these self defined constants! Currently this includes Image (and thus ImageFactory will need to be overridden as well) and file.module! The latter is tricky to impossible. In this case we'd better refactor supportedTypes() or create another method alongside that one, to remove the image_type_to_extension() call from file.module.
If we decide to go for mime type internally, we must write a number of lookup functions ourselves and come with a good story to undo/redo #2066219: Decouple image type from image extension. For me the above are reasons enough.
Comment #5
fietserwinMarking as duplicate of #2211227: Refactor image and imagetoolkit: isExisting, isSupported, supportedTypes, getMimeType (and [##2217783]).