Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I just noticed today that the list of Spam comments is completely broken in 6.x-1.x-dev...
I would expect a regular list, but instead the data is a big jumble!
I will post a PNG with what it should look like and how it looks now.
I'm talking about this list:
admin/content/comment/list/spam
That happens on all my sites so I'm pretty sure there's something fishy.
Thank you.
Alexis Wilke
Comment | File | Size | Author |
---|---|---|---|
#7 | spam-6.x-revert_init.patch | 2.36 KB | AlexisWilke |
#1 | spam-list-good-looking.png | 60.92 KB | AlexisWilke |
#1 | spam-list-broken.png | 19.45 KB | AlexisWilke |
Comments
Comment #1
AlexisWilke CreditAttribution: AlexisWilke commentedThere are the screenshots of a normal list and the broken one...
I know it's not the theme since I tested with several websites all have a different theme (plus if all lists work except that one...)
Thank you.
Alexis
Comment #2
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedthat list is looking fine on my test install.
Can you clear your caches?
Comment #3
AlexisWilke CreditAttribution: AlexisWilke commentedIt's broken on several sites, if the cache was the culprit, it would be strange...
I'll investigate on my end to see what could be wrong. Maybe I have some other module that creates the problem.
Did you do a get latest from git? (git pull) just in case...
Thank you.
Alexis Wilke
Comment #4
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedgit pull
Already up-to-date.
Didn't mean to set it fixed.
Comment #5
AlexisWilke CreditAttribution: AlexisWilke commentedKilles,
Okay. I understand what's happening, although I don't yet have a fix.
The module_implements() function caches the list of modules that support a given hook. If you call it "too soon" then the comment_spamapi() is not yet defined. When you rebuild the themes, somehow, the comment content is not yet there and thus the spamapi() is missing. The result is that the comment spamapi() doens't get called and the corresponding theme is "lost".
I know you changed the direct module_load() and moved them inside the hook_init() instead. This is a nice cleanup but I had not realized it could have such a side effect.
When I change the spam_invoke_api() by adding spam_load_inc_files() before doing any invoke, it works. My table is fixed.
The loading of hooks must be done around the time Drupal Core loads all the modules. Otherwise, it's too late. So we probably need to keep the load outside as it was before.
The hook_boot() is way to early (i.e. way before the modules are being loaded and most everything is not even defined.) So what I'm saying is that the call to spam_load_inc_files() is required immediately whenever the module is being loaded. Either that, or ALL the hooks in the content/* sub-folders need to be moved in the spam.module... or other modules need to be created (i.e. spam_node.module, spam_comment.module, etc.) which is similar to having the load as is in the main spam.module.
At this point, the module may be quite broken (certain things won't work because of this and it will be really hard to determine what.)
However, it is clear that adding a very clear documentation on why the includes are done immediately is necessary (if we choose this method.)
Thank you.
Alexis Wilke
Comment #6
AlexisWilke CreditAttribution: AlexisWilke commentedJust a note... Change was talked about in here: #1194730: Mark node as spam = page not found error
Comment #7
AlexisWilke CreditAttribution: AlexisWilke commentedThere is a patch with the fix and a long comment about why it is done that way. Also there was another call to the function somewhere else which I removed (not necessary.)
I'm not too sure how to replicate the breakage of the theme, although for me it was very easy to simply run Mini -> Rebuild theme registry.
At that point the comment table theme is not making it because it's after another hook_smthg() call and that hoot does not exist (yet) since it was not loaded.
Loading at the same time as the .module is the best solution we have (i.e. all those hooks should be defined before they may be called.)
Thank you.
Alexis Wilke
Comment #8
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedfine with me (for the record: I didn't make that change initially, just tried to fix it)
Comment #9
AlexisWilke CreditAttribution: AlexisWilke commentedOkay! Done. 8-)
It looked like you checked it in. But that being said, you may have used a patch from someone else... I also was thinking that using the hook_init() would work. Unfortunately, not if you load other hooks required from the time the .module is loaded. And I did not notice the problem for a while meaning that in many cases it's probably just fine with the hook_init().
Thank you.
Alexis
See: http://drupalcode.org/project/spam.git/commit/356b74a