1) Create a node with a table with sortable columns.
2) Enable comments on that node, and comment threading.
3) Create new comment 'A'.
4) Create new comment 'B'.
5) Reply to comment 'A' with new comment 'C'.
6) View the node: comment order is A C B.
7) Click a table column: comment order is now A B C.
comment_render() gets its default sort order from _comment_get_display_setting() which, in turn, is happy to return the value of $_GET['sort']. $_GET['sort'], however, is also used by tables to determine which direction to sort a column on. If you sort by a table column, the 'sort' will get set to 'desc' or 'asc', and _comment_get_display_setting() will happily return that. Since 'desc' doesn't match any of the COMMENT_ORDER constants, no ORDER BY is set for the $query, which causes the query to return comments by database insertion date (I suspect) and not in the user's preferred order. Solution: either namespace the GETs (comment_sort, etc) or _comment_get_display_setting() should check that the value of the $_GET is valid against any of the COMMENT_ constants.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | comments_order_tablesort.patch | 2.22 KB | alan evans |
Comments
Comment #1
morbus iffThis appears to be a very old legacy issue. Comment controls used $_GET parameters waaaay back: "Fri May 30 11:13:53 2003 UTC (4 years ago)" for this particular bugfix. Links that actually used GET seem to have disappeared about a year later. So, we're already at a point where the _GET[$setting] of _comment_get_display_setting() is wrong. However, even if it was still in use, the addition of "sort" was an error - consider this patch which refactors it all. In the original issue, tedrapid rolls a new patch with the comment "Also Jaza forgot the sort column so I added it." In fact, Jaza didn't "forget it" - it was never there in the first place.
Comment #2
alan evans commentedI've checked through all instances of "sort", "mode" and "comments_per_page" in the codebase and I'm confident that none of these would ever be set in $_GET in the context of sorting comments (the comment_controls form uses the default "post" method). So I'm attaching a patch that removes the consideration of $_GET['sort'] etc. from comment sorting.
However, if anyone knows any situation where these $_GET params might be used to sort comments, please speak up ...
Comment #3
catchNo longer applies.
See also this alternative approach which would indirectly fix the issue ;) http://drupal.org/node/175841
Comment #4
dpearcefl commentedIs this still a problem in current D6?
Comment #5
dpearcefl commented