I have found in logs that path tagadelic/chunk/images/x.gif gives a warning:

implode() [function.implode]: Argument to implode must be an array. в файле /home/..../sites/all/modules/tagadelic-6.x-1.0/tagadelic.module в строке 212.

To fix this change:


function tagadelic_get_weighted_tags($vids, $steps = 6, $size = 60) {
//Fixing a bug with wrong path. Start
if (!is_array($vids)) $vids=array();
//Fixing a bug with wrong path. End

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vladimir.dolgopolov’s picture

My opinion here is just return an array() instead of $vids = array();
Why should we read from the cache wrong data?

function tagadelic_get_weighted_tags($vids, $steps = 6, $size = 60) {
  if (!is_array($vids)) {
    return array();
  }
  // ...
vladimir.dolgopolov’s picture

FileSize
748 bytes

Here is my patch.
It against module's HEAD.

Bèr Kessels’s picture

Can someone please test this patch?
I like its concept, and like the solution, but need at least one more person to confirm it works and does not break his/her site.

brainlock’s picture

I didn't apply the patch, but made the code change and now receive a "Page Not Found" error, which is acceptable since no tags exist yet.

vladimir.dolgopolov’s picture

You should clear the cache IMHO.

Bèr Kessels’s picture

if cache is causing this, then we need to implement this in the patch: we should clear the cahce in hook_update, in that case.

Bèr Kessels’s picture

Status: Needs review » Needs work

patch should implement a cache wipe in the update, else people get warnings.

rkdesantos’s picture

I am seeing a similar error in v5:

Message implode() [<a href='function.implode'>function.implode</a>]: Argument must be an array in /home/..../modules/tagadelic/tagadelic.module on line 208.

rkdesantos’s picture

I'm interested in getting a fix against D5 but has this ever been fixed against either D6 or D5?

Bèr Kessels’s picture

IMO we should travers up the calling functions and fix it in the first place: simply make sure we never call the function tagadelic_get_weighted_tags without an array if vids!

ilfelice’s picture

Subscribing

rkdesantos’s picture

Any action to address this issue?

Bèr Kessels’s picture

@jorgemare instead of subscribing and spamming us with updates, you could actually do somthing about it :)
@rdesantos: see #10. Maybe you can address it?

rkdesantos’s picture

@Ber Kessels: If I had the necessary knowledge or skills I'd be happy to do so. Unfortunately I don't think I do. My question was an inquiry on the status and that's all.

Bèr Kessels’s picture

Closing "needs work" that has been open for a long time, without anyone working on it.

Bèr Kessels’s picture

Status: Needs work » Closed (won't fix)
kalman.hosszu’s picture

Version: 6.x-1.0 » 6.x-1.x-dev
Status: Closed (won't fix) » Needs review
FileSize
575 bytes

A new patch is attached. This solves the problem on my environment. Would somebody test it?

rkdesantos’s picture

The equivalent patch seems to be good in V5 as best as I can tell. Will test V6 later and advise.

Bèr Kessels’s picture

Thanks for the patch and the test. I'd like a confirmation about how it works on d6.

Also, if you are familiar with Github, consider sending a pull-request there: that is a lot less work for me and the other maintainers to merge, review and test. It will speed up acceptance of a patch enormously.

About the patch:
As I said in #10:
» IMO we should travers up the calling functions and fix it in the first place: simply make sure we never call the function tagadelic_get_weighted_tags without an array if vids! «

This is addressed just fine, thanks!

But, this introduces a so-called "early-return" which, in the already horrible factored (sorry, my fault) code makes it harder to follow and debug. All in all: I'd very much prefer a solution that solves this by refactoring a little more. I've done a quick refactoring of this method, which may be an example: https://github.com/berkes/tagadelic/commit/3ded2f6a20393afe2be67435cadc1...

Bèr

rkdesantos’s picture

I've tested v6 and it appears to be ok with the patch.