Skip to content
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

Image processing is inconsistent between FilesProcessor and Media/ImageViewHelper #247

Closed
NamelessCoder opened this issue Mar 24, 2021 · 1 comment · Fixed by #724
Closed
Labels
enhancement New feature or request
Milestone

Comments

@NamelessCoder
Copy link

Note, the following is reported with .ai / .eps formats which I'm aware aren't natively supported as images. However, the issue still remains the same - FilesProcessor seems to use the correct API for processing files whereas Media/ImageViewHelper takes a shortcut that potentially causes errors when processing files that are image-incompatible, e.g. videos or PDF.

An alternative problem description could also be "Image processing ignores TYPO3_CONF_VARS.GFX.imagefile_ext when determining if file is an image".

Problem description:

Files of types like "ai" or "eps" do not get scaled when using FilesProcessor, but do get scaled when using MediaViewHelper. This is due to using different APIs:

  • FilesProcessor will use FileUtility which has a condition that checks File->getType equal to "image" before doing image processing; if matched it then uses ImageService to apply processing.
  • But MediaViewHelper directly uses ImageService to perform processing and will therefore scale/crop literally any file that can be cropped/scaled by the configured image processing mechanism (im, gm, gd)

This causes two side effects:

Media processing is inconsistent between FilesProcessor and MediaViewHelper
MediaViewHelper will explicitly treat every type of media as an "image", including videos/pdf/etc.

Long-term solution:

Solve inconsistency between FilesProcessor and MediaViewHelper. Problem is, FilesProcessor uses FileUtility to process which supports all file formats, but contains a condition that only uses File->getType to determine if file is an image. If not an image, no scaling/cropping is performed.

  • Solution 1: Make MediaViewHelper also use FileUtility to do processing for consistency. Does not solve the issue that File->getType must be "image" or no processing happens, but does introduce full consistency between the two APIs.
  • Solution 2: Do solution 1 + add a condition in FileUtility that also considers any File that has a file extension that is recorded in TYPO3_CONF_VARS.GFX.imagefile_ext as being an "image", regardless of File->getType return value. Solves both the inconsistency and the problem of eps/ai/etc. not being detected as image regardless of configuration.
  • Solution 3: Do solution 1 + change determination of "type" for File when uploaded, to also consider any file extension recorded in TYPO3_CONF_VARS.GFX.imagefile_ext to be an "image". Solves both issues but does not retroactively fix files that are already uploaded (solution 2 probably preferable).
@lukaszuznanski lukaszuznanski added the enhancement New feature or request label Jul 2, 2021
@lukaszuznanski
Copy link
Collaborator

Thanks, I agree that there is inconsistency, solution 2 looks good and should be implemented.

@lukaszuznanski lukaszuznanski added this to the 3.0.0 milestone Oct 15, 2021
twoldanski added a commit that referenced this issue May 9, 2024
- new simplified output (off by default)
- new options to process files (custom autogenerate feature), filter & alias properties for much cleaned up output. Useful with large sites with a lot of images on page. See options Documentation/Developer/Images.rst
- code refactoring
- allows to process more image formats

Resolves: #247, #617
twoldanski added a commit that referenced this issue May 9, 2024
- new simplified output (off by default)
- new options to process files (custom autogenerate feature), filter & alias properties for much cleaned up output. Useful with large sites with a lot of images on page. See options Documentation/Developer/Images.rst
- code refactoring
- allows to process more image formats

Resolves: #247, #617
lukaszuznanski pushed a commit that referenced this issue May 10, 2024
- new simplified output (off by default)
- new options to process files (custom autogenerate feature), filter & alias properties for much cleaned up output. Useful with large sites with a lot of images on page. See options Documentation/Developer/Images.rst
- code refactoring
- allows to process more image formats

Resolves: #247, #617
twoldanski added a commit that referenced this issue May 10, 2024
- new simplified output (off by default)
- new options to process files (custom autogenerate feature), filter & alias properties for much cleaned up output. Useful with large sites with a lot of images on page. See options Documentation/Developer/Images.rst
- code refactoring
- allows to process more image formats
- add additional tests

Resolves: #247, #617
lukaszuznanski pushed a commit that referenced this issue May 13, 2024
* [FEATURE] Improve file processing

- new simplified output (off by default)
- new options to process files (custom autogenerate feature), filter & alias properties for much cleaned up output. Useful with large sites with a lot of images on page. See options Documentation/Developer/Images.rst
- code refactoring
- allows to process more image formats

Resolves: #247, #617

* [FEATURE] Improve file processing

- new simplified output (off by default)
- new options to process files (custom autogenerate feature), filter & alias properties for much cleaned up output. Useful with large sites with a lot of images on page. See options Documentation/Developer/Images.rst
- code refactoring
- allows to process more image formats
- add additional tests

Resolves: #247, #617
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants