Skip to content
This repository has been archived by the owner on Dec 13, 2021. It is now read-only.

Property List of RTEs returns list of NULL #9

Closed
pgregorynz opened this issue Sep 25, 2018 · 4 comments
Closed

Property List of RTEs returns list of NULL #9

pgregorynz opened this issue Sep 25, 2018 · 4 comments
Assignees

Comments

@pgregorynz
Copy link

pgregorynz commented Sep 25, 2018

When using property list with RTEs, when the value is converted by the property value converter the result is all nulls. The returned type is correct, IEnumerable<IHtmlString> but all the items in that list are null.

@leekelleher leekelleher self-assigned this Sep 25, 2018
@leekelleher
Copy link
Collaborator

Hey @pgregorynz - thanks for raising this one.

I haven't had chance to dive into the code to debug this yet. (It'll most likely be late Thurs before I do).
Although I have my suspicions of what it might be... I'm leaving some notes here, mostly for myself.


TinyMceValueConverter sets the PropertyValueType as IHtmlString, then ConvertSourceToObject returns HtmlString ... which should be castable, but maybe it isn't? 🤔

Property List is using Umbraco's TryConvertTo<T> to perform the casting... so worth checking if (HtmlString).TryConvertTo<IHtmlString>() works as expected, or whether Property List needs to do extra work for interface-types?

@leekelleher
Copy link
Collaborator

leekelleher commented Sep 26, 2018

Curses this has been playing on my mind... and it's lunch time.

It turns out that it is Umbraco's TryConvertTo methods doing different things, e.g.

The generic version works as expected...

(new System.Web.HtmlString("foo")).TryConvertTo<System.Web.IHtmlString>().Success // is true

However the non-generic version fails...

(new System.Web.HtmlString("foo")).TryConvertTo(typeof(System.Web.IHtmlString)).Success // is false

... and Property List is using the non-generic version, (since we only know the object-type at runtime).

Digging into Umbraco's TryConvertTo<T> (generic version) code, this line will check if the non-generic version was successful, if not, it'd attempt a direct cast. Which in this case explains why the generic version works and the non-generic version doesn't.

OK, where does this leave us?

  1. Change Umbraco core so that TinyMceValueConverter return type is a HtmlString, and not an IHtmlString. (not ideal, too much work involved convincing people and waiting for release cycle and asking you to upgrade)
  2. Ask you to develop a custom TypeConverter that will cast an HtmlString to an IHtmlString, (again, not ideal, throws the problem over your side of the fence ... and other people will ask me about this in future, etc)
  3. Add code to Property List to follow a similar pattern as Umbraco's TryConvertTo<T>, check for failure and attempt a direct cast.

While I don't think this is Property List's concern, (I think it should be TryConvertTo responsibility), the last option will be frictionless. I'll see what I can do. 😃

@leekelleher
Copy link
Collaborator

@pgregorynz I've added a fix for this. If you want to try it out, the latest package files from the CI build are here: https://ci.appveyor.com/project/UMCO/umbraco-property-list/build/1.0.1.47/artifacts

Preparing a release takes a little bit more time, I'll schedule a v1.0.1 release soon.

@leekelleher
Copy link
Collaborator

@pgregorynz Just to let you know that v1.0.1 is out! https://github.com/umco/umbraco-property-list/releases/tag/1.0.1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

2 participants