3931.feature allow javascript to be inserted in head-section or at bottom of page.

[jladage]

Fixes plone/Products.CMFPlone#3931

[mister-roboto]

@jladage thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

[mauritsvanrees]

The GitHub Actions tests will keep failing because they need a checkout of the branch from plone.base. We could make that happen, but then we would need to change it afterwards after the branch is merged and later undo it altogether when there is a new plone.base release on dist.plone.org.

I propose that we accept the failing GitHub Actions, and wait for the Jenkins jobs to pass. I have restarted the jobs of the three combines PRs, to test it with the most recent changes. They were already green earlier today.

After merge, I can make a plone.base release and update dist.plone.org and then the gh actions should be fine again.

Alternatively we first merge plone.base and plone.app.upgrade, and only merge the current PR once all is green again.

[thet]

See my remarks. There is some room for discussion - I would remove the filtering of UNWANTED_TAGS. (this needs some review of a security expert like @mauritsvanrees )

In any case at least the changelog entry needs to be updated to reflect what the implementation really does.

[thet]

I'm not sure, if this is really necessary. We're adding JavaScript and with that we can do anything, also unwanted stuff.
Maybe it should be up to the integrator to produce valid HTML?

If we keep it, I do like that e.g. <style> tags and the like are allowed. This gives an easy way of quickly adding a style messing with theming.

[jladage]

The rational is that this is not security but more thinking about what a not so savy site manager, could fuck up :) Off course he/she should handle injecting javascript with care, like it was alway the case with that field in the control panel.

My code, because it injects in the head section (thanks to @yurj ) I got aware of the fact that we could prevent "user error" simply by filtering only script tag. A bit later I realized we can prevent double base or title tags at a low cost using lxml.

A pleasant second benefit is that with this code, even if the user makes a typo in the script, the viewlet will render valid HTML.
I'm not familar enough with the control panel code to write up a validator for this head field which would prevent adding base or title tags in the nicest possible way. Anyone would like to assist or give me some pointers?

Hope this helps to understand why it's there.

[mauritsvanrees]

I have removed the lxml cleaning. Nice idea, but it would be better in a validator so it is done once. We have not done that until now, so there is no hurry, but I do want this merged for a Plone release this week."

I copied the original branch to a new name, so we still have the code:
https://github.com/plone/plone.app.layout/tree/3931.feature-with-lxml-cleanup

[mauritsvanrees]

As said before, GitHub Actions will fail until plone.base is released and available, we can ignore that.
I have just merged the other two PRs (plone.base and plone.app.upgrade).
So now we can do a final check on Jenkins with just this branch:

@jenkins-plone-org please run jobs

[mauritsvanrees]

I have released plone.base and updated its version on dist.plone.org/release/6.0-dev.
I reran the failing GitHub Actions and now they pass on master. So all is well.