Alter comment links.
neochief - February 14, 2009 - 18:23
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | comment.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
Currently, it's not possible to alter comment links from your own module.
Their links are not pasing through the drupal_alter. I've made a patches wich are fixing this. Please, take a look.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| comments_link_alter_D6.patch | 1.09 KB | Ignored | None | None |
| comments_link_alter_D7.patch | 1.07 KB | Idle | Failed: Failed to apply patch. | View details | Re-test |
#1
Hmmm, I'm fairly certain that
comment.modulealready has this. See for example this snippet (from D7, around line #1070):<?php
if ($comment = $result->fetchObject()) {
$comment->name = $comment->uid ? $comment->registered_name : $comment->name;
$links = module_invoke_all('link', 'comment', $comment, 1);
drupal_alter('link', $links, $node);
$output .= theme('comment_view', $comment, $node, $links);
}
?>
(Also, I think calls like
drupal_alterandmodule_invoke_allprobably shouldn't be called from the theme layer, as themeing should only be concerned with presentation.)If you still think this is a bug, please open a support request and describe in detail:
#2
Thanks for reply, mr.baileys. Your code affects only single-coment view (for example, when you replying to comment). But if you'll see little lower in the code (line #1132) in place where rendering comment threads, you'll see that this alter is not triggerring at all. I agree that altering in theme layer is not a best way. But invoking the hooks as well (#1830, #1865). Maybe we should place these hooks into comment_render and do averything there. If so, give me a shot and I'll prepare the patch.
I need these alters for my module ajax_comments. Currently I have an ability to override comment links only in preprocess function. And it would be enough for me, if preprocess functions called for theme functions as well. But they not. And if theme don't have a comment.tpl.php, I have no way to alter comment links at all.
#3
Thanks for the clarification. I do think this is a feature request though, as nothing is currently broken (marking as such). Can you supply a new patch for review?
#4
Here it is.
#5
The last submitted patch failed testing.
#6
#7
#8
The last submitted patch failed testing.
#9
Huh? Invalid PHP syntax? What is it? Patch is straight forward.
#10
Not sure, I've applied the patch locally without issues. You can request a re-test.
#11
I think it's because of #332967: Detect non-unix newlines . Here's a patch with LF only.
#12
#13
Bumping this up, as all test passed successfully.
#14
One approach that would be more work but much more useful would be to make comment thread use drupal_render() style array. then hook_page_alter() could fiddle with comment links. That need not stop this patch though.
#15
http://api.drupal.org/api/function/hook_link_alter/7 needs updating even if this is only an interim patch.
#16
@moshe weitzman, yes, I agree and I can handle it. But maybe it should be done in a new issue, and this one just commited as-is?
@catch, sorry, I don't understand what update do you mean. The hook_link_alter's concept wasn't changed by my patch at all, I just added it to missing places.
#17
neochief - the phpdoc for hook_link_alter() is completely wrong. This is more the fault of the node rendering patch than this one, but we should at least state explicitly that it works for comments now. Probably the parameter should be $comment instead of $node as well.
#18
This really needs to be fixed in Drupal 6 (and probably 7) because as it is now hook_link_alter() is run inconsistently on comments which isn't very good at all and should be considered a bug. It would have been a feature request only if hook_link_alter() was never run on comments.
Anyone having a plan on how we can solve this issue? Since we're in code freeze for D7 any plans on doing new cool solutions to solve this problem is no longer feasible, so the simple fix that the current patch contains should be applied on both D7 and D6 - since it's a bug fix and shouldn't be affected by any code freeze.
#19
comment links can now be alter in hook_comment_build_alter(), so moving this back to 6.
#20
Okay, so can anybody review patch for 6.x? It suppose to be working.