Opened 4 years ago

Last modified 3 weeks ago

#10948 new defect (bug)

wp_list_comments() always assumes walker will echo.

Reported by: MikeLittle Owned by:
Priority: normal Milestone: 3.6
Component: Comments Version: 2.8.6
Severity: normal Keywords: has-patch
Cc: wordpress@…, shidouhikari

Description

wp_list_comments() always assumes the walker called will echo it's constructed content (the default Walker does this).

However, if you want to use your own walker which doesn't echo, or you want to post process the output of the default walker in some way, you cannot.

The changes required are to wp_list_comments() which should check for an 'echo' arg and respond appropriately (like wp_list_pages does). The built in comment walker class Walker_Comment needs to write it's output to the passed $output reference. Finally the function edit_comment_link() needs to take an echo parameter.

The attached patch allows this functionality whilst preserving the status quo by adding a default echo => 1 argument to wp_list_comments and edit_comment_link.

Attachments (6)

comment-walker-echo-issue-trunk-12022.diff (5.5 KB) - added by MikeLittle 4 years ago.
Patch to fix issue. Diff-ed against trunk ar rev 12022
comment-walker-echo-issue-284.diff (5.5 KB) - added by MikeLittle 4 years ago.
Patch to fix issue in version 2.8.4. Diff-ed against tag 2.8.4
comment-walker-echo-issue-trunk-12304.diff (5.6 KB) - added by MikeLittle 3 years ago.
Patch to fix issue. Diff-ed against trunk at rev 12304
comment-walker-echo-issue-286.diff (5.5 KB) - added by MikeLittle 3 years ago.
Patch to fix issue in version 2.8.6. Diff-ed against tag 2.8.6
return-wp-list-comments.diff (1.3 KB) - added by wonderboymusic 9 months ago.
10948.diff (860 bytes) - added by DrewAPicture 3 weeks ago.
Refreshes return-wp-list-comments.diff

Download all attachments as: .zip

Change History (20)

Patch to fix issue. Diff-ed against trunk ar rev 12022

Patch to fix issue in version 2.8.4. Diff-ed against tag 2.8.4

  • Keywords has-patch added; has_patch removed

I should note this has been tested against both 2.8.4 and trunk. The output of various comments displays (default theme, with and without paging) was compared before and after the patch.

Apart from white space, the outputs were identical.

Patch to fix issue. Diff-ed against trunk at rev 12304

Patch to fix issue in version 2.8.6. Diff-ed against tag 2.8.6

  • Cc wordpress@… added
  • Version changed from 2.8.4 to 2.8.6

Added updated patches - against 2.8.6 and against trunk at rev 12304.
It would be great to get this into 2.9 if at all possible.

Looks good, applies clean, appears to work correctly. It looks like all filters are applied as before as well.

FWIW, you only need to patch this against trunk. Only security changes are backported to the 2.8 branch.

comment:4   ryan3 years ago

  • Keywords early added
  • Milestone changed from 2.9 to 3.0

We might want to do get_edit_comment_link() instead of an echo flag to match what we do elsewhere. Otherwise, looks good. Unfortunately, we overlooked this ticket in time to get it into 2.9. Let's mark as early 3.0 and get this in as soon was we branch.

Now that I look, there is get_edit_comment_link(), which returns the URL only, so they function slightly differently. edit_comment_link() calls get_...(). I can think of a few different solutions but I'm not sure which would be best.

comment:6   dd323 years ago

  • Milestone changed from 3.0 to 3.1

This has missed the boat for 3.0 IMO, I'm marking as 3.1 early instead.

  • Cc shidouhikari added
  • Keywords needs-patch added; has-patch early removed
  • Milestone changed from Awaiting Triage to Future Release
  • Keywords has-patch added; needs-patch removed

Added my own patch - it's less intrusive (IMO) to use output buffering

That patch seems to work OK (as you would expect), but I thought there were sometimes issues with using output buffering. This may be historical (old PHP on old Windows) and irrelevant.

Last edited 9 months ago by MikeLittle (previous) (diff)
  • Milestone changed from Future Release to 3.6

Gauging interest in this

The return-wp-list-comments.diff​ patch still applies cleanly for me on today's trunk (revision #23296).

The code works fine:
Adding an 'echo' => false parameter to the call to wp_list_comments in comments.php in twentytwelve theme produces no comment output (so the flag works).
Adding an echo to the front of that function call produces exactly the same output as without the patch.

Last edited 4 months ago by MikeLittle (previous) (diff)

Refreshes return-wp-list-comments.diff

10948.diff refreshes return-wp-list-comments.diff following [23694].

wp_list_comments() continues to work for me as expected with 10948.diff applied.

Note: See TracTickets for help on using tickets.