WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 5 years ago

#23797 closed enhancement (fixed)

Hard coded HTML marking in comment_form()

Reported by: Marie-Aude Owned by: wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.5.1
Component: Comments Keywords: has-patch
Focuses: Cc:

Description

Hello

it seems so ugly hard coding has slipped in the comment form.

On line 1554 of comment-template.php, there is some hard-coded html :

<h3 id="reply-title">
<?php comment_form_title( $args['title_reply'], $args['title_reply_to'] ); ?>
<small><?php cancel_comment_reply_link( $args['cancel_reply_link'] ); ?></small>
</h3>

Semantically speaking, a H3 is far from being optimal.
Also, filters exists for all other elements of the comment form.
The h3 and small should not be hardcoded.

A new value could be included in the defaults :

'comment_reply_markup' => 
    '<h3 id="reply-title">%1$s <small>%2$s</small></h3>',

and line 1554 replaced by :

<?php printf( $args['comment_reply_markup'], 
comment_form_title( $args['title_reply'], 
$args['title_reply_to'] ), cancel_comment_reply_link( $args['cancel_reply_link'] ); ?>

Thanks a lot, hope you'll take it :)

Attachments (6)

23797.diff (1.8 KB) - added by MikeHansenMe 8 years ago.
patch based on comments
23797.refresh.diff (1.8 KB) - added by voldemortensen 6 years ago.
Refresh of 23797.diff
23797.2.diff (1.9 KB) - added by MikeHansenMe 6 years ago.
23797.3.diff (1.7 KB) - added by MikeHansenMe 5 years ago.
23797.4.diff (1.7 KB) - added by MikeHansenMe 5 years ago.
add missing )
23797.5.diff (1.8 KB) - added by MikeHansenMe 5 years ago.

Download all attachments as: .zip

Change History (19)

#1 @toscho
8 years ago

  • Cc info@… added

#2 @GaryJ
8 years ago

If this is going to be addressed, then including a class="reply-title" attribute would mean that themes can use one less ID selector in their style.css.

@MikeHansenMe
8 years ago

patch based on comments

@voldemortensen
6 years ago

Refresh of 23797.diff

#3 @voldemortensen
6 years ago

  • Keywords has-patch added

@MikeHansenMe
6 years ago

#4 @rachelbaker
5 years ago

  • Milestone changed from Awaiting Review to Future Release

Can we also keep the comment-reply-title class added in 24525 to the h3 element? I don't see any reason for removing the id attribute and changing the class attribute. Before we remove the id attribute from the h3 we should check and see how many themes are targeting it for styling.

I would also suggest changing comment_reply_markup to be something more appropriate, perhaps comment_reply_title?

I am +1 on the overall intent.

#5 @rachelbaker
5 years ago

  • Keywords needs-refresh added

@MikeHansenMe
5 years ago

#6 @MikeHansenMe
5 years ago

  • Keywords needs-refresh removed

23797.3.diff is a refresh with @rachelbaker's suggestions.

@MikeHansenMe
5 years ago

add missing )

#7 @johnbillion
5 years ago

  • Milestone changed from Future Release to 4.4
  • Owner set to johnbillion
  • Status changed from new to reviewing

#8 @johnbillion
5 years ago

  • Keywords needs-docs added

#9 @MikeHansenMe
5 years ago

  • Keywords close added

This seems to have been fixed in [34308] in a slightly different way but this ticket is no longer needed I believe.

#10 @rachelbaker
5 years ago

The small tag is still hard-coded after the changes in #33775. I recommend keeping this ticket open to remove all the hard-coded markup.

@MikeHansenMe
5 years ago

#11 @MikeHansenMe
5 years ago

  • Keywords needs-docs close removed

23797.5.diff adds cancel_reply_(before|after) that can be altered the same was as title_reply_before.

#12 @wonderboymusic
5 years ago

  • Owner changed from johnbillion to wonderboymusic
  • Status changed from reviewing to accepted

#13 @wonderboymusic
5 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 34523:

Comments: in comment_form(), add args for cancel_reply_before and cancel_reply_after to eradicate remaining hard-coded HTML bits.

Props MikeHansenMe.
Fixes #23797.

Note: See TracTickets for help on using tickets.