Skip to:
Content

bbPress.org

Opened 10 years ago

Closed 8 years ago

#2757 closed defect (bug) (fixed)

Invalid Markup When Reply Threading Enabled

Reported by: asu666's profile asu666 Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 2.6 Priority: normal
Severity: normal Version: 2.5.4
Component: Component - Replies Keywords:
Cc: robkk17@…

Description

When reply threading enabled, on templates/default/bbpress/loop-replies.php, bbp_list_replies() generate <li> elements without <ul> element, but the parent is an <li> element.

I think, it should be something like this.

	<li class="bbp-body">

		<?php if ( bbp_thread_replies() ) : ?>

			<ul>
				<?php bbp_list_replies(); ?>
			</ul>
		
		<?php else : ?>

			<?php while ( bbp_replies() ) : bbp_the_reply(); ?>

				<?php bbp_get_template_part( 'loop', 'single-reply' ); ?>

			<?php endwhile; ?>

		<?php endif; ?>

	</li><!-- .bbp-body -->

Attachments (2)

2757-2013-threaded-replies-disabled.html (20.6 KB) - added by netweb 9 years ago.
2757.patch (1.2 KB) - added by Robkk 9 years ago.
Using a div instead of an unordered list as the style for the walker fixes this issue for some reason.

Download all attachments as: .zip

Change History (11)

#1 @netweb
9 years ago

  • Milestone changed from Awaiting Review to Under Consideration

#2830 was marked as a duplicate.

#2 @netweb
9 years ago

Possibly related to the following error when trying to create a reply to another reply, aka "threaded reply"

bbPress 2.5.7 (2.5 branch)

Uncaught TypeError: Failed to execute 'insertBefore' on 'Node': 2 arguments required, but only 1 present.addReply.moveForm @ reply.js?ver=2.5.7-5693:18onclick @ ?bbp_reply_to=172&_wpnonce=990d3f1501:335

bbPress 2.6-alpha r5812 /build

Uncaught TypeError: Failed to execute 'insertBefore' on 'Node': 2 arguments required, but only 1 present.addReply.moveForm @ reply.min.js?ver=2.6-alpha-5566:2onclick @ ?bbp_reply_to=172&_wpnonce=34492a5af4:201
Last edited 9 years ago by netweb (previous) (diff)

#3 @netweb
9 years ago

In 2757-2013-threaded-replies-disabled.html is using the 2013 theme without threaded replies enabled and all HTML elements are correctly opened and closed.

#4 @Robkk
9 years ago

  • Cc robkk17@… added

@Robkk
9 years ago

Using a div instead of an unordered list as the style for the walker fixes this issue for some reason.

#5 @Robkk
9 years ago

@netweb

The error code you posted here seems to be with another issue with the reply form not showing below the reply the user is trying to reply to when immediately pressing the reply admin link, much like threaded comments.

Last edited 9 years ago by Robkk (previous) (diff)

#6 @netweb
8 years ago

  • Milestone Under Consideration deleted
  • Resolution set to worksforme
  • Status changed from new to closed

I've just dug around trying to reproduce this and I can no longer reproduce it.

Here's a screenshot of the code https://cloudup.com/cgNGg-RgBse (Source: https://cloudup.com/cfQe1TXHuUL)

The highlighted line #76 contains the parent level <ul> opening unordered list element for threaded replies:

  • <ul class="bbp-threaded-replies">

The source of that <ul> element is from bbp_list_replies() where this is parsed into BBP_Walker_Reply

FYI: If this can be reproduced, please include the HTML code demonstrating this and include details of theme being used and/or custom templates.

#7 follow-up: @Robkk
8 years ago

The issue I stated in my old duplicate ticket is still there even in the code you just posted @netweb .

https://bbpress.trac.wordpress.org/ticket/2830

My quick patch fixes that specific issue I was facing, the issue was causing responsive styles not to work correctly.

The initial tickets issue stated by the author is fine though.

Version 0, edited 8 years ago by Robkk (next)

#8 in reply to: ↑ 7 @netweb
8 years ago

  • Milestone set to 2.6
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Replying to Robkk:

The issue I stated in my old duplicate ticket is still there even in the code you just posted @netweb .

Right you are Rob, thanks for pointing out exactly where that is, line 25 here https://cloudup.com/cgNGg-RgBse shows the <li class="bbp-body"> immediately closed on line 26 with </li>

#9 @johnjamesjacoby
8 years ago

  • Owner set to johnjamesjacoby
  • Resolution set to fixed
  • Status changed from reopened to closed

In 6317:

Replies: Ensure hierarchical replies are correctly wrapped & concatenated into their output buffers in the correct order.

This updates the walker class to not immediately echo it's contents, and wraps it in an ul element.

Fixes #2757. See #2830.

Note: See TracTickets for help on using tickets.