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

GetCropUrl returns null with valid cropAlias param V8 #5737

Closed
cleversolutions opened this issue Jun 27, 2019 · 22 comments
Closed

GetCropUrl returns null with valid cropAlias param V8 #5737

cleversolutions opened this issue Jun 27, 2019 · 22 comments

Comments

@cleversolutions
Copy link
Contributor

cleversolutions commented Jun 27, 2019

Bug summary

ImageCropperTemplateExtensions.GetCropUrl returns null when provided a valid cropAlias parameter unless you save each image in your media library after adding a new crop.

Specifics

This occurs in 8.0.2 and v8/dev using Chrome (but should affect all browsers)

Steps to reproduce

  • V8 with SK installed
  • Update Image Media Type -> Update umbracoFile Image Cropper DataType settings to include a crop type (ie square)
  • Update Products.cshtml / template with <img src='@product.Photos.GetCropUrl("square")' />
  • View page on frontend & confirm image renders (they don't)
  • Goto Media Section & Choose a Product Image
  • Set manual crops for the image
  • View page on frontend & confirm image renders (they do)

Expected result

The images should be rendered. See reproduction for more details.

Actual result

The images are not. See reproduction for more details.

@nul800sebastiaan
Copy link
Member

Yes, unfortunately this will be true for any updates you will do for datatypes. We do not update existing data when a datatype updates simply because it is an expensive operation that could take a very long time and much processing power.

It's up to you to do some defensive querying for this or possibly provide a migration tool for when you know things will need to change. Just be aware that you'll need to be careful with that migration script in case you have a large number of nodes (see previous paragraph).

@nul800sebastiaan
Copy link
Member

Hmm, I just saw your related PR, and it's not what I thought you wanted to get solved. ;-)

I'll have a look at it soon, this might be an okay solution, not sure yet.

@cleversolutions
Copy link
Contributor Author

I did something terribly wrong to that branch. I'll submit a new PR shortly.

@kjac
Copy link
Contributor

kjac commented Jan 2, 2020

@cleversolutions I can't reproduce this issue on the latest codebase. GetCropUrl(string alias) returns URLs for newly added crops on the image cropper datatype, even if the image in question hasn't been saved with the newly added crops.

Could I ask you to try this out again on the latest codebase?

@cleversolutions
Copy link
Contributor Author

Thanks @kjac I’ll retest and confirm tomorrow.

@cleversolutions
Copy link
Contributor Author

Hi @kjac I reproduced it this morning from a fresh clone of v8/dev.
I created a new site with the starter kit.
Added a 100x100pixel crop to ImageCropper data type called Test.
cropper

Then on line 35 of Products.cshtml changed the line to use the crop

<div class="product-grid">
            @if (Model.FeaturedProducts != null)
            {
                foreach (Product product in Model.FeaturedProducts)
                {
                    <a href="@product.Url" class="product-grid__item" style="background-image: url('@product.Photos.GetCropUrl("Test")')">
                        <div class="product-grid__item__overlay">
                            <div class="product-grid__item__name">@product.ProductName</div>
                            <div class="product-grid__item__price">@Model.DefaultCurrency @product.Price.ToString("F")</div>
                        </div>
                    </a>
                }
            }
        </div>

No images render. I save the jacket image then it renders while the others don't.
result

@kjac
Copy link
Contributor

kjac commented Jan 7, 2020

Hi again @cleversolutions

This is mysterious! For the life of me I can't reproduce the issue 😞

I have tried both with an imagecropper property directly on the content item and using an imagecropper on a picked media item. To no avail.

Here's how I have tested the first scenario (I'm guessing this is much along the lines of what you're experiencing):

  1. I have some content based on a content type that has an imagecropper property named Media.
  2. I am rendering the Media property like this:
<ul>
    <li>Crop 1: @Model.Media.GetCropUrl("Crop 1")</li>
    <li>Crop 2: @Model.Media.GetCropUrl("Crop 2")</li>
    <li>Crop 3: @Model.Media.GetCropUrl("Crop 3")</li>
</ul>
  1. Initially the imagecropper datatype has no crop named "Crop 3", so the last <li> is empty. But when I add "Crop 3" to the imagecropper datatype without saving the content, the crop is immediately available to the rendering.

Here's a GIF of the whole thing:

image-cropper-add-crop

@cleversolutions
Copy link
Contributor Author

Perhaps the difference is your document type uses an image cropper directly where as starter kit uses a media picker. I’ll test this idea further tomorrow.

@kjac
Copy link
Contributor

kjac commented Jan 8, 2020

@cleversolutions I tried the same thing with a media picker... same result.

@IbrahimMNada
Copy link
Contributor

did any one fixed this issue ?

@nul800sebastiaan
Copy link
Member

@uppercuut We can't seem to reproduce the issue, so if you have more info on how to make it fail then we might have a chance to fix it.

@IbrahimMNada
Copy link
Contributor

IbrahimMNada commented Jan 11, 2020

@nul800sebastiaan

Im using Umrbaco8 Image Cropper
i retrieve the Data using: "Model.Value<IPublishedContent("websiteImagecropped")"is always Null

"Model.Value<dynamic>("websiteImagecropped")"returns the url as string
"Model.Value("websiteImagecropped")" returns the url as string
umbraco doc says that:

"The cropper returns a dynamic object, based on a json structure like this"

{
"focalPoint": {
"left": 0.23049645390070922,
"top": 0.27215189873417722
},
"src": "/media/SampleImages/1063/pic01.jpg",
"crops": [
{
"alias": "banner",
"width": 800,
"height": 90
},
{
"alias": "highrise",
"width": 80,
"height": 400
},
{
"alias": "thumb",
"width": 90,
"height": 90
}
]
}

@cleversolutions
Copy link
Contributor Author

We are launching a new Umbraco 8 site on Monday. Once it is live I’ll have a bit more time to spend testing this further. I’ll post a gif of reproduction maybe mid week.

@rbottema
Copy link
Contributor

@kjac Perhaps the difference is that you are going from 2 to 3 crops, where @cleversolutions is going from 0 to 1 crops?

@jesperbrasmussen
Copy link

jesperbrasmussen commented Jan 31, 2020

I had this issue as @cleversolutions describes on an Umbraco 8.1.5 installation, but after upgrading to 8.5.3 it looks like it has been solved

@cleversolutions
Copy link
Contributor Author

I’ll record a screen capture after I get back from vacation. I was able to still reproduce the problem on a fresh 8.6 rc install.

@cleversolutions
Copy link
Contributor Author

Here is a screen video of the problem using 8.6 RC-0. I fire up a new starter site, show the default products page working. I add a crop called "test" to the image cropper, update the products template to use the crop, show that the page doesn't render with the new cropper, then save the biker jacket image and reload the products page to show that the biker jacket image shows while the others do not. The expectation, if everything is working properly, is that all images should show without having to resave the images after adding the new crop alias to the image cropper.

2020-03-23-10-25-01

@cleversolutions
Copy link
Contributor Author

@rbottema good catch. I continued my test above adding a second crop, and it works as expected. It seems this problem only happens when going from 0 to 1 or more crops.

2020-03-23-10-44-50

@joepvtl
Copy link
Contributor

joepvtl commented Apr 28, 2020

Hi @cleversolutions,

Have you tested this on 8.6.1?

@ronaldbarendse
Copy link
Contributor

@joepvtl This issue is verified to exist in the latest 8.6.1 release, but only happens when adding the first crop to the Image Cropper configuration. PR #6950 fixes the problem, but that's waiting to get reviewed/merged.

@thaihoc2
Copy link

thaihoc2 commented Jul 25, 2020

I'm working with Umbraco 8.6.3 and got the same issue: GetCropUrl() returned the below value without Image Url
<img src="?anchor=center&amp;mode=crop&amp;width=200&amp;height=200">
I think that's a very big issue, is there any plan to release the fixes?

@nul800sebastiaan
Copy link
Member

Fixed in #6950

@thaihoc2 This PR is now merged but it's not for your issue, I think you have the problem as described in #8116 (the short version of this is: try using @Url.GetCropUrl("").

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

10 participants