| Project: | Calendar |
| Version: | 7.x-3.0-alpha2 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
The errors:Warning: Division by zero in template_preprocess_calendar_week() (line 491 of /var/www/html/sites/all/modules/calendar/theme/theme.inc).Notice: Undefined index: groupby_times in template_preprocess_calendar_week() (line 477 of /var/www/html/sites/all/modules/calendar/theme/theme.inc).
The groupby_times issue is related to: #1398584: calendar-date/week generates errors: Undefined index: groupby_times in template_preprocess_calendar_week().
This seems to expose a coding flaw.
The code on question is:
<?php
foreach ($grouped_items[$start_time]['values'] as $wday => &$items) {
foreach ($items as &$item) {
if ($display_overlap) {
switch ($view->style_options['groupby_times']) {
case 'half':
$group_time = 30;
$divisor = 7.5;
break;
case 'hour':
$group_time = 60;
$divisor = 15;
break;
default:
continue;
}
?>There are two spots in calendar/theme/theme.inc where this issue exists: ~line #265 and ~line #475.
I believe the logic is that the loop should continue if $view->style_options['groupby_times'] is neither 'half' nor 'hour'.
The problem is that the 'continue' command is inside of a switch() command.
When 'continue' is executed inside of a switch block, it applies to the switch() statement itself and not the for() loop.
This is a problem, because the code will generate the divide by zero later on because $divisor is undefined.
Here is a simple php snippet that proves the switch/continue usage bug (could this be php version specific?):
<?php
foreach (array('x', 'y', 'x', 'a', 'b', 'c') as $value){
switch ($value) {
case 'a':
break;
default:
continue;
}
print("$value!\n");
}
?>The above example should ONLY print "a!", but it does not.
Instead, the following code has to be used:
<?php
foreach (array('x', 'y', 'x', 'a', 'b', 'c') as $value){
if ($value == 'a'){
// do nothing
}
else {
continue;
}
print("$value!\n");
}
?>Making this fix exposes yet another bug in that $item['class'] does not get initialized so the following error happens:Notice: Undefined index: class in include() (line 201 of /var/www/html/sites/all/modules/calendar/theme/calendar-week-overlap.tpl.php).
So I opted to initialize $item['class'] before executing the continue command.
Attached is my patch that changes the for+switch+continue statements to for+if+continue statements in the two spots that I happened to notice the problem.
Please Review.
| Attachment | Size |
|---|---|
| calendar-7.x-3.x-for_switch_continue-to-for_if_continue-2.patch | 2.26 KB |
Comments
#1
I screwed up and made a copy&paste mistake where the both if conditions where checking for 'half'.
This next patch hopefully has no more mistakes.
Please Review.
#2
This should have been se to needs review.
#3
I can't replicate any error but I can see the logic of the fix, so I guess I'll go ahead and apply it.
#4
http://drupalcode.org/project/calendar.git/commit/0d47bd7
Thanks!
#5
Automatically closed -- issue fixed for 2 weeks with no activity.