Bug #52729

Image viewhelper not cropping if property maximumWidth is set

Added by Jan-Erik Revsbech almost 2 years ago. Updated 11 months ago.

Status:Closed Start date:2013-10-12
Priority:Must have Due date:
Assigned To:Adrian Föder % Done:

0%

Category:-
Target version:-

Description

During working on a new Neos site I experience a problem with the builtin Media viewhelper. In a custom Element (nodetype) I specify an imagenode and render it with

<media:image image="{image}" alt="My image image" maximumWidth="200" />

But cropping given in the Neos image inspector is not respected when the maximumWidth attribute is set! This happens because the viewhelper is i handed a "ImageVariant" domain model, and if the maximumWidth is set (and is less than the images width and allowUpScale is FALSE) then the getThumbnail method og the ImageVariant is called.
For ImageVarians, this method is correctly documented as "Creates a thumbnail of the original image" and thus is forgets all about the variants ProcessingInstructions which are needed for the cropping.

I think this is a bug. The easyest way to reproduce it is setting the following TypoScript

prototype(TYPO3.Neos.NodeTypes:Image).maximumImage = 200

(default is 2560)

and then insert a Image content element, crop it with the very cool Image Inspector, and watch how the cropping is not in effect any longer.

Can any other confirm this behaviour, and that is it undesired?

If this is a bug, then I suggest either of these solutions:

1: Change the viewhelper, to be aware of ImageVarians, and if an ImageVariant is given, then add the new transformations, instead of creating a new ImageVarians
2: Change the viewhelper to not actually use Thumbnails, but do a scale transformations directly using a \TYPO3\Media\Domain\Service\ImageService
3: Change the behaviour of the getThumbnail method on the ImageVariant (this will probably have sideeffects)
4: Add a new method that generates a thumbnail that respects the exising transformations on the ImageVariant.

In my opinoin, option 2 is the most clean.

I will create the patchset if any other can confirm the problem and we can agree on the correct solution.

I'm running alpha5 and, I'm aware that a lot happened the last week, but from what I can see in the Gitlog, this is not changed.

History

#1 Updated by Adrian Föder over 1 year ago

  • Status changed from New to Accepted
  • Assigned To changed from Jan-Erik Revsbech to Adrian Föder
  • Priority changed from Should have to Must have

can totally confirm this, came here for this report. I do my best to take care.

#2 Updated by Jan-Erik Revsbech over 1 year ago

Let me know if you need any help. I can fix it pretty fast, if we can agree on which solution is best.

#3 Updated by Dominique Feyer over 1 year ago

I think that the TypoScript context must be "reseted" in a way ... I will check that next week during the code sprint.

#4 Updated by Aske Ertmann 11 months ago

  • Status changed from Accepted to Closed

Also available in: Atom PDF