WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9935 closed defect (bug) (fixed)

Quick editor for comments converts encoded HTML entities back into plain form

Reported by: michaeltyson Owned by:
Milestone: 2.9 Priority: normal
Severity: minor Version: 2.7.1
Component: Comments Keywords: has-patch needs-testing early comments quick edit html entities encode htmlspecialchars
Focuses: Cc:

Description

If there are any encoded HTML entities in a comment, such as >, <, etc, these are evaluated and converted into their original characters in the Quick Edit view. For example, if a commenter enters:

<Files wp-config.php>
..
</Files>

This will appear as:

<Files wp-config.php>
...
</Files>

Under 'Quick Edit'. This means that a subsequent save will store the original unescaped characters.

Note that this does not occur in the full comment editor - HTML entities are kept as-is, correctly.

Two in-use plugins that may be relevant: Markdown for Wordpress, and Peter's Literal Comments.

Tested under Safari and Firefox

Attachments (1)

9935.patch (820 bytes) - added by hakre 10 years ago.
First Patch

Download all attachments as: .zip

Change History (17)

#1 @Denis-de-Bernardy
10 years ago

  • Component changed from Comments to Administration
  • Keywords reporter-feedback added
  • Milestone changed from Unassigned to 2.8

Markdown is a formatting plugin. Please double check that the bug exists after disabling it.

#2 @joostdevalk
10 years ago

  • Resolution set to worksforme
  • Status changed from new to closed

Tested in latest SVN and I can NOT reproduce this issue.

#3 @demetris
10 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I can reproduce this on current trunk without any plugins active, exactly as michaeltyson described:

Quick Edit displays the characters unescaped.

#4 @hakre
10 years ago

Confirm as well for trunk. Only in Quickedit. If Quickedit is _not_ saved then normal comment editor will still display the entities.

I assume this is related to the ajax request of the quickedit.

#5 @Denis-de-Bernardy
10 years ago

  • Component changed from Administration to Quick Edit
  • Keywords needs-patch added; reporter-feedback removed
  • Severity changed from normal to minor

#6 @hakre
10 years ago

  • Component changed from Quick Edit to Comments

I could take a deeper look into this Issue. It has been show that my last assumption that it is QuickEdit based was wrong. QuickEdit used markup that is provided /wp-admin/edit-comment.php. The markup is the output result of a function called _wp_comment_row() that is located in /wp-admin/includes/template.php. That function then re-uses a frontend (!) output function, namely comment_text from /wp-includes/comments. This function does not properly output the data from database for HTML output.

A quick cross-check with the frontend output does reveal that the comment itself isn't properly display on the blog as well (same kind of problem as in the backend).

Therefore the first thing to do is to fix the root. This is:

function comment_text from file /wp-includes/comments.

The funny thing is: This is a kind of multi filter. You have got multiple filters that all do the same now. Here is a little structure displayed:

comment_text()
echo apply_filters('comment_text', get_comment_text() );
get_comment_text()
return apply_filters('get_comment_text', $comment->comment_content);
$comment
global $comment

so therefore there can be an invalid filter applied per default to either comment_text-filter or get_comment_text-filter or the default value of the global variable $comment->comment_content is just plain wrong which would show that there is a problem with fetching the database data.

or instead of the template function to use comment_text it must use get_comment_text and HTML-Encode the return value prior to displaying.

The scenario with $comment->comment_content should not be considere much because the database contains the unencoded text and the html-output needs it properly encoded. That is not located in the global variable.

Trying to find out more now. I'm currently on the Issue.

#7 @hakre
10 years ago

Filters (Backend)

comment_text-Filter (simplified)

9: make_clickable - make_clickable(1 arg)
10: wptexturize -  wptexturize(1 arg)
10: convert_chars - convert_chars(1 arg)
20: convert_smilies -  convert_smilies(1 arg)
25: force_balance_tags - force_balance_tags(1 arg)
30: wpautop - wpautop(1 arg)

get_comment_text-Filter

null

If you see the list of applied filters it is perfectly clear, why this bug is in. it is by implementation. It is totally unforseen to properly escape ampersands.

This is not OK for both, the frontend and backend. For the frontend it is not OK because it prevents to display ampersands well in comments. And in the backend it prevents to display and to edit such comments.

Because the standard comment editor does not have the problem that the comment-list and quickedit have, the template function can be adopted to the standard comment editor in wp-admin/comment.php ($comment = get_comment_to_edit( $comment_id );) resp. wp-admin/edit-form-comment.php (<?php the_editor($comment->comment_content, 'content', 'newcomment_author_url', false, 4); ?>) (the the_editor_content-filter might apply which was null).

Considering the content editor working perfectly right means that the comment list in the backend should use the comment data as in comment.php.

For the bug on the frontend the one who wrote the output implementation in the template include should take of that. I would rate it "defect by design" because it does not handle ampersands well. The cause can be somewhere else as well, as shown, the comment_text() function does make use of a lot of filters. just guessed, maybe convert_chars() is unable to handle that?

Whatever. I could figure out that wp-action/comment.php does it right with a plain output of $comment->comment_content but this does not do the job for the list. The standard editor has encoded ampersands (with or w/o javascript) therefore there must be another processing on the data i have not found so far. I need to dig into that further first.

#8 @hakre
10 years ago

function get_comment_to_edit() in wp-admin/includes/comment.php does the needed magic. so this bug is two faced:

we have a working function-set that was intended for editing but displaying is buggy as well.

a first patch is to offer the correct editing. but this is only a workaround and not a fix.

to fix this bug someone who is responsible for the blogs output system must take care of. imho it is defect by design.

@hakre
10 years ago

First Patch

#9 @hakre
10 years ago

  • Keywords has-patch added; needs-patch removed

First patch attached. This is a workaround for the bogus behavior in the backend using the same function-set that the standard comment editor is using. It uses it for both: display and quickedit.

This is a workaround only. Frontend is not touched. Frontend would at least need another default filter for comment_text. If that is patched, then the first patch must be looked into again because it still uses comment_text() to display the comment in function _wp_comment_row() in wp-admin/includes/template.php .

#10 @Denis-de-Bernardy
10 years ago

  • Keywords needs-testing added

haven't tested, but probably commit-worthy

#11 @ryan
10 years ago

Seems like a good patch, but if this isn't a regression in 2.8 I'm tempted to postpone to early 2.9.

#12 @azaozz
10 years ago

  • Milestone changed from 2.8 to 2.9

Adding htmlspecialchars($comment->comment_content, ENT_QUOTES); for the hidden textarea holding the content for Quick Edit seems to properly double-encode entities but as Ryan said, perhaps better to leave for early 2.9 as it might break something else (not enough time to test it properly).

comment_text()/get_comment_text() will need fixing. Looks like they will need improved context filtering as the comment text is used in several different contexts: display, feeds, email...

#13 @Denis-de-Bernardy
10 years ago

  • Keywords early added
  • Milestone changed from 2.9 to 2.8.1

#14 @hakre
10 years ago

We need a list of contexts then. Would be interesting to get that.

#15 @azaozz
10 years ago

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

(In [11711]) Fix converting of HTML entities in Quick editor for comments, don't output QE data when the user cannot edit comments, fixes #9935

#16 @azaozz
10 years ago

  • Milestone changed from 2.8.2 to 2.9
Note: See TracTickets for help on using tickets.