If a module like Entity Embed, which uses post-render-cache placeholders like <drupal-post-render-cache> wants to work on a text format that uses the 'Limit allowed HTML tags' filter, we have a problem. It seems that Xss::split() doesn't match on tags with dashes in the name.

This is a blocker to using Web Components in D8 filtered text since they should/will all be using dashes in the name.

For reference custom elements with dashes in the name should be allowed:
http://stackoverflow.com/questions/18433287/html5-tag-name-conventions-c...
http://www.reddit.com/r/webdev/comments/1khhif/brick_by_mozilla_ui_compo...
http://w3c.github.io/webcomponents/spec/custom/

CommentFileSizeAuthor
#11 xss-split-custom-element-dash-tag-name-2315255-11.patch1.57 KBdevin carlson
#11 xss-split-custom-element-dash-tag-name-2315255-11-tests-only.patch1.1 KBdevin carlson
#4 2315255-xss-split-custom-element-dash-tag-name.patch2.5 KBdave reid
#1 2315255-xss-split-custom-element-dash-tag-name.patch1.89 KBdave reid
#1 2315255-xss-split-custom-element-dash-tag-name-TEST-ONLY.patch1.31 KBdave reid

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Needs review
StatusFileSize
new 2315255-xss-split-custom-element-dash-tag-name-TEST-ONLY.patch1.31 KB
new 2315255-xss-split-custom-element-dash-tag-name.patch1.89 KB

The last submitted patch, 1: 2315255-xss-split-custom-element-dash-tag-name-TEST-ONLY.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work 
+++ b/core/lib/Drupal/Component/Utility/Xss.php
index 1895588..d8960c6 100644
--- a/core/modules/filter/src/Tests/FilterUnitTest.php

--- a/core/modules/filter/src/Tests/FilterUnitTest.php
+++ b/core/modules/filter/src/Tests/FilterUnitTest.php

The XssTest is the proper place to add this test.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Needs review
StatusFileSize
new 2315255-xss-split-custom-element-dash-tag-name.patch2.5 KB

Revised patch against XssTest instead, thanks dawehner. I wasn't sure if this should also be expanded to include tags with colons in the name, but if I figured get this in first then file a follow-up.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Thank you for changing the test!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 1d8f75b and pushed to 8.x. Thanks!

Status: Fixed » Needs work

The last submitted patch, 4: 2315255-xss-split-custom-element-dash-tag-name.patch, failed testing.

  • alexpott committed 1d8f75b on 
    Issue #2315255 by Dave Reid: Fixed Xss::split() fails on custom HTML...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

devin carlson’s picture

devin carlson commented
Version: 8.0.x-dev » 7.x-dev
Status: Closed (fixed) » Needs review
Issue tags: +Needs backport to D7
StatusFileSize
new xss-split-custom-element-dash-tag-name-2315255-11-tests-only.patch1.1 KB
new xss-split-custom-element-dash-tag-name-2315255-11.patch1.57 KB

I ran into this issue while working on the D7 backport of Entity Embed.

Making the same change to _filter_xss_split() fixed the problem.

The last submitted patch, 11: xss-split-custom-element-dash-tag-name-2315255-11-tests-only.patch, failed testing.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented
Status: Needs review » Reviewed & tested by the community

I can confirm fixing the regex in _filter_xss_split() allows for entity_embed to work properly in 7.x

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: xss-split-custom-element-dash-tag-name-2315255-11.patch, failed testing.

devin carlson’s picture

devin carlson commented
Status: Needs work » Reviewed & tested by the community
adarkling’s picture

adarkling commented

Works in 7.x for me as well!
The patch needs to pass LocaleConfigurationTest->testLanguageConfiguration(), though.

David_Rothstein queued 11: xss-split-custom-element-dash-tag-name-2315255-11-tests-only.patch for re-testing.

David_Rothstein queued 11: xss-split-custom-element-dash-tag-name-2315255-11.patch for re-testing.

The last submitted patch, 11: xss-split-custom-element-dash-tag-name-2315255-11-tests-only.patch, failed testing.

David_Rothstein’s picture

David_Rothstein commented
Status: Reviewed & tested by the community » Fixed
Issue tags: +7.37 release notes

Committed to 7.x - thanks!

  • David_Rothstein committed 254424d on 7.x 
    Issue #2315255 by Dave Reid, Devin Carlson: Allow custom HTML tags with...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.