FIX Don't strip <header> tag from HTMLValue
#11302
Conversation
src/View/Parsers/HTMLValue.php
Outdated
| $content = preg_replace('#</?(html|head|body)[^>]*>#si', '', $content); | ||
| $content = preg_replace('#</?(html|head|body)(\s[^>]*)?>#si', '', $content); |
There was a problem hiding this comment.
Went for the more broad approach here rather than the suggestion in the issue - HTML6 could introduce new tags like <heading>, or someone could be using a front-end JS library that consumes custom tags that this would otherwise strip.
There was a problem hiding this comment.
UPDATE: I've reverted back to using the suggestion from the issue because it doesn't have the same CI failure my attempt did. Specifically there was an edge case (which we already have a unit test for, which is what failed) where some really weird invalid HTML could result in multiple <body> tags existing in the HTML.
The HTMLValue implementation relies on exactly one <body> tag being present, so that's a non-starter. People who want custom weird tags will have to name them something else, and we'll just have to be aware of changes when future versions of HTML roll around.
|
Prefer-lowest failures are expected - that's happening all over the place. I suspect that's what silverstripe/gha-ci#132 is intended to fix. |
GuySartorelli
force-pushed
the
pulls/5.2/no-strip-header
branch
from
30e7959 to
e68b65c
Compare
GuySartorelli
force-pushed
the
pulls/5.2/no-strip-header
branch
from
e68b65c to
f46c0d8
Compare
|
See #11302 (comment) |
| @@ -32,7 +32,7 @@ public function __construct($fragment = null) | |||
| */ | |||
| public function setContent($content) | |||
| { | |||
| $content = preg_replace('#</?(html|head|body)[^>]*>#si', '', $content); | |||
| $content = preg_replace('#</?(html|head(?!er)|body)[^>]*>#si', '', $content); | |||
There was a problem hiding this comment.
Just a comment here — I know it's cooler to use more regex foo, but imho here it would be much easier for anyone reading the list of tags if it was explicitly listed as html|head|body|header.
Description
Stops
HTMLValuefrom stripping the<header>tag.Manual testing steps
See linked issue
Issues