This is a follow up to #3239287: Fix \Drupal\Core\Extension\ModuleDependencyMessageTrait to not cause deprecations in PHP 8.1 which fixed two instances of

Exception: Deprecated function: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated
Drupal\Component\Utility\Html::escape()() (Line: 424)

However, one more needs to be corrected. The problem occurs when a version is specified in the dependency requirement, but the module being required has no version information. The failure is because null is passed as the version in the 'incompatible with version' return message.

The newly added test module's dependency just has the module name 'system_no_module_version_dependency_test' but no version. Which means that !$dependency_object->isCompatible($version) is not true in the test case, and the message is never attempted to be displayed. If a version is specified in the requirement then that condition will be true and the message is displayed. Then PHP8.1 complains that the string is null. I discovered this on a site where the required module was a git repo and so has no explicit version in .info.yml

CommentFileSizeAuthor
#8 php8_1_deprecation_in-3310555-8.patch3.14 KBbrentg

Issue fork drupal-3310555

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jonathan1055 created an issue. See original summary.

jonathan1055’s picture

jonathan1055 commented
Status: Active » Needs review

Added test coverage first, without the fix. This should fail.

jonathan1055 opened merge request !2771

jonathan1055’s picture

jonathan1055 commented

As expected, we get "Exception: Deprecated function: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated"

Now with the fix ...

jonathan1055’s picture

jonathan1055 commented

Now restore the full set of tests.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +PHP 8.1

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

brentg’s picture

brentg
Primary language Dutch
Location Ghent
commented
StatusFileSize
new php8_1_deprecation_in-3310555-8.patch3.14 KB

Rerolled the MR from Jonathan1055 against 9.4 branch so it can be more easily applied during updates.

jonathan1055’s picture

jonathan1055 commented

Hi breng,
Thanks for that. Just wondering, have you tried using the automatic 'plain diff' file linked in the MR box?
https://git.drupalcode.org/project/drupal/-/merge_requests/2771.diff

The benefit of using this is that when the MR is rebased any script or update process will automatically get the required change and not have to use a new patch.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: PHP8.1 deprecation in ModuleDependencyMessageTrait - null value for module->info['version'] » htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge require as a guide.

Reading the comments and threads appears there is 1 open thread that needs resolved. Moving to NW for that.

jonathan1055’s picture

jonathan1055 commented
Status: Needs work » Needs review

I've commented and resolved the thread. Now back to 'NR'

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Reviewing the MR confirmed the threads are resolved.

Looking at the changes they look good

jonathan1055’s picture

jonathan1055 commented

Thanks for the review. The merge is blocked probably because the branch was created at 9.5 but the issue version is now 10.1
I can work on that tomorrow, unless somone else wants to. We are allowed to force-push MR branches so can resolve the conflicts locally.

  • catch committed 9696ac2b on 10.0.x 
    Issue #3310555 by jonathan1055, smustgrave: htmlspecialchars(): Passing...
catch’s picture

catch
he/him
Primary language English
commented
Version: 10.1.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x, thanks! The MR diff applied fine even though the branch was out of date.

  • catch committed d98f5613 on 10.1.x 
    Issue #3310555 by jonathan1055, smustgrave: htmlspecialchars(): Passing...

  • catch committed 9255ef85 on 9.5.x 
    Issue #3310555 by jonathan1055, smustgrave: htmlspecialchars(): Passing...
chi’s picture

chi commented

I think this issue duplicates #3276589: Passing null to parameter #1 ($string) of type string is deprecated in Drupal\Component\Utility\Html::escape().

jonathan1055’s picture

jonathan1055 commented

Thank you catch.

jonathan1055’s picture

jonathan1055 commented

@chi from #19, No this does not duplicate that issue. Yes the subject matter is the same, but the changes on #3276589: Passing null to parameter #1 ($string) of type string is deprecated in Drupal\Component\Utility\Html::escape() do not touch core/lib/Drupal/Core/Extension/ModuleDependencyMessageTrait.php

jonathan1055’s picture

jonathan1055 commented
Title: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated » ModuleDependencyMessageTrait - htmlspecialchars(): Passing null to parameter #1 is deprecated

Altered the title for clarity

jonathan1055’s picture

jonathan1055 commented

Ah, apologies @chi I looked at the latest patches on that issue and they were nothing to do ModuleDependencyMessageTrait. However, that issue is long, been around since April 2022 and there are some mentions of the "module incompatible with version" message. The focus seems to have wandered a bit, and would take some investigation to get up to speed, but as I'm not following that issue, I won't make any comment over there.

chi’s picture

chi commented

@jonathan1055 yes, the issue was messed up at some moment. Because its title "Passing null to parameter #1 ($string) of type string is deprecated" is too generic. People started to submit patches to address their specific use cases even though the issue summary refers an error on /admin/modules page.

See patch #25. https://www.drupal.org/project/drupal/issues/3276589#comment-14706855

xjm closed merge request !2771

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone credited DanielVeza.

quietone’s picture

quietone commented

I have closed #3258379: Deprecation message on Appearance page as a duplicate and and moving credit here.

Thanks to @Chi for commenting on the other issue.