Private RichTextEditorIntermediateValue makes it impossible to extend PVC for RTE

[enkelmedia]

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

13.0.3

Bug summary

Hi!

We have created a custom PVC to be able to make some adjustments to the HTML in the rich text editor output before the "core processing" takes place. For example, we use the data-udi attributes on img elements to insert HTML picture elements and not just "simple" img elements. We used to do this by inheriting from RteMacroRenderingValueConverter, override ConvertIntermediateToObject and then have our override call the base RteMacroRenderingValueConverter.ConvertIntermediateToObject. This way we would get access to the data-udi attributes before the core processing removes them from the output markup.

With the addition of the "block in rich text editor"-feature ConvertIntermediateToObject expects inter to be of the private type RichTextEditorIntermediateValue which kind of breaks the workflow that we used have since we cant pass down valid data to the base class (RteMacroRenderingValueConverter).

I would like to be able to reuse all the existing processing in the core but just make some minor adjustments before it kicks in.

Would you consider making RichTextEditorIntermediateValue public or could this be facilitated in any other way?

Specifics

I basically would like to be able to do something like this:

public override object ConvertIntermediateToObject(IPublishedElement owner,
    IPublishedPropertyType propertyType,
    PropertyCacheLevel referenceCacheLevel,
    object? inter,
    bool preview)
{
    if (inter is RichTextEditorIntermediateValue rteInter)
    {
        rteInter.Markup = MakeMyChanges(rteInter.Markup);
    }

    var baseResult =_baseConverter.ConvertIntermediateToObject(owner, propertyType, referenceCacheLevel, inter, preview) as HtmlEncodedString;

    return baseResult;
}

Steps to reproduce

Se above.

Expected result / actual result

No response

[github-actions[bot]]

Hi there @enkelmedia!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[Zeegaan]

Heyo, as this is more of a feature request, I will move it to the discussions board 😁

[enkelmedia]

@Zeegaan not really a feature either - maybe something in between. There is a public API that can’t be used due to the hidden type used in the implementation. This used to be possible before the type was introduced.

I think that this one is simple, either we make the type public or use a Tuple or something so that someone can call the method when inheriting from the class.

Which would be prefer? I can make a PR if I just know which approach is preferred :)

[bergmania]

I wonder if it would work with a public interface on RichTextEditorIntermediateValue instead? We dont like to have public nested classes.

[enkelmedia]

@bergmania It does not really matter to me, the problem is the type-check that you're doing using the internal type if (inter is RichTextEditorIntermediateValue rteInter), this makes it impossible to provide a value from any sub-class. If the RichTextEditorIntermediateValue would implement a public interface and the type check would be on that interface it would be possible for me to create my own class that implements this interface so that I can provide a value for the PVC.

I could make a PR if you want an example - just need to know where to put that interface? =D

[bergmania]

Sounds like the interface is non breaking and allow us to keep the nested model private. PR Sounds good 🙏

[mikecp]

Closing the discussion as per merge of PR #15645

[thomashdk]

@enkelmedia have you found a workaround until it's released :)

have to rewrite my converter after upgrading from 12 to 13.

trying to chagne the markup saved, but can't get it to convert it..

public object ConvertIntermediateToObject(IPublishedElement owner,
            IPublishedPropertyType propertyType,
            PropertyCacheLevel referenceCacheLevel,
            object? inter,
            bool preview)
{
    if (inter is RichTextEditorIntermediateValue rteInter)
    {
        rteInter.Markup = "<p>Test</p>";
        var attemptConvertText = rteInter.Markup.TryConvertTo<IHtmlEncodedString>();
        if (attemptConvertText.Success)
            return attemptConvertText.Result;
    }

    return null;
}

This is false if (inter is RichTextEditorIntermediateValue rteInter)

[thomashdk]

I'm going this in my IPropertyValueConverter for the Umbraco.TinyMce datatype

[enkelmedia]

@thomashdk The code that I posted was an example of how I would like to be able to do. It not possible until the PR is included which looks like 13.2.0.

PR is here: #15645

To temporarly hack my way around this I used reflection, including some inspiration below - but be sure to update once the 13.2-release is out.

public class MyRteOverridePVC : RteMacroRenderingValueConverter
{

    /// <summary>
    /// Cache-property to avoid having to re-create the PropertyInfo for the Markup-property of
    /// RichTextEditorIntermediateValue
    /// </summary>
    private static PropertyInfo? _MarkupProperty { get; set; }


    // ... ctor and stuff....


   public override object ConvertIntermediateToObject(IPublishedElement owner,
        IPublishedPropertyType propertyType,
        PropertyCacheLevel referenceCacheLevel,
        object? inter,
        bool preview)
    {
        if (inter == null)
            return new HtmlEncodedString(string.Empty);
        
        if (_MarkupProperty == null)
        {
            /*
             * Extracting the "Markup"-property of the internal RichTextEditorIntermediateValue
             * so that we can adjust the HTML markup before calling the base class.
             * Created issue to ask to make this type public to avoid the reflection: https://github.com/umbraco/Umbraco-CMS/issues/15549
             */

            var richTextInterType = inter.GetType();
            var properties = richTextInterType.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
            var property = properties.FirstOrDefault(x => x.Name == "Markup");
            _MarkupProperty = property;
        }
        
        var markup = _MarkupProperty!.GetValue(inter) as string ?? "";

        string markupWithPictures = ProcessRteMarkup(markup, preview);
        
        _MarkupProperty!.SetValue(inter, markupWithPictures);

        
        var baseResult = _baseConverter.ConvertIntermediateToObject(owner, propertyType, referenceCacheLevel, inter, preview) as HtmlEncodedString;

        return baseResult ?? new HtmlEncodedString(string.Empty);
    }
}

In 13.2 and above you should be able to do something like:

if(inter is IRichTextEditorIntermediateValue rte)
{
  rte.Markup = " New stuff";
}

[thomashdk]

Thanks ! 👍

[thomashdk]

Last question ! :D How do you inject _baseConverter ? @enkelmedia

[enkelmedia]

Ohh... looking at my code I'm not sure why but in this case I'm injecting the RteMacroRenderingValueConverter in the constructor of my property value converter. I don't think that this is necessary, you should be able to do:

var baseResult = base.ConvertIntermediateToObject(owner, propertyType, referenceCacheLevel, inter, preview) as HtmlEncodedString;

[croban]

My hack
inter.Adapt<Dictionary<string, string>>();
where Adapt is a method of Mapster which is already used elsewhere