Problem/Motivation

Notice: Trying to access array offset on value of type bool in system_modules() (line 889 of /modules/system/system.admin.inc).

Drupal 7.73
Nginx 1.19.4
PHP 7.4.11

CommentFileSizeAuthor
#5 3182785-5.patch2.77 KBmcdruid
#5 3182785-5_test_only.patch1.9 KBmcdruid
#2 drupal_php74_notice_for_systemadmininc-3182785.patch616 bytesmar4ehk0

Comments

elsteff1385 created an issue. See original summary.

mar4ehk0’s picture

mar4ehk0 commented
StatusFileSize
new drupal_php74_notice_for_systemadmininc-3182785.patch616 bytes

Hey! I had some issue. and I created for this issue patch.

vladimiraus’s picture

vladimiraus
Primary language English
Location Brisbane 🇦🇺
commented
Status: Active » Reviewed & tested by the community

Thanks for the commit. Applies and works!

john franklin’s picture

john franklin commented
Status: Reviewed & tested by the community » Needs work

While it does work, it will also mask real issues. For example, if your have a module with an admin page at admin/config/foo-module, and the foo.info includes configure = config/foo-module, you'll get the notice when you load up the Modules page.

A better patch would check $configure_link is valid and generate a watchdog log for the offending $module->info['configure'] path.

mcdruid’s picture

mcdruid commented
Status: Needs work » Needs review
StatusFileSize
new 3182785-5_test_only.patch1.9 KB
new 3182785-5.patch2.77 KB

Patch taking the approach outlined in #4 plus a test, and a test-only patch to verify the fix.

One slight annoyance is that the test module cannot be hidden or the admin UI will not try to render the configure link.

We've already added a pair of non-hidden test modules in #3255068: [D7 PHP 8.1] system_modules(): Deprecated function: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated - they couldn't be hidden for similar reasons IIRC.

We may want to consider making the Testing fieldset collapsed by default on the modules page or something like that? That'd be a follow-up issue.

The last submitted patch, 5: 3182785-5_test_only.patch, failed testing. View results

mcdruid’s picture

mcdruid commented

Based on #3281772: Hide the "Only For Testing" package by default on the modules admin page we might want to change the package to "Only For Testing" in the test module here.

fabianx’s picture

fabianx commented
Assigned: Unassigned » mcdruid
Status: Needs review » Reviewed & tested by the community

Let's indeed put that to "Only for Testing"

RTBC + 1, approved for Merge! Thanks all!

  • mcdruid committed c91d728 on 7.x 
    Issue #3182785 by mcdruid, mar4ehk0, elsteff1385, John Franklin: PHP 7.4...
mcdruid’s picture

mcdruid commented
Assigned: mcdruid » Unassigned
Status: Reviewed & tested by the community » Fixed

Changed the package to "Only For Testing" on commit.

Thank you everybody!

Status: Fixed » Closed (fixed)

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