Throw an error on unrecognized pl-* elements

[mwest1066]

In traverseQuestionAndExecuteFunctions() inside freeform.js, if visitNode() finds an element with tagName starting with pl- that we don't recognize, then it should throw an error rather than silently proceeding.

[eliotwrobson]

I think that implementing this is actually somewhat problematic. The traversal function traverses the inner pl tags as well as the outer ones, but the platform doesn't have information about which inner tags should be recognized. It could retrieve these from a hardcoded list somewhere, but this seems like overcomplicating things for no huge benefit.

[echuber2]

I agree this may be a problem if course staff had intentionally been using pl- for course elements (or something similar). Although the server could conceivably include course elements in the list of "known" tag names, perhaps course staff are doing something unusual. Would it make sense to make this optional with a "strict" setting on the course? Or have a way to explicitly add tag names to ignore to the course?

[echuber2]

On second thought, I haven't looked at the renderer in detail recently. If the expectation is that every pl- tag should be recognized and converted at that stage (so the other action would be to discard it silently), then it seems better to error than to discard it. If the current action is to leave it alone (to be converted in another pass), then erroring out seems overzealous.

[nwalters512]

IIRC the traversal algorithm always processes parents before children, so even if you have a pl-answer inside a pl-checkbox, the renderer itself should never see that child element as it will have been turned into something else when the parent (pl-checkbox) renders. This should mean that we don't have to know about any "inner" elements like that.

[echuber2]

Maybe I'm not reading your comment correctly, but did you mean it processes children before parents? (Actually, I should just study the implementation...)

[nwalters512]

Nope, I meant parents first. Studying the implementation is definitely recommended!

[eliotwrobson]

@echuber2 This is actually a fairly easy change to play around with, since this is really just adding an if statement. All that I remember from doing this is that I was getting errors about unrecognized child elements that started with pl-.

[eliotwrobson]

@nwalters512 is this the type of warning that would be better suited to being behind a feature flag? Looking at this again, I'm wondering if bundling this with the Python renderer is the way to go (as I did in #7983), since the new renderer is still opt-in only. Having a feature flag for just this warning feels like a very small change to put behind something like that, and enabling it for all courses at once is probably going to throw a ton of errors.

[nwalters512]

I agree that if we want to do this, we should do it with the standard rendering pipeline.

I'm not really sure how we'd go about rolling out this change if it was behind a feature flag. How do we decide which courses to enable it for? And at any rate, we'll eventually have to turn it on for everyone.