Set filter "activate_tinymce_for_media_description" to "true" is breaking meadia_descripton by running it through "htmlspecialchars()"

[dagobert24]

Adding filter activate_tinymce_for_media_description was great but it creates an issue.

Bug

When activating it see here

<?php
        $editor_args        = array(
                'textarea_name' => 'content',
                'textarea_rows' => 5,
                'media_buttons' => false,
                /**
                 * Filters the TinyMCE argument for the media description field on the attachment details screen.
                 *
                 * @since 6.6.0
                 *
                 * @param bool $tinymce Whether to activate TinyMCE in media description field. Default false.
                 */
                'tinymce'       => apply_filters( 'activate_tinymce_for_media_description', false ),
                'quicktags'     => $quicktags_settings,
        );

The textarea will be processed

<?php
        <?php wp_editor( format_to_edit( $post->post_content ), 'attachment_content', $editor_args ); ?>

The function format_to_edit() here will than execute esc_textarea() here and "convert" all HTML tags using htmlspecialchars().

Solution

The reason why the HTML tags are convertred is that the parameter $rich_text for format_to_edit() is not set to true. Since $editor_args['tinymce'] = true; and format_to_edit() parameter $rich_text is directly dependend it is simple to correct by changing line 3292 from

<?php
        <?php wp_editor( format_to_edit( $post->post_content ), 'attachment_content', $editor_args ); ?>

to

<?php
        <?php wp_editor( format_to_edit( $post->post_content, $editor_args['tinymce'] ), 'attachment_content', $editor_args ); ?>

Additional comment

If someone wants to use tinymce to edit textareas one surely wants to add more than just a few words but rich content such as within a normal post. Hence I propose to change the value

$editor_args['textare_rows'] = 5;

to

$editor_args['textare_rows'] = 20;

This will give the user a nice-sized textarea to enter rich content without having to resize it every time.

[dagobert24]

This bug was introduced in (probably) version 6.6 of WordPress and should be pushed in the next bugfix release of 6.6.

Using tinymce with media_description will convert all HTML tags to a sanitized version. This way tinymce is not usable with media_content since TinyMCE will show the HTML tas insted of interpreting them as styles, eg. <h2> will be show instead of the heading style.

This PR fixes this bug and changes the textarea_rows value if tinymce is used.

Trac ticket: https://core.trac.wordpress.org/ticket/62361

[yogeshbhutkar]

Hi @dagobert24, thank you for raising the ticket and the PR.

I was going through the PR and had a teeny-tiny query about the approach. Here, if we pass the second parameter of the format_to_edit() function true then it would consider the content as rich text and skip the usage of esc_textarea(). I was wondering if this could cause security concerns as the data might not be escaped.

How about using wp_kses_post() to sanitize the content and pass it to wp_editor() directly? That way, the content will be sanitized and will serve the purpose as well.

Would love to hear your thoughts on this.

Thank You.

[dagobert24]

Replying to yogeshbhutkar:

Hi @dagobert24, thank you for raising the ticket and the PR.

I was going through the PR and had a teeny-tiny query about the approach. Here, if we pass the second parameter of the format_to_edit() function true then it would consider the content as rich text and skip the usage of esc_textarea(). I was wondering if this could cause security concerns as the data might not be escaped.

How about using wp_kses_post() to sanitize the content and pass it to wp_editor() directly? That way, the content will be sanitized and will serve the purpose as well.

I guess running wp_kses_post() makes sense. The question is how to implement this in a nice and clean way. Also I am afraid to break something unexpected.

We should continue to use the format_to_edit() function. Within this function is a hook which we would otherwise break or more precisely we would skip this hook if we are not using this function.

I think we should make use of this hook ourselves like this for example (L3280):

if ( true === $editor_args['tinymce'] ) {
	$editor_args['textarea_rows'] = 20;
        add_filter('format_to_edit', function() {
                return wp_kses_post($post->post_content);
        });
}

I am not that deep into wordpress that I could be sure or assigning the correct priority to the hook to make sure it will run at a very high priority but still has "room" for someone to superseed it.

What do you think?

[azaozz]

Frankly I think that the change from #60158 should be reverted. It doesn't seem a good idea to add support for TinyMCE to the content of attachment pages just to have paragraph tags. Seems that missing paragraph tags there should be treated as a bug when using the (so called) Text editor, quicktags. I think this used to work before.

There are few reasons to remove this:

• TinyMCE in WordPress is in "deep maintenance" mode meaning it is not supported any more except for eventual security fixes.
• The attachment pages are not used/shown any more. They are disabled by default on new(er) installs.

Also the fact that this bug is being reported several months after this functionality was introduced seems to signify there is very little interest.

Thinking this type of functionality is best left for plugins that will also ensure attachment pages are used in the theme/at the front-end. Without that this is useless.

Imho instead of fixing this it seems better to revert [58372], make a follow-up ticket for #60158 and look at fixing the Text editor to create paragraphs.

[azaozz]

Replying to dagobert24:

I guess running wp_kses_post() makes sense.

Not so sure about that :) KSES, and wp_kses_post() are really slow and are only intended to sanitize HTML on saving to the database. What would be the reason to run KSES on content retrieved from the database? It is assumed it was run when that content was saved (which seems to be the case here too).

[dagobert24]

Replying to azaozz:

Replying to dagobert24:

I guess running wp_kses_post() makes sense.

Not so sure about that :) KSES, and wp_kses_post() are really slow and are only intended to sanitize HTML on saving to the database. What would be the reason to run KSES on content retrieved from the database? It is assumed it was run when that content was saved (which seems to be the case here too).

Ok, I understand this part. Than the current fix to the bug should be correct because the content retrieved from the database is already sanitized.

[dagobert24]

Replying to azaozz:

Frankly I think that the change from #60158 should be reverted. It doesn't seem a good idea to add support for TinyMCE to the content of attachment pages just to have paragraph tags. Seems that missing paragraph tags there should be treated as a bug when using the (so called) Text editor, quicktags. I think this used to work before.

Thank you for your opinion. I on the other hand don't think it should be reverted. It was something I was waiting for for a long time not having to use plain HTML tags only and hoping to get the style right or constantly saving and looking at the changes on the page itself.
The idea of offering a plaintext textarea only for this made it basically unusable for 95% of all WP users. In addition to the fact that the content is basically a post directly connected to the picture itself.

There are few reasons to remove this:

• TinyMCE in WordPress is in "deep maintenance" mode meaning it is not supported any more except for eventual security fixes.

Ok, I understand but it still in use and one can replace the tinymce by a different editor. Removing the introduced hook will disable using any editor completely (without major hoops by plugin creators maybe).

Actually this would rather lead to arguing for the complete functionallity of the editor-set of the post page. I mean all features including switching between text, classic editor and the new block editor. (Would be great btw.)

• The attachment pages are not used/shown any more. They are disabled by default on new(er) installs.

Yes, this is true. Disabled does not meand removed and it certainly does not mean depricated, does it? There are plenty installs still using this feature, even of cause many others don't know or want to use it. There are many option/feature disabled by default and these defaults only reflect the WP product owners descisions and/or surveys about how WP users normally configure their WP. E.g. most WP users don't want anyone to register hence this option is disabled by default (not removed).

Also the fact that this bug is being reported several months after this functionality was introduced seems to signify there is very little interest.

Sorry, but I have to disagree. This option/functionallity is not exposed to the UI. Noone knows about it. Only if one searches for it you can find it. Many already using the attachment page don't know this. No user updating WP look at the changes.
It took me some efford too, even finding the right search phrase to stumble upon it. Their is no plugin for this either. (I am workiing on one right now and am only waiting that the fix is released.)

Thinking this type of functionality is best left for plugins that will also ensure attachment pages are used in the theme/at the front-end. Without that this is useless.

Anyone turning on this feature will know that the theme needs to support attachment pages. This is specially true because of the introduced disabling of attachment paged by installation.