U4-9088 - Macro Container Property Type returns null

Created by Martin Griffiths 21 Oct 2016, 08:49:45 Updated by Sebastiaan Janssen 28 Jun 2018, 18:30:27

Tags: PR

Added Macro Container property to document type and picked a macro partial view file with the picker in a newly created page.

Code example added to partial view of type UmbracoViewPage

Old way (using IHtmlString as type)

@Model.GetPropertyValue("macro")

Models Builder way (default type is IHtmlString)

@Model.Macro

Both ways return null.

If I change the type to string or object or dynamic I get the HTML. This issue is mostly affects Models Builder as it picks IHtmlString as the default type which I can change but would get overwritten on a rebuild.

Comments

Lee Kelleher 05 Apr 2018, 16:35:19

The issue here is an object-type mismatch. The MacroContainerValueConverter.ConvertDataToSource method is returning a string...

https://github.com/umbraco/Umbraco-CMS/blob/release-7.10.1/src/Umbraco.Web/PropertyEditors/ValueConverters/MacroContainerValueConverter.cs#L59-L68

...so when that value is trying to be cast as an IHtmlString, it is failing and returning null.

Potential fixes are:

  1. Return the value from MacroContainerValueConverter.ConvertDataToSource as new HtmlString(sourceString) ... ''note, this is considered a breaking-change!''

  2. Include a TypeConverter for string to IHtmlString (and HtmlString) object-types. ''Again, I'm not sure if this would be considered a breaking-change or not?''


Lee Kelleher 05 Apr 2018, 16:49:24

@martinjgriffiths If you want an interim fix, you could try this? https://gist.github.com/leekelleher/fef3dd7d6a934b8333e270bd5a6aa7ef

It will remove the default value-converter for the MacroContainer and replace it with a custom one that returns a HtmlString.


Lee Kelleher 05 Apr 2018, 17:01:59

I've taken a punt and submitted a PR for option 1: https://github.com/umbraco/Umbraco-CMS/pull/2565

With a cursory test, it's possible to cast an IHtmlString to a string, just not the other way around.

So with this PR in place, users could call either...

  • @Model.GetPropertyValue<string>("macro")
  • @Model.GetPropertyValue<HtmlString>("macro")
  • @Model.GetPropertyValue<IHtmlString>("macro")


Martin Griffiths 06 Apr 2018, 08:06:57

@leekelleher wow two patches in quick succession! Amazing, thankyou!


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted: Pull request

Affected versions: 7.5.3

Due in version: 7.12.0

Sprint:

Story Points:

Cycle: