Skip to content

[DomCrawler] [Tests] Adapt testAddHtmlContentWithErrors to be HTML5 compliant #61347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 6, 2025
Merged

[DomCrawler] [Tests] Adapt testAddHtmlContentWithErrors to be HTML5 compliant #61347

merged 1 commit into from
Aug 6, 2025
+3 −2

Conversation

renanrodrigo
Copy link
Contributor

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #61346
License MIT

  • What it does and why it's needed
    The new libxml2 uses HTML5 by default, and the current snippet used to test invalid HTML is HTML5-valid. This commit changes the snippet to be invalid both for HTML4 and HTML5, so the test passes.

  • A simple example of how it works (include PHP, YAML, etc.)

    • Running the test suite using a php build compiled against libxml2 < 2.14 should keep working
    • Running the test suite using a php build compiled against libxml2 >= 2.14 should now work

  • If it modifies existing behavior, include a before/after comparison
    It does not.

@renanrodrigo
[Tests] Adapt testAddHtmlContentWithErrors to be HTML5 compliant
3c0410b 
The new libxml2 uses HTML5 by default, and the current snippet used to test
invalid HTML is HTML5-valid. This commit changes the snippet to be invalid both
for HTML4 and HTML5, so the test passes.
@carsonbot carsonbot added this to the 6.4 milestone Aug 5, 2025
@symfony symfony deleted a comment from carsonbot Aug 5, 2025
@nicolas-grekas
Copy link
Member

Wow really!
Which versions of PHP support libxml 2.14?
This relates directly to #54383
Maybe we can withdraw this PR! WDYT ?

@nicolas-grekas
Copy link
Member

Well, not really:

https://gitlab.gnome.org/GNOME/libxml2/-/issues/211

@carsonbot carsonbot changed the title [Tests] Adapt testAddHtmlContentWithErrors to be HTML5 compliant [DomCrawler] [Tests] Adapt testAddHtmlContentWithErrors to be HTML5 compliant Aug 6, 2025
@derrabus derrabus removed the Bug label Aug 6, 2025
@nicolas-grekas
Copy link
Member

Thank you @renanrodrigo.

@nicolas-grekas nicolas-grekas merged commit e6cad90 into symfony:6.4 Aug 6, 2025
11 checks passed
@renanrodrigo renanrodrigo deleted the fix-invalid-html-test-6.4 branch August 7, 2025 01:39
@renanrodrigo
Copy link
Contributor Author

Thanks for approving+merging!

Sorry if I didn't provide enough context - but I see you found it now so all good (:
It's not much about the php version per se, but we're building it with the new libxml2 underneath, thus the error.

On one hand this is just one test, on the other hand more behavior may change in loadHTML as new builds happen out there with libxml2 2.14.... We have seen different behavior in a few more packages when using the new build... Maybe loooking at DOM\HTMLDocument instead of DOMDocument for php8.4+?

Curious to see how this will evolve

@xabbuh
Copy link
Member

xabbuh commented Aug 7, 2025

@renanrodrigo #54383 will hopefully help here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
DomCrawler Status: Reviewed
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.
5 participants
@renanrodrigo @nicolas-grekas @xabbuh @derrabus @carsonbot