#new links don't work on paged threads
singularo - March 2, 2004 - 08:34
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | comment.module |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Description
If you have a lot of comments on a node/forum topic, the recent posts link does not take you to the correct page to view the comments. Instead, you are sent to the first page.
For example, the recent posts on www.dirtbike.ws lists this as the most recent comment
However, when you click onto it, you are not taken to that comment, but just to the start of the first page.

#1
confirmed. this is happenning on long threads at drupal.org as well.
#2
There's a bit more discussion in this duplicate. Additionally, the patch in this issue might fix this problem... I'm not sure though.
#3
The "New posts" link on the Forum topics page doesn't work if there are multiple pages sorted by "oldest first".
Here is a fix.
Sorry! I don;t know how to make diffs, so I commented the changes instead!
I didn't set this to patch because it's not a diff file
#4
Uh, you might want to put a ";" after "global user" or it will break.
#5
And this function is from forum.module, not comment.module.
Sorry if that is confusing
#6
I neglected to offset the number of forum replies here's a new version.
Sorry posting so many times. This one works.
I also fixed this issue in the tracker and will post the fix there.
Does anyone know a good free diff maker for windows?
#7
Great overall CVS tool for Windows:
http://www.tortoisecvs.org/
You can use it do almost anything which you would do from a command line, with little less flexibility of course, but much more convenience. After you make a drupal checkout and make a change to some file, open windows explorer (or any other standard file-tree browser), navigate to your file, right-click and select "Make Patch" from CVS menu.) It usess "-u" by default, so all you need to do is to select a place to save the patch.
HINTS: When making a checkout you may want to select "Use UNIX line endings" under Options.
DISCLAIMER: I only made two such patches so far, and none of them was yet reviewed, so I might be completely wrong abut this :-)
#8
perhaps someone wants to take on this issue. its rather frustrating on popular threads.
#9
I've been looking into this. It's tough--beyond my hacking skills at the moment. But I think what needs to be done is in comment_render() in comment.module. After around line 883, when we do the pager_query, we gotta check to see if the '#comment-x' line is in the URL, and if so, cycle through all the results, making sure that the one you want is there. Otherwise, you gotta change the "from" in the URL (making sure to preserve the numbers in other pagers) and run the pager query until you find the comment you want.
As far as I can tell, there's no easy way to tell what page a comment will appear on just by looking at it. If there is, obviously that would be better.
I don't know whether it's faster to do that check during the theming, or do a separate loop before the theming. The latter seems like the better idea.
I'm not assigning this to myself because as I said, it's a little beyond me right now. But I thought I'd put this out there.
#10
http://drupal.org/node/12333 was a DUPLICATE of this bug.
#11
http://drupal.org/node/12324 was a DUPLICATE of this bug.
#12
http://drupal.org/node/23097 was a DUPLICATE of this bug.
#13
http://drupal.org/node/13177 was a DUPLICATE of this bug.
#14
There are about what, 5 patches to this bug floating around? Why the hell hasn't one made it into the system yet?
#15
BUMP. We should really try to fix this for 4.7., it's been there way too long now.
#16
Another duplicate: http://drupal.org/node/22316
#17
I was looking at another one...
http://drupal.org/node/26966
So I guess nobody take care of this issue ?
Who is maintaining comment.module ? I'll try to supply a patch, but I'm not sure it would cover all issues. Review is welcome.
#18
this is for comment.module
I don't know if it's 100% right
it's a compromise; it's works for me
it use the fact that always the latetest comments ARE on the last page
if 2 showed latest comments are in two different pages (the last one & the last - 1 one) ... wrong: both are linked as they are on the last page. but it's better than actual behavior.
#19
There was a fix for tracker new link for 4.5, but im using 4.6. Does anyone know, where to get a fix for this issue on thre tracker new comment link for drupal 4.6.x ?
#20
Still affecting drupal.org. Try visiting this page and click any of the comment titles. It should take you to the second page, directly to that comment, but instead takes you to the first page of the thread.
#21
This is a real longtimer. A fix would probably come in handy for long forum topics to be expected now that 4.7 is out :)
With this patch you are always sent to the right page in the following situations:
- clicking on a link in 'recent comments' block
- clicking on '# new comments' node link
- clicking on '# new' in forum topic lists
- clicking on '# new' in tracker lists
- after submitting a comment
The patch introduces three new functions:
_comment_page($nid, $cid)Returns the page a comment is on
_comment_page_query($nid, $cid)Returns a query string used in links to comments to point to the page the comment is on
_comment_first_new($nid)Returns the specified node's first new comment for the current user
I know this is a lot of code for such a tiny bug but it's the many ways you can display comments that makes it rather complex.
#22
While I think that it would be nice to fix the issue, I think your solution uses too many ressources for this.
#23
This does seem like a lot of code and queries to fix this. I would think a solution that would find the page number as part of the origional query would be best.
#24
Yes, it's true. The above patch had some redundant database queries. Here is a new one.
The first optimization step was to spot the two cases where we need to know the page a comment is on.
The next step was to optimize the number of database queries needed to get this page.
The result are two generic functions that return a 'page=#' query string for each case:
Case 1:Get the page a specific comment is on ('recent comments' block, after comment submission)
comment_page_query($comment)where $comment is a comment object
required fields: 'nid', 'timestamp', 'thread'
optional: 'comment_count' (saves a count query and is retrieved with a JOIN on the node_comment_statistics table)
Case 2:Get the page a node's first new comment is on ('# new' links)
comment_page_new_query($nid)Additional queries overview
'recent comments' block
additional queries per comment when the affected node has
one page of comments: 0
multiple comment pages: 1
+1 query per node if the user has the 'administer comments' permission
'# new' links
additional queries per node when the affected node has
one page of comments: 0
multiple comment pages: 2
+1 query per node if the user has the 'administer comments' permission
So in most cases (only one page of comments and user has no 'administer comments' permission) there are no additional queries needed.
#25
The cases in your last follow-up should be in the inline comment documentation with @param, etc.
By your summary we are adding one query per node, so that will be 10 for a default front page, and similar numbers elsewhere.
#26
bump. just so I can keep an eye on it. really want this one to be fixed.
#27
This new patch includes the suggested inline documentation for
comment_page_query()andcomment_page_new_query()and a little optimization. Tested with MySQL and PostgreSQL.Updated additional queries overview:
'recent comments' block
additional queries per comment when the affected node has
one page of comments: 0
multiple comment pages: 1
'# new' links
additional queries per node when the affected node has
one page of comments: 0
multiple comment pages: 1
For users with the 'administer comments' permission:
+1 query per node in both cases.
So the additional query per node is only for users with the 'administer comments' permission because we have to count published and unpublished comments. This number is not part of the node_comment_statistics table.
For regular users (anonymous user, registered users without 'administer comments' permission) there are no additional queries needed as long the comments fit on one page.
This is clearly a 4.7 solution only. In HEAD this could be better solved by linking to comments like this
node/23/comment/34ornode/23/comment/firstnewand then redirect to the correct page. By doing it this way you also get always working permalinks to comments no matter what page the comment is on.#28
No longer applies.
#29
I would like to see http://drupal.org/node/31645 reviewed and merged in if needed. This approach looks cleaner than that patch (although it doesn't seem to add any queries).
#30
Just to throw a suggestion in, pyromanfo had put together several patches relating to multiple pages of comments a while back. They were never commit-ready, and are bitrot at this point, but might still be minable for some ideas.
http://drupal.org/node/24880
http://drupal.org/node/24882
http://drupal.org/node/24881
http://drupal.org/node/24884
#31
Hi, not my code, but one of our forum users made some alterations to forum.module to achieve this. It worked with 4.7.2 but when we (without his help this time) tried to apply it for 4.7.3 it didn't - might be me rather than the code though!
He also made some changes to comment.module which changes the redirect after posting to go to the comment you just posted - again very useful for multiple pages of comments. We could really do with this fix since we have threads running to hundreds of posts, and more importantly - hundreds of users missing this functionality from phpbb. Hope this is the right place to put it, and hope it helps. I personally can't do a lot more with this code than post it up here - like I said it's not my code.
This is the modification to forum.module:
"function forum_get_topics"
replace the if function at the end with this:
if ($topic->num_comments > 0) {$last_reply = new StdClass();
$last_reply->timestamp = $topic->last_comment_timestamp;
$last_reply->name = $topic->last_comment_name;
$last_reply->uid = $topic->last_comment_uid;
//*************************************************
// Get the cid for the last comment so we can build
// a link to it on the forum list for each topic
$cid_sql = db_query("SELECT cid FROM comments WHERE timestamp = '" . $last_reply->timestamp . "';");
$cid_obj = db_fetch_object($cid_sql);
$last_reply->cid = $cid_obj->cid;
//*************************************************
$topic->last_reply = $last_reply;
}
function get_forums
replace from the $sql query onwards:
$sql = "SELECT n.nid, ncs.last_comment_timestamp, IF (ncs.last_comment_uid != 0, u2.name, ncs.last_comment_name) AS last_comment_name, ncs.last_comment_uid FROM {node} n INNER JOIN {users} u1 ON n.uid = u1.uid INNER JOIN {term_node} tn ON n.nid = tn.nid INNER JOIN {node_comment_statistics} ncs ON n.nid =
ncs.nid INNER JOIN {users} u2 ON ncs.last_comment_uid=u2.uid WHERE n.status = 1 AND tn.tid = %d ORDER BY ncs.last_comment_timestamp DESC";
$sql = db_rewrite_sql($sql);
$topic = db_fetch_object(db_query_range($sql, $forum->tid, 0, 1));
$last_post = new StdClass();
$last_post->timestamp = $topic->last_comment_timestamp;
$last_post->name = $topic->last_comment_name;
$last_post->uid = $topic->last_comment_uid;
//******************************************
// We need to get the last post cid here too
// But we also need the nid of the last topic
// since it isn't saved to the $forum variable
$cid_sql = db_query("SELECT cid FROM comments WHERE timestamp = '" . $last_post->timestamp . "';");
$cid_obj = db_fetch_object($cid_sql);
$last_post->cid = $cid_obj->cid;
//******************************************
$topic->last_reply = $last_post;
$forum->last_topic = $topic;
$forums[$forum->tid] = $forum;
function theme_topic_list
last line of the $rows declaration - change to this:
array('data' => _forum_lastreply_format($topic), 'class' => 'last-reply')function theme_forum_list: - class last reply again:
array('data' => _forum_lastreply_format($forum->last_topic), 'class' => 'last-reply'));and a new function - obviously this is creating a new link to the last post rather than replacing the "new" link, but principle is the same:
/**
* Custom topic formatter for last reply
*/ function _forum_lastreply_format($topic) {
if ($topic && $topic->last_reply->timestamp) {
//***************************************
//Modified to create a link to the last post
$comments_per_page = variable_get('comment_default_per_page', 0);
$result = 0;
$pagenum = "";
if ($comments_per_page > 0) {
$result = $topic->num_comments / $comments_per_page;
if ($result > 1)
$pagenum = "?page=" . (intval($result) + 0);
}
return t('%time ago<br />by %author <a href="/node/' . $topic->nid . $pagenum . '#comment-' . $topic->last_reply->cid . '"><img src="/misc/forum-default.png" /></a>', array('%time' => format_interval(time() - $topic->last_reply->timestamp), '%author' => theme('username', $topic->last_reply)));
//***************************************
else {
return message_na();
}
}
}
NB - the "intval +0" was an even more hacky fix by me when it was taking you one page ahead of where it should've been.
Then in comment.module - to get the redirect to go to the comment you posted instead of the original posta complete replacement of this function:
function comment_form_submit($form_id, $form_values) {$form_values = _comment_form_submit($form_values);
if ($cid = comment_save($form_values)) {
//******************
//Add paging support for new comment redirects
$comments_per_page = variable_get('comment_default_per_page', 0);
$comments_sql = db_query("SELECT comment_count FROM node_comment_statistics WHERE nid = " . $form_values['nid'] . ";");
$comments_obj = db_fetch_object($comments_sql);
$num_comments = $comments_obj->comment_count;
$result = 0;
$pagenum = NULL;
if ($comments_per_page > 0) {
$result = $num_comments / $comments_per_page;
if ($result > 1)
$pagenum = "page=" . (intval($result) + 1);
}
return array('node/'. $form_values['nid'], $pagenum, "comment-$cid");
//******************
}
}
#32
#33
see this for views module: http://drupal.org/node/99783
#34
Node 81866 was a DUPLICATE of this bug.
#35
Just installed 5.0-beta2 and confirmed this bug is still present. Updated version for this issue.
#36
Hang on - can't we get a fix for 4.7? I just found this bug myself, and I can't believe people have working on this since May with no definitive answer... this is pretty fundamental stuff, not just for forums but the whole site. Is anyone working on it?
#37
Surely somebody needs to take ownership of this? I'm not remotely qualified, but someone out there must be. This has been outstanding since May this year if not longer, and as a "normal" priority bug this broken functionality in the core of Drupal has been left to fester through several releases already, so I'm raising it to critical to get the attention of someone working on Drupal's core... before you lower it again, please think carefully about whether it's really acceptable to leave this broken for any longer?
The way I see it, this is a bug in comments.module. Any link to #new or #comment-123 etc should take you straight to that first new comment, or comment number 123 or whatever, regardless of what page that comment falls on (which would depend entirely on your own display settings for number of comments per page). This is core functionality which effects every part of Drupal, and which particularly damages Drupal's reputation for forums.
#38
This is an annoying problem, but it requires some significant changes to be fixed. I'm postponing this to 6.x, so we can take enough time to find the best solution.
#39
And it won't be fixed even for 5.0 too?
#40
Even 5.0 can't fix everything. This problem is a nontrivial one; there's no way to fix it without seriously changing some underlying Drupal functionality, and we're too late in the 5.0 cycle to wedge something like that in. It *is* an important issue, but like other things, it will have to wait until the next release cycle.
#41
I guess I'll be skipping 5 and waiting in baited breath for 6, then.
#42
That seems kind of silly, but if you don't want any of the improvements and fixes in 5, more power to you. :-)
#43
In all honesty I'm not aware of anything else broken in 4.7.4 that needs fixing, aside from this issue. I'm sure 5 has some improvements, but some (such as web-based installation) mean nothing to a working site, and I'm not aware of anything else that I particularly need that would warrant the upheaval of an upgrade. Obviously if and when support for 4.7 is withdrawn that might change, but that's not happening yet (and when it does hopefully 6 will be up and running, and this fix will finally be on the cards).
It seems kind of silly to me to be putting so much effort into rolling out new features when the existing code (on which these new features are being built) is broken. But it's not up to me I guess.
#44
As a temporary solution would it be possible for the "new" links on the tracker to take you to just the last page of comments by default? That would be quite a minor change no?
If not, does anyone know how this could be achieved via views or similar.
#45
Catch - I think the issue is that some logic (and database querying) is needed before the comment module knows what page the "new" comment is on, or even just what the last page is, as it depends entirely on the users display settings. If I'm set to 50 comments per page and you're set to 30, then the "last page" on any thread is going to be completely different for both of us.
#46
I can't help noticing this has all gone quiet since it was put off till 6.x - are we all thinking it doesn't need to be worried about now?
Can I remind people that this is a pronounced bug (not just a missing feature) that affects both 4.7 and the about-to-be-released 5.0? The only reason it's been punted off to be fixed in 6.x is that - apparently - there's no time to fix it in 5.0. However, if we all carry on ignoring it now, aren't we just going to get till the launch of 6.0 and say "hey, it's too late to fix it now, let's leave it till 7.x...?"
I maintain this is a big ugly bug that needs to be squashed now, preferably in 5.x and ideally in 4.7 too. At the very least there needs to be some proper prioritisation of it so that we don't just *say* it will be fixed in 6.x, but actually ensure that happens.
I wish I had the time or ability to do it myself, but I don't - the only useful thing I can do is nag, unfortunately.
#47
I agree it's a very nasty bug, having the "new" and individual comment links working consistently is a significant usability issue on long threads, and on our site the issue comes up from users much more often than it does on here.
The main problem seems to be the variation in paging that occurs by allowing users to set comment options.
Since user comment options is an optional admin setting, one way to do it might be to assume admin settings for paging so it works for that default. Then add a warning for admins (and maybe users) if user comments settings are enabled/altered that it'll break the links if the number of comments per page is customised. That way it'd work for the vast majority of users instead of none and be at least partially less complex.
#48
This comment on a forum topic details a way to change the redirect from comment posts to the last page - linking in case it's relevant to this issue. I may open a seperate issue for this redirect - since most users would prefer to see their comment after it's posted.
http://drupal.org/node/79352#comment-178013
#49
Subscribe.
#50
I too am seeing this on my 4.7 site, as are my users who are voicing their annoyance now - it's a pretty major bug in the whole comments system, as it's effectively breaking the "Ease of social discussion and interaction", that is virtually the core concept of Drupal.
#51
OK. So this isn't my code, one of our forum users contributed it here: http://libcom.org/forums/feedback/jump-straight-to-last-post - I've just stuck it into a patch.
Also the patch is against 5.1 since that's what I'm working on, but my next post will be one against HEAD.
We're pretty sure this'll only work for flat comments - not threaded. I think it'll probably need to be refactored eventually to find the last unread comment cid per node along with working out which page it'd be on from user settings, instead of the #new (and does having multiple anchors for the same thing on one page work anyway?) - but that's a significant reworking of comment/forum/tracker which is likely to be beyond us.
I'd love to see this fixed in 6.x so hopefully it'll be a start towards an acceptable solution, since the current method is completely broken.
#52
alright so this is the same thing, but patch against HEAD this time.
#53
The proposed patch won't work if the user has changed their per-page comment settings. It should use _comment_get_display_setting('comments_per_page') to calculate the page number.
It also has coding style issue: IF statement and string concatenation spacing; see http://drupal.org/node/318
#54
Thanks for the feedback. My second patch ever, so I'll do my best to tidy up and repost with the corrections.
#55
So am I right in thinking that replacing:
variable_get('comment_default_per_page', 0);
with
_comment_get_display_setting('comments_per_page');
will fix the first issue?
Attached replacement 6.x patch in case that's right and marking back to review. Done some style changes on the code as well but it may well not be enough to fix everything - this is all pretty new to me.
NB. If it uses _comment_get_display_setting then presumably this patch won't work if users have posts threaded, most recent first etc. right? Does that mean a load of switch statements depending on the four variables (flat x2, threaded x2)?? If so, that's probably beyond me, but will stick at it.
By the way, anyone who'd like to try this out on my live site please feel free to do so at http://libcom.org - you'll need to register to get the "new" links of course. Not yet updated with _comment_get_display_setting though.
#56
OK a new patch - also supplied by our same forum user (i.e. not my code).
He said:
This is against 5.1, not HEAD. I'll try to roll one up against HEAD when I get back to my own PC. Would really appreciate reviews on this.
Assuming there's not an easier/cleaner way to do this and if this code is too dirty for core then I'm going to post a new issue suggesting that comment options apart from posts per page be taken out of core - plus maybe options per content type. See http://www.lullabot.com/articles/drupal_usability_comment_configuration for some of the arguments on this.
#57
A simpler solution attached.
-K
#58
This is non-critical.
#59
Zen - I don't see how that patch deals with either the tracker or forum new links, could you explain?
Come to think of it - should this really be two related bug reports against tracker and forums (and views which uses the same logic in the "new" links)?
The comment module creates pages with usable links (notwithstanding comment options), it's tracker and forums which deal with the "new" links.
#60
Just to let you know... i used tenrapids solution #27 and it works.
http://drupal.org/node/6162#comment-98014
i also modified views_comment.inc to fix the same problem in views module
<?php
/*
* Format a field as a number of comments, plus the number of unread comments.
*/
function views_handler_comments_with_new($fieldinfo, $fielddata, $value, $data) {
$comments = intval($value);
if ($comments && $new = comment_num_new($data->nid)) {
$comments .= '<br />';
- $comments .= l(t('%num new', array('%num' => $new)), "node/$data->nid", NULL, NULL, 'new');
+ $comments .= l(t('%num new', array('%num' => $new)), "node/$data->nid", NULL, comment_page_new_query($data->nid), 'new');
}
return $comments;
}
?>
#61
subscribe
#62
Just to come back on this from tenrapid:
The one additional query if comments go to an additional page is more than outweighed if users no longer have to go to the first page (at say 90 queries per page), then manually browse to their unread comments using the pager. That's one extra query vs. 90 (or 60, or 120 depending on what's running n terms of blocks).
I'm going to change this from comments to forum module, since it's only really a #new links problem, and they're only present in the forum and tracker. Will have a quick look for matching issues against tracker and views to make sure they're tied up.
#63
And a change of title.
#64
OK so there seem to be two ways to deal with this:
1. one of the variations on: http://drupal.org/node/6162#comment-98014 - determining from settings and comment numbers whether there's multiple pages and making the #new link point to those.
2. the more drastic (but with potentially with more applications) solution of comment permalinks - something like the format
http://drupal.org/node/6162/comment/98014
or:
http://drupal.org/comment/98014/node/6162 (in-context)
http://drupal.org/comment/98014 (out-of-context but maybe with breadcrumb trail/link to the in-context view)
I have a couple of questions:
1. option 1 is a straight bug-fix (if apparently not a popular one judging by the history of this issue), option 2 would be a feature
1. Option one is a bug-fix, so I guess it could get into Drupal 6 before code freeze (but probably not after rc?), Option two I know won't becuase it'll be called a feature even though it fixes a very annoying bug.
It'd be good to guage what kind of need/support there is for comment permalinks, but ideally option 1 (even if the code needs fixing/optimising which is beyond me), could get into drupal 6 and option 2 as a feature request for drupal 7.
How's that sound?
#65
Subscribing.
#66
OK I'm marking this back to code needs work, because it does - although at least a couple of solutions work, they just need tidying/optimising which is a bit beyond me. And also back to critical, because on any busy community site it's a major issue with people not being able to follow conversations past a certain length, and it's something that affects at least three modules.
#67
I would very much like to keep this bug on the radar. In fact, if I could entice someone to fix for 5.x, I would greatly appreciate this.
#68
Subscribing ... this is a big issue for us ... when a user clicks on a link, they shouldn't get a surprise when they end up somewhere else (I know everyone here understands that -- just venting)!
Any update on whether this will be in 6.0?
#69
this will be fixed in D6 if someone makes a commitable patch. there is still plenty of time.
#70
I reviewed both #56 and #57.
#56:
- tries to figure out what page of comments to link to by your settings (this is a good idea :)
- does not modify the date storage code, so although you only see a page of new stuff, if not all the new stuff was on the page, all other new stuff is also marked as old (read)
#57:
- links to individual comments instead of conversations
- this is bad UI-wise, you cannot get the context
- it still saves the current date, so *all* other comments are marked as read
So with #57, you see less of the new stuff, but with both patches, you still mark all new stuff as read, by visiting any link as far as I see. So none of the patches solve the issue at hand.
I would really much support a real solution in Drupal 6 by the way. This is an issue for a long time.
#71
Gabor:
When comments are on ?page=1 or higher, you don't see them at all - you just get taken to node/123#comment-1234 which tries to take you to a non-existent anchor (because it's not on that page). This also marks all the new stuff as old, so you never get the benefit of those links.
Like you said#56 fixes this by handling each of the four different options seperately (+ comments per page) - I have a slightly different version of the patch working fine on my site at http://libcom.org (you'll need to register an account to get the #new links).
However, the patch is a fudge (as admitted by both my friend who wrote the code, and me who rolled the patch), and chx has said that the only way to fix this properly is a ground-up rewrite of comment.module - something I'm keen to assist as much as possible with for drupal 7 (or D6 if it was at all possible), but which would normally get frozen out a week or so ago.
If the approach in #56, fixed for code style and against HEAD obviously, would be committable in D6, then that'd be a good thing - but I'd like to hear that before I ask favours or crack at it again myself.
Also how much time is there for this? Until beta?
The date code - i.e. new messages being set as old on viewing any page in a thread, I think that's a different issue to this - it's about the persistence of the anchors themselves in comment.module, rather than the links to them frpm forum/tracker. So it could easily be a seperate issue (and patch).
#72
Per #64, we could write a menu callback function in the comment module e.g. comment/6 that applies the logic of #56 to call the correct link eg. /node-4-forum?page=1#comment-6.
Of course that would mean rewriting all comment links to use the callback for it to work properly, which wouldn't be a big deal, but I'm not sure it would get committed now.
I agree with Catch that the "new" forum topic thing is a separate issue. Which i've opened at the following link: http://drupal.org/node/158676
#73
typo fix.
bennybobw - that'd be useful for other things as well, but I don't think it'll get into D6, so better thought about for D7.
#74
The underlying issue here is that Drupal doesn't have a working system for comment permalinks. This has been a problem for a long time.
I'd be happy to work on this, but would need to hear some feedback on what people think the best solution is.
#75
There's a start on the underlying issues here: http://drupal.org/node/152417
I very much doubt any major refactoring of comment.module would get into drupal 6 at this stage.
#76
Well, comment.page.link_1.patch works perfect on 4.7 , but it seems no one of suggested patched works on 5.1 , isn't it? Help me please fix it on 5.1.
#77
This may not get into 6.x, let alone 5.x. The patch I posted above works on 5.1 though, so you could just apply that if you don't mind hacking core. It might even work with an small module and theme function, but I've not got to that yet - more interested in fixing it properly for later.
I will try to re-roll against HEAD this week if at all possible to get this to 'needs review'.
#78
hi all,
tenrapid's solution #27 seems to be for 4.7 and i needed to fix this bug in 5.x-dev.
catch's patch in #56 has given an error in when patching tracker.module. i manually made changes, but as an overall result nothing unfortunately changed.
could anyone successfully solve this headache issue using any method for 5.2? can you share how to exactly do this?
#79
In case someone needs this, I rerolled the patch from #27 against DRUPAL 5.2 :)
#80
tenrapid, i did try your last patch for 5.2. and unfortunately it did not work for me :( have you successfully tried your patch for 5.2?
#81
Yes, I did. What exactly did not work for you?
#82
#new link does not lead to the last page, but to the first. i use special block created by views module, but i made to sure to go to /tracker page and it gives same result - tracker's #new link still doesn't recognize multiple pages. can you take a look if i send you a link?
#83
trying different ways and having not succeeded to make a pager's #new link to go to last page, i have come to completely different concept of solving this issue.
instead of modifying several different modules, why don't we do it much simpler:
to set "Date - newest first" in admin/content/comment/settings and make the first page always be the last page. thus we do not need to modify tracker module, since #new link always goes to the first page, which is in our case a last page, actually.
but then we have a problem with wrong listing of the comments, right? isn't it possible by theming or somehow otherwise to make old messages on top of new messages? we need to set "Date - newest first" just for Drupal work in its usual way and get last page to first. we only need reverse the listing of the comments on the generated pages. i guess it would be also easier to reverse of the pager.
as i said it is just a concept. unfortunately i am not a good coder to make it work. i've set "Date - newest first" (which eliminates problem with #new link) and did go to comment.module and changes DESC to ASC, but unfortunately it makes the whole system to work as in the very beginning - just as if i set "Date - oldest first". and i only need to reverse order of the comments in separate pages, not in along the thread.
idea from me, but could anyone make it work and share the code, please.
and i strongly believe that this issue in general should be solved in this way. just not to intervene to lot's of modules codes.
#84
snegny - views module handles this with it's own function which inherits the problems from forum/tracker - so you can't test this patch with it.
This is a quick hack for that:
In the file modules/views/modules/views_comments.inc change the function views_handler_comments_with_new to the following
function views_handler_comments_with_new($fieldinfo, $fielddata, $value, $data) {
$comments = intval($value);
if ($comments && $new = comment_num_new($data->nid)) {
$comments_per_page = _comment_get_display_setting('comments_per_page');
$pagenum = NULL;
$pageno = ($node->comment_count - $new) / $comments_per_page;
if ($pageno >= 1) $pagenum = "page=" . intval($pageno);
$comments .= '<br />';
$comments .= l(t('@num new', array('@num' => $new)), "node/$data->nid", NULL, $pagenum, 'new');
}
return $comments;
}
#85
catch, i am in 5.x.dev and just applied your code against views_comment.inc. unfortunately, #new links still do not recognize multipaged threads. was i supposed to apply this together with #79 or separately? i did separately without any other changes.
what about my post above? now i have this kind of listing in my forums:
#new links always go to 1 st page
1st page 2nd page 3rd page
-------------------------------
1 4 7
2 5 8
3 6 9
we can easily solve the problem with #link not going to last page by setting comments as "Date - newest first" and it becomes:
1st page 2nd page 3rd page
-------------------------------
9 6 3
8 5 2
7 4 1
And the whole problem could be solved if we could make it look like:
1st page 2nd page 3rd page
-------------------------------
7 4 1
8 5 2
9 6 3
i only need to reverse order of the generated pages, not the whole thread. please anyone if you know share your idea how to do that?
#86
catch, i use pathauto to generate custom urls. could it be why your code doesn't work with my views_comment?
#87
@snegny - try looking at the instructions here: http://libcom.org/forums/feedback/jump-straight-to-last-post?page=3#comm...
That ought to work - it does for me. My comment settings are flat, expanded, most recent last.
Changing your comment settings then theming them to undo it isn't going help much I don't think.
#88
OK.
Following Gabor's comments in #70, I've re-rolled the patch at #56 against HEAD. Reviews welcome!
What this does is work out the last page of comments based on comment_get_display_setting then change the 'new' url in tracker and forum module to include the relevant page number.
#89
how about a patch!
#90
reply to #87
catch, thank you for the link. i carefully changed all three of the files. unfortunately, neither tracker nor views trackers work. might the reason be that i am using one of the 5.2.dev versions of Drupal? Namely, changelog shows this:
// $Id: CHANGELOG.txt,v 1.173.2.7 2007/07/12 07:54:44 drumm Exp $
i would gladly compensate if someone would make this work for my site. i really need to solve this asap.
#91
vow, i was wrong. at least in one respect. #new link work for forum.module file (in the function theme_forum_topic_list). but not yet for tracker and views.
#92
sorry for haste. tracker also works. in views' modification $pagenum returns no result. i guess something wrong with this part:
if ($comments && $new = comment_num_new($data->nid)) {$comments_per_page = _comment_get_display_setting('comments_per_page');
$pagenum = NULL;
$pageno = ($node->comment_count - $new) / $comments_per_page;
if ($pageno >= 1) $pagenum = "page=" . intval($pageno);
$comments .= '<br />';
#93
Hi snegny.
I'm the guy who wrote the code that catch have been posting.
There does seem to be an error in patch for the views module. My copy of the patch says:
$pageno = (intval($value) - $new) / $comments_per_page;instead of
$pageno = ($node->comment_count - $new) / $comments_per_page;Hope that helps.
#94
Also, the code for the views module posted above is what I wrote for libcom.org, so it only works for flat comments/oldest first. If you have already installed my patch for the forum and tracker modules, you can use the following instead, and it will also work on threaded comments and newest first:
function views_handler_comments_with_new($fieldinfo, $fielddata, $value, $data) {$comments = $value;
if ($comments && $new = comment_num_new($data->nid)) {
$comments .= '<br />';
$comments .= l(t('@num new', array('@num' => $new)), "node/$data->nid", NULL, comment_new_page_count(intval($value), $new, $data->nid), 'new');
}
return $comments;
}
#95
hi Felix,
#94 gave an error message, but #93 WORKED! i waited so much for this day and tried so many things to make it work. i guess you code can easily go to next version. to save so many drupal users from frustration. thank you Felix!
#96
this thread started in 2004. great job guys, finally there is a solution.
now i would like to ask another question, for which probably i need to open new thread, but i though that if felix solved the problem with #new links for multipage comments, he might have an answer to this issue too.
how to get rid of commenting field on all the pages but last?
#97
snegny - please open a new issue for that question.
For those following along at home, the patch on #83 is the one against 6.x that needs review.
#98
in http://drupal.org/node/169964 i opened the following issue, but nobody reacts. so posting here since it is kind of related to this thread.
#99