Fieldset Labels Don't Sanitize Rich Text Immediately

[Nicholas-Westby]

Just upgraded to 1.6.0 and noticed this:

Notice the rich text isn't sanitized (i.e., the markup is still there). If I expand the fieldset, the digest cycle runs again and causes it to be sanitized. Here's what that looks like (I happened to collapse the fieldset again in this screenshot):

IIRC, the label template is something like this:

[Location Grid] {{header}}

The "Header" field is rich text.

Probably related to that recent change I made for promises.

[kgiszewski]

Yep, confirmed. We should probably fix that.

[kgiszewski]

I just tried again but this time with the {{header}} only in the template. So it seems this only happens if you have something before the template with rich text. i.e. {{headline}} {{text}} where text is the RTE.

[kgiszewski]

foo {{text}}, {{text}} foo and {{text}} {{headline}} also fail. So it may have to do with multiples in one template.

[Nicholas-Westby]

I'll try to look into this tonight, but I've got a busy week, so might have to wait until next week. Hopefully it's a quick fix though (may be since it's pretty easy to reproduce).

[Nicholas-Westby]

Note to self, look into coreTinyMce function: https://github.com/kgiszewski/Archetype/blob/master/app/services/archetypeLabelService.js#L392

[Nicholas-Westby]

I suspect the issue may be this line: https://github.com/kgiszewski/Archetype/blob/master/app/services/archetypeLabelService.js#L494

On the first run, it would return null for the data type info, and so the subsequent if statement would never be entered into, which means getNativeLabel won't be called.

I have a couple fixes in mind. One would be to add an optional parameter to getDatatypeByGuid that would have it return a promise, in which case I can wait for it to resolve to a value rather than just accepting null as a temporary response until it gets cached.

Hopefully I can find time to submit a pull request tonight.

[kgiszewski]

👍 no rush

[Nicholas-Westby]

Currently working off of this branch: https://github.com/Nicholas-Westby/Archetype/tree/fix/428-label-sanitization

I think I've fixed it (I've pushed to that branch too), but I haven't done much testing yet, so I'll hold off on the pull request for now.

[kgiszewski]

I've fixed this one and it was data type caching related, however #429 still needs it's own fix. I will look at that in the morning.

[kgiszewski]

https://github.com/kgiszewski/Archetype/pull/431/files?w=1

[kgiszewski]

I think the fundamental problem we're seeing is cache related. Entity\datatype lookups are missing on the first try. The cache gets populated quietly in the background, then the second try is a cache hit. I've preloaded all of the datatypes but I can't do that for entities.

The original label templates were designed to run rapidly and hence the cache was needed to prevent http spam. I'm trying to wrap my head around the current state more. I'm surprised I didn't catch this in testing. We'll figure something out. I might need to rewrite the 'built-ins' to be promises.

[Nicholas-Westby]

If we can rewrite the built-ins as promises, the solution is pretty easy. Just didn't want to do that, as it may break backward compatibility.

[kgiszewski]

As I count the number of hours I've put in on this today, I'm leaning towards that :)

[kgiszewski]

I think i got it all working, would love a peer review when you get a chance.

https://github.com/kgiszewski/Archetype/pull/431/files?w=1

I tried 1001 things and finally landed on one that seems to work without any breaking changes.

tl;dr => caching woes

[kgiszewski]

Oh and I tested @kipusoep 's template, the native template for RTE and I will test a promise tomorrow (it's late here).

[kgiszewski]

Closing as a result of #431