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

Removing cropping setting for picture results in bad picture parameters in URL #934

Closed
robinboening opened this issue Feb 8, 2016 · 8 comments
Assignees
Milestone

Comments

@robinboening
Copy link
Contributor

Given:
Element with a picture content and crop: true setting. Deployed to the server.
Now I decide to remove cropping for that picture content by setting crop: false and deploy again.

Expectation:
1.) The picture should be rendered
2.) The picture URL should not contain cropping information
3.) The picture should not be cropped anymore

Problem:
The picture is not rendered at all.
screenshot 2016-02-08 16 38 57

The picture URL contains the cropping parameters: /pictures/59/show/260x260/181x111/277x277/some_picture.png?sh=87f2dd0b355c3368

It says:

Bad picture parameters in /pictures/59/show/260x260/181x111/277x277/some_picture.png

@tvdeyen
Copy link
Member

tvdeyen commented Feb 8, 2016

This is related to caching. Publishing the page should fix your problem.

But we should address this. I.e. by adding the essence settings to the cache key.

@robinboening
Copy link
Contributor Author

I tried publishing the page. Does not solve the issue. Also in the preview frame the picture can not be rendered. Even swapping the picture and using another one from the library will not help rendering the picture.

@tvdeyen
Copy link
Member

tvdeyen commented Feb 9, 2016

Yeah. Because the content gets cached in the essence_picture_view. Please try to override the partial in your app, remove the cache(content) do block and see if this fixes your problem. We should then remove all cache blocks from essence views or at least include the settings and options in the cache key.

@tvdeyen
Copy link
Member

tvdeyen commented Feb 17, 2016

@robinboening did overriding the app/views/alchemy/essences/_essence_picture_view partial and removing the cache(content) block help?

@robinboening
Copy link
Contributor Author

I just tried overwriting the partial and removed the cache block. It fixed the issue partly, but not for all the picture essences. There are some pictures left that are not rendered.

My guess is: The issue might be related with the remaining cropping information in the database. I have this feeling because overwriting fixed the issue with those pictures I never cropped before, but for the remaining pictures I still have cropping information in the database which are still used in the url.

@robinboening
Copy link
Contributor Author

It was indeed the remaining cropping information in the database. We found a good solution for that. I will come up with a PR, hopefully today.

@tvdeyen
Copy link
Member

tvdeyen commented Nov 4, 2017

I just revisited this bug on latest master and can confirm that it's fixed in terms of the image getting rendered when no cropping information is in the database. But if cropping information is still in the database it uses that and crops the image. This is intentional and changing that would be a change to current behaviour. At least it displays the image.

I think the proposed change is right to do, as disabling cropping by passing crop: false should not crop the image at all. The next mayor version 4.0 would enable us to do so.

@tvdeyen tvdeyen added this to the 4.0 milestone Nov 4, 2017
@tvdeyen
Copy link
Member

tvdeyen commented Nov 4, 2017

Added a fix for this bug in #1321

@tvdeyen tvdeyen closed this as completed in 0bf8d0a Nov 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants