Views strips out video tags in custom field formatter

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

Did not test on D10
But this will need a test case to show the issue.

jitumiet → closed merge request !4594

Applying patch for Views Global Text area field to allow extra HTML tags. As video, source and iframe tag is not rendering. Due to which Media embedded video and remote-video not rendering in Views Global Text area field.

The last submitted patch, 22: 2942327-views-global-text-allow-extra-html-tags.patch, failed testing. View results →

Was previously tagged for tests which will see need to happen. Thanks!

codebymikey → changed the visibility of the branch 2942327-views-strips-out to hidden.

Made the change so that it only affects the views module rather than globally allow iframes in the admin.

Some further thought is probably necessary for whether this exposes any new potential security vectors.

e.g. if the site builder is using a user supplied token like a query string as a filter. But I'd assume as long as the value isn't rendered as {{ token|raw }}, it should be properly escaped or better yet, they should be using an appropriate filter in the first place like {{ token|escape('uri') }}.

Reuploaded an updated static patch of the latest MR.

Issue summary appears to be missing standard templates so tagging for such

have not reviewed

Wrong status sorry

Updated the issue summary

Hi,
Able to reproduce the issue in the Drupal11.x. After creating the view with specific html element data is not coming for the iframe tag.

MR !4595 has been verified in the Drupal 11.x.

Before Patch:

After Patch:

Thanks @codebymikey summary looks good!

Left some small comments on the MR.

Resolved PHPSTAN recommendations in the MR. Tests are passing. Looks like a change record can close this.

The more I think back on this, the more I think the current approach wouldn't actually solve the long-term issue (we'll forever be needing to add new elements as necessary to fit the site's use-case).

I think we can keep the API changes to \Drupal\Component\Utility\Xss::filterAdmin() as it's helpful for modules.

But we probably also require a UI option to either:
1. Skip XSS filtering entirely on the specific field rewrite,
2. Allow the user to specify the additional HTML elements which are exempt from XSS for view fields (not sure if this should be a global setting, or on an individual field-level basis).

More thoughts on this are more than welcome.

@codebymikey I think your plan at #36 should be a new child issue. It seems a feature request whereas this issue is a bug. The current MR fixes the bug.

@oily, I get your point, I think it just depends whether the XSS filtering is necessary in the first place given how the rendering supposedly works and views being configured by the site builders with admin permissions rather than arbitrary users.

If XSS filtering is not necessary, then removing it solves the actual issue rather than fixing it for this specific video/iframe situation - since the real issue is that the same supposedly "dangerous" HTML elements are rendered on the page when rewrites aren't turned on.

However if the core team determines that filtering is still necessary, then we can make a follow-up issue with direction for how they propose we go about opting out of it. But I think it's worth raising those questions here before this lands.

@codebymikey That sounds good but a CR is required all the same then can change to 'Needs review' and get it reviewed. The CR should reflect what is in the MR. Could update the description to state what has been actually done in the MR as distinguished from several possible approaches.

Yeah, that's completely fine, feel free to update the issue summary/CR and issue tags as necessary.

oily → changed the visibility of the branch 2942327-views-global-text-allow-extra-html-tags to hidden.

oily → changed the visibility of the branch 2942327-views-global-text-allow-extra-html-tags to active.

The pipeline is all green and test coverage works: a failing test is confirmed.

Rebased, pipeline successful.

Changing to 'Needs review'.

There is a merge conflict. May need a new branch based on 11.x.