Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
To produce this bug, define a requirements array without setting $requirements['value'], such as:
function example_code_requirements($phase) {
if ($phase == 'runtime') {
$requirements['example_code'] = array(
'title' => t('Example'),
'description' => "Blah blah blah",
'severity' => REQUIREMENT_INFO,
);
}
return $requirements;
}
The API documentation for hook_requirements at http://api.drupal.org/api/drupal/modules--system--system.api.php/functio... states:
value: The current value (e.g., version, time, level, etc). During install phase, this should only be used for version numbers, do not set it if not applicable.
So I would expect that it not need be set, but lines 2499-2506 of /modules/system/system.admin.inc expects this value.
// Output table row(s)
if (!empty($requirement['description'])) {
$output .= '<tr class="' . $severity['class'] . ' merge-down"><td class="status-icon">' . $severity['icon']
. '</td><td class="status-title">' . $requirement['title'] . '</td><td class="status-value">'
. $requirement['value'] . '</td></tr>';
$output .= '<tr class="' . $severity['class'] . ' merge-up"><td colspan="3" class="status-description">'
. $requirement['description'] . '</td></tr>';
}
else {
$output .= '<tr class="' . $severity['class'] . '"><td class="status-icon">' . $severity['icon']
. '</td><td class="status-title">' . $requirement['title'] . '</td><td class="status-value">'
. $requirement['value'] . '</td></tr>';
}
Not sure if the code or the documentation is in error.
Comments
Comment #1
FatherShawnHere's a patch that prevents the error.
Comment #2
droplet CreditAttribution: droplet commentedif $requirement['value'] = NULL, then should not output "value" part HTML code
(d8 first and backport)
Comment #3
FatherShawnThat makes sense, but why move this to D8? At present the omission of an optional parameter throws an error. Shouldn't we fix that directly in D7?
Comment #4
droplet CreditAttribution: droplet commented@FatherShawn,
read it : http://drupal.org/node/767608
Comment #5
FatherShawn@droplet
Thanks for the reference, that makes sense.
I don't agree, however about omitting the table cell if $requirements['value'] is null. The design of the Status Report really expects two columns and there's nothing wrong with an empty cell on a particular row when there is no data available for that cell.
Rerolled the patch against. 8.x
Comment #6
FatherShawnComment #7
kscheirer#5: requirements_params-1143922-5.patch queued for re-testing.
Comment #9
kscheirerrerolled against HEAD. Does this need a test? And you're right to keep printing the whole table cell, that's just how html tables work.
Comment #10
kscheirergah, for testbot.
Comment #11
FatherShawn@kscheirer Thanks for the reroll!
Comment #12
kscheirer#9: system-requirement_value-1143922-9.patch queued for re-testing.
Comment #14
FatherShawnRerolled the patch. Applies cleanly to today's HEAD: commit e21c1a1
Comment #15
FatherShawn#14: requirements_params-1143922-14.patch queued for re-testing.
Comment #17
FatherShawnRerolled the patch. This is a little piddling error, but it would be nice if one of the followers would have a look and mark RTBC so we can get it in...
Applies cleanly to todays HEAD, commit f7a4182e2fe
Comment #18
Crell CreditAttribution: Crell commentedMakes sense to me. Thanks, Shawn!
Comment #19
Crell CreditAttribution: Crell commented#17: drupal-system_requirements_value_optional-1143922-17.patch queued for re-testing.
Comment #20
catchCommitted/pushed to 8.x, thanks! This is sufficiently minor that I don't think it's worth adding a test for.
Moving to 7.x for backport.
Comment #21
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedPatch for d7 attached.
Comment #22
parthipanramesh CreditAttribution: parthipanramesh commentedworks fine.
Comment #23
David_Rothstein CreditAttribution: David_Rothstein commented21: drupal-system_requirements_value_optional-1143922-21.patch queued for re-testing.