Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Administer triggers produces the following errors:
Notice: Undefined index: triggers in trigger_assign_form() (line 134 of /var/www/foobar/modules/trigger/trigger.admin.inc).
Warning: in_array(): Wrong datatype for second argument in trigger_assign_form() (line 134 of /var/www/foobar/modules/trigger/trigger.admin.inc).
An bug fix are attached.
Comment | File | Size | Author |
---|---|---|---|
#10 | trigger.admin_.inc_.patch | 638 bytes | chx |
trigger.admin_.inc_.patch | 788 bytes | quiptime | |
Comments
Comment #1
jpmckinney CreditAttribution: jpmckinney commentedCNR for the testing bot.
Do we need the call to count()? I can't replicate the bug, btw. How did you do it?
Comment #2
quiptime CreditAttribution: quiptime commented- How did you do it?
I call the URLs
admin/structure/trigger/node
admin/structure/trigger/system
and so on
- Do we need the call to count()?
I think yes.
There are 2 situations with $metadata['triggers']
Alternatively, if you do not want count($metadata['triggers']), you can even use !empty($metadata['triggers'])
Comment #3
jpmckinney CreditAttribution: jpmckinney commentedBut you can call in_array() on an empty array -- it will just return FALSE.
I don't see those errors when I access those pages. Can you replicate it on a clean install of HEAD?
Comment #4
quiptime CreditAttribution: quiptime commentedYes, i will replicate it on an fresh HEAD installation.
Comment #5
quiptime CreditAttribution: quiptime commentedTest result with an fresh HEAD install.
The reported Bug no exist.
I have found the cause for the reported error: It is the module "Views Bulk Operations".
Sorry for my hasty error report. And thank you believed not immediately my error report. I have informed the maintainer of the Views Bulk Operations module about this problem.
Comment #6
infojunkieHi, I'm the maintainer of Views Bulk Operations. If I understand this report correctly, actions in D7 should all contain a 'triggers' attribute. Given that there are dozens of modules that declare actions, I suggest that it would be better to keep the guard in this patch as a robust way against actions that might neglect to contain this attribute (which is, after all, optional).
Comment #7
jpmckinney CreditAttribution: jpmckinney commentedChecking the rest of core, it looks like isset() && count() is a common pattern so I take back my only note.
Comment #8
Dries CreditAttribution: Dries commentedIt is not clear to me whether this needs to be fixed here, or in the calling code. I guess it is OK to fix here.
Comment #9
jpmckinney CreditAttribution: jpmckinney commentedYou think it should be actions_list()'s responsibility to clean up the array of actions (by assuming one of either array() or array('any') as the intended 'triggers' value)?
Comment #10
chx CreditAttribution: chx commentedIterating the array twice to find $hook or any? Ouch. Using count? Why on earth? If either $hook either 'any' is in the array then it's already nonempty.
Comment #11
catchWas going to ask for a test for this but it's not reproducable in HEAD, so we just need to confirm the existing tests don't break with the patch, which they don't. RTBC.
Comment #12
webchickCommitted to HEAD. Thanks!