U4-10372 - Changing a datatype from MediaPicker2 to ImageCropper causes incompatible data in your property

Created by Claus Jensen 31 Aug 2017, 08:40:27 Updated by Sebastiaan Janssen 22 Feb 2018, 16:07:12

Relates to: U4-9295

Subtask of: U4-9609

We do not clear property data when switching the datatype of a property. This is fine for most scenarios (like changing a textbox to a RTE actually retains whatever value you had in your textbox to now be in the RTE).

This however doesn't work when going from a MediaPicker2 (storing a umb://media/whatever) to a ImageCropper expecting null or a json object containing ImageCropper data.

Since the ImageCropper doesn't handle this, it is now basically completely incapable of doing anything and you will get errors no matter what you do (trying to save empty and saving with new image doesn't fix the corrupt data).

The ImageCropper should be more tolerant and have checks to prevent simply failing to do anything if it's being served corrupt data.

Preferably it should handle cases where the data is a media udi and simply wrap it inside a real imagecropper object - since it isn't really a uncommon scenario to switch from the MediaPicker2 to a ImageCropper when you find out that you need this instead.

Comments

Bjarne Fyrstenborg 31 Aug 2017, 14:40:22

I had similar issue when changing datatype from Image Cropper to Media Picker: http://issues.umbraco.org/issueMobile/U4-9295


Robert Copilau 13 Sep 2017, 07:23:02

PR:https://github.com/umbraco/Umbraco-CMS/pull/2192 Thanks @Bjarne.Fyrstenborg, that fixed half of the problem :D.

Changes: *MediaPicker2 no longer becomes unusable when switching from a ImageCropper *ImageCropper now retains MediaPicker2 image after switching from it

Note - After the fix, when switching from MediaPicker2 to ImageCropper - the ImageCropper will now have the image path of the MediaPicker, which belongs to a media item. If you were to remove that image from the cropper and publish, it would also remove image - and now we have a media item with no image, that's not what we want.

The fix, in ImageCropperPropertyValueEditor.cs, MediaService has been used to check if the image path belongs to a media item, if not, is safe to delete.


Robert Copilau 27 Sep 2017, 14:08:32

Small changes: https://github.com/umbraco/Umbraco-CMS/pull/2192/commits/b62225fdbaf85cf657f746daae20474209578e15


Sebastiaan Janssen 02 Oct 2017, 08:41:58

Moving this to 7.7.3 so we can look at this properly one more time and decide what to do.


Priority: Normal

Type: Bug

State: Review

Assignee: Sebastiaan Janssen

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.6.5

Due in version:

Sprint:

Story Points: 1

Cycle: 4