In its implementation of hook_requirements(), geshifilter uses integers as requirement keys. Since the new status page in Drupal 8.3 uses switch statements to render some requirement keys differently and switch statement comparisons are loosely typed, this leads to the following error on the status page:

Warning: Invalid argument supplied for foreach() in Drupal\system\Element\StatusReportPage::preRenderGeneralInfo() (line 43 of core/modules/system/src/Element/StatusReportPage.php). array(19)

The hook_requirements() documentation says that requirement keys are arbitrary but must be unique. This is technically more like a core bug, since the current implementation fulfils that, as long as no other module uses integer keys for its requirements. However, the documentation also says, that it is suggested to use the module short name as a prefix. Also, it is easy to imagine a case, where the current implementation might return non-unique keys, if another module uses a similar implementation. So it is probably a good idea to adhere to the suggested best practice and use string keys prefixed by module name.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

FeyP created an issue. See original summary.

FeyP’s picture

Assigned: FeyP » Unassigned
Status: Active » Needs review
FileSize
1.81 KB

Attached is a patch against 8.x-1.x.

yukare’s picture

Status: Needs review » Needs work

Since we gonna touch this, can we improve the messages for status? In missing library add a link to readme.txt(it is in the project page) so the user can read the install instructions and fix it. And in css add something like: geshifilter needs that public://geshifilter exists and be writable, with a link to readme.txt too? We need to improve the readme with info about this, but we can do latter.

It may be a core bug, but we are wrong too, so lets fix this, thanks.

PS: i really love the new status page.

FeyP’s picture

Status: Needs work » Needs review
FileSize
4.11 KB

Sure, I added the link to the README file. The CSS mode requirement already had some good pointers in the description, so I just added the README file link there. New patch attached.

> PS: i really love the new status page.

Same here, it is a big improvement over the old one :)

Status: Needs review » Needs work

The last submitted patch, 4: 2868173-04.patch, failed testing.

FeyP’s picture

Status: Needs work » Needs review

  • FeyP authored fcc8f1a on 8.x-1.x
    Issue #2868173 by FeyP: Use strings prefixed by module name as...
yukare’s picture

Status: Needs review » Fixed

Commited to git, thanks!

Status: Fixed » Closed (fixed)

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