Deprecate masterminds/html5 as a production dependency for Drupal 10

I think minimising Drupal 9's production dependencies at the start of the life cycle is important. It'll make dependency management throughout the cycle simpler because there are less important dependencies.

+++ b/composer.lock
@@ -4021,9 +3953,6 @@
-            "bin": [
-                "bin/composer"
-            ],

:D

This looks great.

I guess we need to consider contrib at this point - http://codcontrib.hank.vps-private.net/search?text=Masterminds&filename=

What's really really interesting is that cases where Masterminds\HTML5\Exception all appear to be errors!!! I guess this might be the first thing some IDEs suggest for the Exception class :(

I created the change record and added the release note.

With backporting this I'm a bit concerned about all those uses of Masterminds\HTML5\Exception

Won't they get class not found?

But also now presumably that exception is not being caught at all because it's the wrong one.

So if there were no usages, I would say definitely get this into 9.0.x, but since there are it's a bit weird. I think what will happen is instead of getting an uncaught exception (because code is trying to catch the wrong one), it would change to a fatal class not found. Discussed with @xjm and we both think it's too close to RC to change this now, also with a lot of modules theoretically ported already.

However I do think we should immediately deprecate this as a production dependency in 9.1.x, to be moved to dev dependency in 10.0.x, and also open an issue (probably for drupal-check?) to detect the specific case of importing the wrong exception class since that is a pre-existing bug in apparently a lot of modules.

I'll add this to the 10.0.x beta blockers list too.

@catch yeah I was super surprised by the amount of Masterminds\HTML5\Exception in contrib BUT all those instances are misguided and adding an unneeded dependency in their code.

This will break the inline_ad_entity (not D9 compat), entity_print (not D9 compat but active issue), external_data_source (not d9 compat), crossword (not d9 compat), and onomasticon (d9 compat), modules - we can open issues to add a composer.json

Regarding #11, I have updated the Crossword module to require mastermind/html5 itself. Thanks for calling this out, @alexpott.

Tagging for Drupal 10.

How do we deprecate a production dependency? Is there even a process for that?

I think the only thing we can do is add a change record and a release note, telling people to add an explicit dependency if they have one.

re-roll for 9.3

@andypost dev dependencies go in the root composer.json not core/composer.json

We could issue a deprecation from the class loader, but unsure how to determine (efficiently!) whether the dependency is explicitly installed elsewhere.

In an ideal world we wouldn't have to tell

people to add an explicit dependency if they have one.

Each Drupal extension should list its direct dependencies.

Can we just drop this dependency entirely? We only use it in two tests, and they don't seem to strictly need it anyway.

Failed asserting that 'Topic bad_help_topics.bad_html3 fails HTML validation: Tag nav invalid\n
' contains "Opening and ending tag mismatch".

So this does feel like an HTML5 issue, but how was this triggered? masterminds/html5 doesn't appear to do any kind of polyfilling, it is only used when you instantiate the classes directly?

Answering #24: symfony/dom-crawler optionally uses masterminds/html5 if it's available, and the exception is thrown from there, so I think it's OK to adjust this test for the different message, as the end result is the same.

Used to try apply related but it does not help to parse html5 markup

Wondering why <p>{% trans %}<a href="/foo">Text here{% endtrans %}</a></p> is not fail with twig renderer

There is another stream of work that is going in the other direction from this issue - #1333730: [Meta] PHP DOM (libxml2) misinterprets HTML5 and people are now working on issues adding more usage of masterminds/html5! See #2441373: Upgrade tests to HTML5 for example.

Meantime there's PHP 8.1 compatibility release 2 weeks ago https://github.com/Masterminds/html5-php/releases/tag/2.7.5

Is the only reason for this issue just that core isn't yet using masterminds/html5 (much) in production code? If so, then #27 will change that and I think that we should close or postpone this issue based on that.

Or, is there a problem with masterminds/html5? I'm not clear from reading #3040037: Update masterminds/html5 to 2.7.5 how big a problem that presented to the amp module. Are there other html5 parsing libraries that we want to consider instead of masterminds/html5? I don't think we can get away from needing some html5-compatible parser in core.

Filed patch #3040037-23: Update masterminds/html5 to 2.7.5 to upgrade it

Re #29 as far as I can see there are no other viable HTML5 parsers. libxml2 isn't likely to do it in the near future and masterminds/html5 is the only PHP library that I can find.

After reviewing the recently linked issues (one of which I commented in 9 years ago!) I now also think this should be closed and we should aim to complete #2441811: Upgrade filter system to HTML5 instead which will require this library at runtime.

+1 on it

The #2441811: Upgrade filter system to HTML5 is great way forward

No need for a change record if we're not doing this!