To mark language changes in body text, editors can use <span lang="nl">Nederlands</span>.
But this get rendered in invalid HTML, where we will find the attribute xml:lang twice. This will fail in validators and fail wcag2.
<p><span dir="rtl" lang="he" xml:lang="he" xml:lang="he">תוכן עניינים</span></p>
- add basic page
- keep text format 'basic HTML'
- change wysiwyg-editor to 'source'
- add <span dir="ltr" lang="fr">vive la vie</span>
- click save and publish
- look in the source HTML (not DOM), you will find:
<span dir="ltr" lang="fr" xml:lang="fr" xml:lang="fr">vive la vie</span>
we would expect:
<span dir="ltr" lang="fr">vive la vie</span>
Basic HTML is using several filters. The problematic filter is 'Restrict images to this site', in combination with 'Correct faulty and chopped off HTML'. As other combinations with 'Correct faulty and chopped off HTML' work, 'Restrict images to this site' is suspicious.
This might be related to the fact that Drupal has no html5 parser (#1333730: [Meta] PHP DOM (libxml2) misinterprets HTML5). But even for html4, this result is invalid.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | restrict_images_to-2346989-12.patch | 639 bytes | BarisW |
| #9 | restrict_images_to-2346989-9.patch | 587 bytes | BarisW |
Comments
Comment #1
hanno commentedComment #2
hanno commentedInteresting:
1. it works correct if you create a new text format with both filters applied.
2. it also works correct if I add ckeditor to it
3. when I switch to basic html, it fails.
Comment #3
hanno commentedTracked down to 'Restrict images to this site' in combination with 'Correct faulty and chopped off HTML'. If you enable both filters on a new text format, it will produce invalid HTML.
Comment #4
hanno commentedComment #5
wim leersThis is a duplicate of #2105841: Xss filter() mangles image captions and title/alt/data attributes.
Comment #6
hanno commentedYou're sure this is a duplicate of #2105841: Xss filter() mangles image captions and title/alt/data attributes? That issue filters colons (caused by caption filter), but In this case we have twice 'xml:lang' in the output (caused by restrict images filter).
Comment #7
BarisW commentedAgreed with Hanno, I see no direct connection to that other issue. Let's re-open for consideration.
Comment #8
mgiffordThis is an accessibility bug we should fix.
Comment #9
BarisW commentedI have no idea if my solution makes any sense, but here is a patch to get the ball rolling.
It seems that only the
langresults in unwanted XML attributes. so I figured: let's simply remove it.Comment #11
BarisW commentedThis patch removes the xml:lang completely. This is a bit too much. Maybe we can change the regex to only remove duplicate xml attributes?
Like here: http://stackoverflow.com/a/10655934
Comment #12
BarisW commentedOk, so the issue seems to be that both the FilterHtmlImageSecure and the FilterHtmlCorrector filter the complete body text using Html::normalize($text).
This introduces the double xml:lang tags. Here is another approach. This might not be THE perfect fix, but it is at least way better.
Basically, there seems no need to filter the html when the content doesn't contain an image?
Comment #13
wim leersAFAICT the correct fix is #1333730: [Meta] PHP DOM (libxml2) misinterprets HTML5: using the
\Masterminds\HTML5library in those filters, instead ofDOMDocument. Then there's no need for these hacks.Comment #14
BarisW commentedThanks for the info Wim! Should I need to create another issue for the patch in #12 then?
It still seems strange that the whole bunch of text is filtered when it is not needed.
Comment #15
wim leersYes, that would be great. This is a plain and simple performance improvement :)
Comment #16
BarisW commentedDone: #2665684: FilterHtmlImageSecure serializes all content, even when it has no images
Comment #17
wim leersLet's postpone this on #1333730: [Meta] PHP DOM (libxml2) misinterprets HTML5. I'm pretty sure we'll be able to close this once #1333730: [Meta] PHP DOM (libxml2) misinterprets HTML5 is done, but let's keep this around just to be sure.
Comment #20
mgiffordComment #31
wim leersPer #17, #2441811: Upgrade filter system to HTML5 fixed this 🥳