U4-5286 - Approved Color Picker value (black) returns zero (int)

Created by Lee Kelleher 28 Jul 2014, 16:03:46 Updated by Stephan 29 Jul 2014, 07:53:08

Using the Approved Color Picker (v7.1.4), one of my pre-defined colours is black (#000000).

When I try to access the value from my Razor view, (using .GetPropertyValue) the returned value is "0" (Int32).

The XML cache (umbraco.config) has the value as "000000".

I've followed through the Umbraco Core code and found that ConvertUsingDarkMagic is being called (because a ValueConverter isn't available); which sees "000000" as an integer, converting it to "0" (zero).

I've given this a quick test with some other numeric color/hex values... the only once that are affected are those with leading zeros (e.g. "000099" would return "99").

I can currently workaround this by implementing my own custom IPropertyValueConverter to handle the Color Picker value, but should we look to handle this within the Core?

Comments

Stephan 28 Jul 2014, 16:23:45

ConvertUsingDarkMagic did not get its name by accident. We should avoid it as much as we can provided that we do not break backward compatibility in a too nasty way. AFAIK the color picker should return a string containing the six RGB digits, correct?

If you can confirm I'll add a converter in 7.1.5.


Lee Kelleher 28 Jul 2014, 16:32:29

@zpqrtbnk Yes, the value should be the colour's hex value. ConvertUsingDarkMagic will only collapse any leading zeros (e.g. "000099" returns as "99", but "999999" returns as "999999" as you'd expect - although it is now an int, not a string).

In terms of backwards compatibility, tricky... as either developers haven't encountered this issue (or they have and not reported it), or they have implemented a fix/workaround - which then a converter could be considered a breaking change?


In terms of my own fix, I'm literally returning back the original source object:

public class ColorPickerValueConverter : PropertyValueConverterBase { public override bool IsConverter(PublishedPropertyType propertyType) { return propertyType.PropertyEditorAlias.InvariantEquals(Constants.PropertyEditors.ColorPickerAlias); }

public override object ConvertDataToSource(PublishedPropertyType propertyType, object source, bool preview)
{
	return source;
}

}


Stephan 29 Jul 2014, 07:14:58

Just a Q (asking for @sebastiaan's opinion): considering that this would not be strictly backward compatible, and that we want to respect SemVer as much as we can, would you add the converter to 7.1.5 or 7.2.0? Until now I'd have quietely fit it into 7.1.5... wondering whether it's time to become more serious?


Sebastiaan Janssen 29 Jul 2014, 07:44:24

7.1.5. Anybody using it now will need to do some pretty interesting stuff to make it work, don't think anybody is using GetPropertyValue else they'll run into this immediately.


Stephan 29 Jul 2014, 07:53:05

Pushed 53ea491bd6ee2c183722bcbdf351e2eaba7f375f to 7.1.5 - fixed.


Priority: Normal

Type: Bug

State: Fixed

Assignee: Stephan

Difficulty: Normal

Category:

Backwards Compatible: False

Fix Submitted:

Affected versions: 7.1.4

Due in version: 7.1.5

Sprint:

Story Points:

Cycle: