When posting a new comment to a node with more comments than in comment_default_per_page_nodetype, the user should be redirected to the right page, along with the right anchor for the comment.

For example, a forum thread has 90 replies, and forums are set to 30 comments per page. When a user posts the 91st comment, she should be taken to page 2 (page 3 of the comments), i.e. example.com/node/1234?page=2#comment-8765

The attached patch fixes this issue. With Drupal 6's new ability to have custom comment settings per node type, I could not figure out a way to do this without calling node_load or writing a database query to determine the node's type. I'm thinking writing an individual query for this may be better than calling node_load (the current solution). Please review and look for optimization possibilities.

CommentFileSizeAuthor
#18 comment_newpage.patch3.98 KBsnufkin
#10 comment-redirect_after_post-2_1.patch874 bytescatch
#9 comment-redirect_after_post-2_0.patch682 bytescatch
#6 comment-redirect_after_post-2.patch684 bytesjrbeeman
comment-redirect_after_post.patch919 bytesjrbeeman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jrbeeman’s picture

jrbeeman
he/him
Primary language English
Location Arizona, USA
CreditAttribution: jrbeeman commented
Version: 6.0-beta2 » 6.x-dev

(Changing version to 6.x-dev)

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

No time to review this evening, but a big +1 for this. Subscribing so I can take a look tomorrow.

JirkaRybka’s picture

JirkaRybka CreditAttribution: JirkaRybka commented

This is closely related to http://drupal.org/node/26966 (comment-links in general), and might be probably fixed together with that.

While reviewing (I've no time this moment, unfortunately), please double-check whether this works for both flat AND threaded mode - for threaded the comment may end up anywhere if it's a reply, not only the last page.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Review of this issue is blocked by: http://drupal.org/node/187482

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Needs work

This doesn't work on threaded comments - posting a comment to the first page of a thread redirects you to the last page regardless. Without permalinks for comments (i.e. a way to link to a specific comment whilst retaining context) it's going to be near impossible to fix this issue, so marking as needs work and bumping to D7. Obviously if you come up with an approach which will work for D6, please reset the issue and I'll do my best to review it again.

jrbeeman’s picture

jrbeeman
he/him
Primary language English
Location Arizona, USA
CreditAttribution: jrbeeman commented
Status: Needs work » Needs review
FileSize
comment-redirect_after_post-2.patch684 bytes

New attempt at a patch for this. This patch uses the new comment_new_page_count method, which returns the query and looks like it handles all the threading stuff correctly.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

OK so I just installed this and tested it with a couple of different comment settings, it passed in all cases.

In a related issue Dries pointed out that this doesn't work when comments are in moderation - I think it's worth arguing that the existing implementation doesn't work when comments are in moderation either, so this is a major step forward in usability - allowing people to see their comment once it's posted regardless of paging and threading and bringing the comment system into line with most discussion forums etc. which do the same thing.

I won't mark to RTBC yet in case there are code optimisations, the extra queries are only on comment submit though (and also save people clicking through pagers to see their comment in context afterwards), so that might not be an issue. Very nice work, thanks!

jrbeeman’s picture

jrbeeman
he/him
Primary language English
Location Arizona, USA
CreditAttribution: jrbeeman commented

Great, thanks! Since the new patch uses comment_new_page_count, which requires a node object as a parameter, the node_load is required now. But, like you said, this only occurs upon comment submission, and significantly saves on paging through comments.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Reviewed & tested by the community
FileSize
comment-redirect_after_post-2_0.patch682 bytes

OK so looking at this again, I reckon $query should be renamed to $page. Attached patch modifies this and nothing else. On the assumption there's no other way to do this than the node_load (and the fact it's only a single node_load on form submission, saving a lot of clicks through the pager caused by current behaviour), going to mark this RTBC. A solution for comments in moderation needs to be found, but that's a seperate issue from the one dealt with by this patch, and will have to wait for D7 I think.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
FileSize
comment-redirect_after_post-2_1.patch874 bytes

Or even better rerolled from root with no offset this time.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Title: Posting a comment does not redirect to the correct page for the new comment » Comment submit redirect should respect paging
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

No more hacking comment.module in D6 for me! Thanks!

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

mykle’s picture

mykle CreditAttribution: mykle commented
Version: 6.x-dev » 5.x-dev
Status: Closed (fixed) » Active

still a problem in drupal 5.x ... is a backport practical?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Active » Patch (to be ported)

I don't think this will get committed to 5.x even if backported , but if not it would be a small patch to maintain or might be possible to do it in a module. Changing status anyway.

snufkin’s picture

snufkin CreditAttribution: snufkin commented
Status: Patch (to be ported) » Needs work
FileSize
comment_newpage.patch3.98 KB

Patch implements the comment_new_page_count from D6 and adds the page query part similarly to D6 in comment_form_submit(). It works with threaded commends in oldest first mode, for some reason it doesn't work for the newest first mode, perhaps someone with more insight into the database queries in that branch of the if can use this as a starting point.

I had to add a +1 into the comment count, otherwise the function thinks that we still dont have enough comments. From node load we get the number of comments without our new comment, so the first if branch if ($num_comments <= $comments_per_page will be true for the first comment that should be on the next page.

Its certainly possible to do this in a contrib module, just implement the function that calculates the new pages, override the comment_form_submit to your own, replicating it and adding the paging part.

Max_Headroom’s picture

Max_Headroom CreditAttribution: Max_Headroom commented

Can you please remove the settings.php changes from your patch.

thinkyhead’s picture

thinkyhead CreditAttribution: thinkyhead commented

This set of patches seems to ignore the fact that comments can be edited. At least, the behavior I'm seeing is that even if I edit a comment on the first page of a pager, submitting the comment always takes me to the last page. Can the patch authors confirm that this is a known side-effect?

snufkin’s picture

snufkin CreditAttribution: snufkin commented

Yeah, if i remember correctly after so long, the patch automatically calculates the number of comments and forwards the user regardless if that was an edit or a new comment inserted.

TR’s picture

TR CreditAttribution: TR commented
Status: Needs work » Closed (won't fix)

A fix for this issue was committed to Drupal 6.x in comment #12.

This issue is still open only for a backport of this fix to Drupal 5.x.

Drupal 5.x is no longer supported. Backport won't happen.

JirkaRybka’s picture

JirkaRybka CreditAttribution: JirkaRybka commented
Version: 5.x-dev » 6.x-dev
Status: Closed (won't fix) » Closed (fixed)

In which case, return to the status before #15 is more accurate IMO.