Problem/Motivation

When comparing revisions with the Visual Inline (HTML5) layout, any list (<ul>/<ol>) that contains inline SVG produces PHP warnings from caxy/php-htmldiff.

Steps to reproduce

- Install Drupal 11 and Diff Plus
- Enable "Visual Inline (HTML)" in admin/config/content/diff/general
- Change Full HTML to have no text editor and remove filters (quickest way to give markup that will cause the error)
- Create content with something like <ul><li><svg width="16" height="16"><use href="#icon"></use></svg> Opening times</li></ul> in it.
- Make an edit
- Compare the two versions using "Visual Inline (HTML5)"
- See the errors

Screenshot:

"Tag svg invalid in Entity..."

Proposed resolution

I'm not 100% sure, the underlying cause can be fixed here so really there's different strategies for suppressing warnings like these... open to suggestions.

Remaining tasks

- Decide if suppressing the warnings is okay
- Decide a strategy for suppressing the warnings (e.g. using libxml_use_internal_errors(TRUE) at some point during the page diff rendering)
- Raise MR
- Review
- Revise
- Approve

User interface changes

- Warnings gone

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
DiffSvgError.png113.51 KBjacobupal

Comments

jacobupal created an issue.

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented

Hey, I'll do some research on this. I had noticed this, but was hoping that the internal HTML5 DOM parser update with php 8 would have resolved this.

Can you try on php 8.4+?

jacobupal’s picture

jacobupal commented

Thanks!

Funnily enough, though it was in a php 8.3 project where I first saw it - when I reproduced it on simplytest.me it used php 8.5 (simplytest.me's default) so I can confirm it does still occur in 8.4+

I don't know if we're using the latest version of vcaxy/php-htmldiff but I didn't see any fixes in their issue queue for this sadly.

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented

Hey Jacob,

I think I've tracked this one down. The upstream caxy/php-htmldiff needs to update to use https://www.php.net/manual/en/class.dom-htmldocument.php instead of https://www.php.net/manual/en/domdocument.loadhtml.php!

I don't have time this week to open an upstream pull request, but I think that it should be fairly straight-forward if the maintainer is willing to cut a new major release that only supports php 8.4+.

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented

This is very, very, very rough, but I have https://github.com/lal65/php-htmldiff/tree/html5-parsing started as an upstream conversion to the HTML5 API.

The unit tests are passing, but there are still some very taboo things happening that I need to dig into deeper.