[wp-trac] [WordPress Trac] #34073: Comment walker calls get_comment_link with invalid arguments
WordPress Trac
noreply at wordpress.org
Wed Sep 30 04:25:40 UTC 2015
#34073: Comment walker calls get_comment_link with invalid arguments
-------------------------------------+--------------------
Reporter: peterwilsoncc | Owner:
Type: defect (bug) | Status: new
Priority: normal | Milestone: 4.4
Component: Comments | Version: trunk
Severity: normal | Resolution:
Keywords: has-patch needs-testing | Focuses:
-------------------------------------+--------------------
Changes (by boonebgorges):
* keywords: => has-patch needs-testing
Comment:
[attachment:34073.diff] is a pass at fixing this ticket as well as #34068.
(They're slightly different, but the same technique has to be used in both
places.)
The trick in the case of `wp_list_comments()` was (a) to detect that we
were inside of a `comments_template()` loop (something we already know
because of the `$wp_query->max_num_comment_pages` check), and if so, (b)
pass the new `cpage` parameter down through the stack, all the way to
`get_comment_link()`. When `get_comment_link()` sees `cpage`, it trusts it
- it doesn't do any calculation at all.
As for `get_comment_link()` when called outside the context of a comment
loop (the problem in #34068), the logic is much the same - calculate total
pages, and then remove the 'cpage' query arg when looking at the
newest/oldest page, depending on your settings - but it had to go into the
calculation logic of the function itself. This meant reorganizing the
function quite a bit, mainly for readability, but also to break apart some
hairy if/else chains.
My eyes are nearly bleeding from writing tests and doing manual tests of
the approach in the patch. I feel pretty confident about it, but a second
(and third, and fourth) opinion would be swell.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/34073#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list