Here is a fix for four notices generated by system.module.
1. Notice: Undefined index: block callback in .../modules/system/system.module on line 344
The offending line:
if ($block['block callback'] && function_exists($block['block callback'])) {
can simply be fixed by using isset():
if (isset($block['block callback']) && function_exists($block['block callback'])) {
2. Notice: Undefined index: children in .../modules/system/system.module on line 360
The offending line:
if (is_array($block['children'])) {
can be fixed by checking whether $block['children'] is set before testing if it is an array:
if (isset($block['children']) && is_array($block['children'])) {
3. function theme_admin_page() generates the following notices:
Notice: Undefined variable: container in .../modules/system/system.module on line 1738
Notice: Undefined index: left in .../modules/system/system.module on line 1738
Notice: Undefined index: right in .../modules/system/system.module on line 1738
The offending line:
$container[$block['position']] .= $block_output;
can be called before either $container or $container['left'] etc is set. Two-part fix:
(a) initialize $container earlier at top of function with the extra line
$container = array();
and (b) change the setting line to e.g.
$container[$block['position']] = (isset($container[$block['position']]) ? $container[$block['position']] : '') . $block_output;
4. Notice: Undefined index: description in .../modules/system/system.module on line 1819
The offending line:
$output .= '<dd>'. $item['description'] .'</dd>';
generates a notice when a module with no description is found (e.g. devel.module). Suggest change to
$output .= '<dd>'. (isset($item['description']) ? $item['description'] : '[No description available]') .'</dd>';
(which is in the patch), or you could just leave a space.
Patch attached fixes these four points. Mark.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | system_notices_1.patch | 4.26 KB | asimmonds |
| #8 | notices_0.patch | 3.89 KB | RobRoy |
| #7 | admin-notices_2.patch | 4.18 KB | webchick |
| #6 | admin-notices_1.patch | 4.63 KB | webchick |
| #3 | system.module_33.patch | 2.19 KB | nickl |
Comments
Comment #1
beginner commentedI cannot test this patch because of non-standard line endings.
Please provide a patch with Unix line endings as per guidelines: http://drupal.org/patch .
Comment #2
beginner commentedComment #3
nickl commentedRerolled to fresh head.
Removed [No description available] to leave previous functionality intact.
Comment #4
nickl commentedComment #5
beginner commentedAll those items fix notices at ?q=admin.
(@plumbley: thank you very much for your detailed reports. In future reports, what would be useful is if you can say where and when those notices can be seen, i.e when looking at which web page).
About:
I tried to look for a place to initialize $container[$block['position']] but I found none obvious to me.
btw, the same code needing the exact same fix exist in theme_system_admin_by_module().
Should it be fixed at the same time?
About:
always at ?q=admin, to give an example, the description for the link "user settings" is "Configure default behavior of users, including registration requirements, e-mails, and user pictures.".
If one item does not have a description, I guess it is a useability bug, and it should be given one, so my vote is to remove this last fix, but add a description wherever one is missing.
If you disagree, you can set as RTBC: I won't object.
Comment #6
webchickHere's a patch that catches those and a few others from system.module.
Affected pages:
- admin
- admin/by-module
- admin/build/modules
- admin/build/themes
Comment #7
webchickActually, I found beginner's patch for initializing info['dependencies'] in module.inc here: http://drupal.org/node/88030 and I like that approach better. Removing that hunk from the patch.
Comment #8
RobRoy commentedI revised that last patch with two changes.
1.
instead of
as it reads better and is a bit more intuitive about what we're really doing.
2.
instead of
even though the latter doesn't generate a notice (I thought it would), I like the idea of clearly initializing a variable as an array because if this was deep in a super long function for example and there was already an array above named $container, we'd just be adding to it instead of initializing it. (I know that's not the case here, just explaining why I like it.)
Comment #9
webchickWorks for me. :) But then you should probably do that for theme_admin_page as well so it's consistent.
Comment #10
asimmonds commentedRe-rolled with webchick's suggestion in #9
Comment #11
webchickThis one still applies.
Comment #12
webchickActually, per Steven, I'm going to mark this one a dupe and instead lump it under a massive Fix all remaining notices patch.