Problem/Motivation
Seeing The following warnings:
- Deprecated function: Implicit conversion from float 19.5 to int loses precision in bawstats_create_map_image() (line 256 of /bawstats/modules/render_map.inc.php)
- Warning: Increment on type bool has no effect, this will change in the next major version of PHP in baw_render_htmlchart() (line 50 of /bawstats/modules/render_htmlchart.inc.php)
- Warning: Undefined variable $site in bawstats_get_sites() (line 253 of /bawstats/bawstats.data.inc)
Steps to reproduce
Simply go to admin/reports/bawstats
Proposed resolution
Will provide a patch shortly…
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | Hours.jpg | 124.01 KB | bisonbleu |
| #17 | Days-of-week.jpg | 105.04 KB | bisonbleu |
| #17 | Days-of-month--rolling.jpg | 150.69 KB | bisonbleu |
| #16 | days-of-month-display-3531406-16.patch | 1.87 KB | egfrith |
| #16 | days-of-month-march-30-days.png | 17.27 KB | egfrith |
Comments
Comment #2
bisonbleu commentedHere's the promised patch. It applies cleanly to 2.x-dev when added to composer.json.
Comment #3
egfrith commentedMany thanks for the report and the patch.
I'm very happy to accept the patch for the files render_map.inc.php and render_htmlchart.inc.php.
However, the changes to bawstats.data.inc have alerted me to other changes I should make to prevent file names associated with sites other than the current site (i.e. current host) being cached, unless the bawstats_admin_access permission is enabled. I think I'd like to work on that in a separate issue. Are you OK with me committing just the changes you've suggested for render_map.inc.php and render_htmlchart.inc.php, or would you prefer to re-roll the patch with just those changes?
Comment #4
egfrith commentedComment #5
bisonbleu commentedI'm ok with providing a new patch. Can it excludes just this ?
@@ -250,8 +250,7 @@ function bawstats_get_site_files_yearmonth($regenerate_cache=FALSE, $site=NULL)
* If TRUE, clear the cache and regenerate it
*/
function bawstats_get_sites($regenerate_cache=FALSE) {
- $site_files_year_month = bawstats_get_site_files_year_month($regenerate_cache, $site);
+ // Pass NULL instead of undefined $site variable.
+ $site_files_year_month = bawstats_get_site_files_year_month($regenerate_cache, NULL);
return(array_unique(array_keys($site_files_year_month['files'])));
}
-
-?>
Comment #6
bisonbleu commentedHere a new patch where I've only removed the last change in bawstats.data.inc (see #5). Does that work for you?
Comment #8
egfrith commentedGreat - I have committed the float-to-int conversion bits, and I've created a separate issue for the issues I wanted to fix in bawstats.data.inc, which your patch alerted me to: see separate issue: #3531663: data files caching fixes and improvements.
Comment #9
egfrith commentedI've discovered a problem with the part of the patch in render_htmlchart.inc.php . The change from
$top_x++to
has resulted in the monthly totals being displayed incorrectly.
Before patch:

After patch:

Thoughts welcome!
Comment #10
bisonbleu commentedHm… I'm running the dev version of the module from a few days ago with only my 2 patches (not yours yet):
- implicit-conversion-from-float-to-int-3531406.patch
- timezone-must-be-explicitly-set-to-utc-3531531-2.patch
What I see:
$top_x++or$top_x = is_numeric($top_x) ? $top_x + 1 : 1;, Monthly History displays correctly (I don't see what you see in capture 2);$top_x++, I get «Warning: Increment on type bool has no effect», but not so with$top_x = is_numeric($top_x) ? $top_x + 1 : 1;.Did you try flushing all caches?
Could it be because of fixes you introduced?
Any particular errors in the watchdog?
An then there may be a problem with -3531531-6.patch. I'll check tomorrow.
Note - Maybe unrelated, but in render_htmlchart.inc.php there are quite a few occurrences of
$some_var++where $some_var might be set tofalse. So these will eventually need to be fix as well I guess…Comment #11
bisonbleu commentedNow having a look at the capture labelled «After patch:» which shows not the Monthly History (which displays fine with the latest dev version) but rather Days of month. Ok, I see what's wrong. Let's see if I can figure why that is…
Comment #12
bisonbleu commentedA patch will follow shortly.
Comment #13
bisonbleu commentedHere's the patch… It applies cleanly to the latest 2.x-dev when added to composer.json.
Similar work needs to be done for the following:
I suggest we do this in another ticket.
Comment #14
bisonbleu commented@egfrith I sent you an email a few days ago using the contact form of your d.o account, did you get it?
Comment #15
bisonbleu commentedCreated a separate issue for Days of Week & Hours charts: #3532354: Days of Week & Hours charts are broken
Comment #16
egfrith commentedMany thanks for this patch. However, I think the hard-coding of 30 is causing problems for months with 31 days: see below, where March has 30 days and then "others".

I feel that the root cause of this patch is in render_htmlchart.inc.php, which is (I think) designed to deal with a variable number of rows in the chart when $top_x==false. I'm proposing an alternative patch that only increments $top_x if it's numeric. If it's false, it remains so throughout the rest of the function, which is important later in the function. I've tried to document the function at the top, which should explain how it's working. (I didn't write this function, so I've inferred the docs from the code.)
Could you test this patch please?
Comment #17
bisonbleu commentedOh… looks like you found the Holy Grail… the patch that patches them all!
Tested just this patch (removed days-of-week-and-hours-charts-3532354.patch) and now all charts render perfectly. Even Days of Month is rolling!
I spent time in render_htmlchart.inc.php but could not quite untangle its intricacies. And so my patches are «localized» i.e. Zork grade, at flashlight point, deep down in a dark cave. ;^)
So I also marked #3532354: Days of Week & Hours charts are broken as Closed (outdated).
Comment #19
egfrith commentedThank you for testing! (And for your kind words.) baw_render_htmlchart() is quite complex... I will mark this as fixed and release an updated version.
Comment #20
egfrith commented