wp_ajax_send_attachment_to_editor() bug

[otterstein]

there is a problem at wp_ajax_send_attachment_to_editor()
in the file ajax-actions.php in wp-admin/includes/
on line 2605

the link to editor comes with <a href="mediafile"">mediafile</a>

i fixed the file with:
Old one

$html = isset( $attachment['post_title'] ) ? $attachment['post_title'] : '';
                $rel = $rel ? ' rel="attachment wp-att-' . $id . '"' : ''; // Hard-coded string, $id is already sanitized

                if ( ! empty( $url ) ) {
                        $html = '<a href="' . esc_url( $url ) . '"' . $rel . '">' . $html . '</a>';
                }

into new one

$rel = $rel ? ' rel="attachment wp-att-' . $id . '"' : ''; // Hard-coded string, $id is already sanitized

                if ( ! empty( $url ) ) {
                        $html = '<a href="' . esc_url( $url ) . '"' . $rel . '>' . $html . '</a>';
                }

the " before the '>' . $html was a doublicate

[joemcgill]

Hi @otterstein,

Great catch. Thanks for taking the time to track down the solution as well.

This was introduced in [37035] and is fixed by 36578.diff​.

[azaozz]

Ah I see. There is a typo in the tag. Seems TinyMCE is correcting it so nobody noticed until now?

Thinking this is minor but should be considered for 4.5.1 as well.

Wondering why the tests didn't catch that... :)

[swissspidy]

Replying to azaozz:

Ah I see. There is a typo in the tag. Seems TinyMCE is correcting it so nobody noticed until now?

Thinking this is minor but should be considered for 4.5.1 as well.

Wondering why the tests didn't catch that... :)

That's because the tests are not for the ajax handler, where the bug resides :)

[swissspidy]

Patch including ajax tests to ensure the bug is fixed

[swissspidy]

As asked by @ocean90, I wrote some ajax tests in 36578.2.diff​. Not sure if we'd backport these to 4.5 as well, but we can certainly use them for trunk.

[swissspidy]

Note: Some comments ajax tests are currently failing with 36578.2.diff​ applied, which #36616 fixes. Looking into it.

[boonebgorges]

The failures referenced by @swissspidy are pretty baffling to sort out. Briefly, the generation of user fixtures during ajax tests is happening in such a way that the WP_UnitTest_Generator_Sequence incrementor - which is intended to ensure uniqueness for user_login etc - is being incremented inconsistently or in an inconsistent order. I think that this has something to do with the fact that some AJAX tests are run in separate processes, along with some weirdness having to do with the way that PHPUnit loads setUpBeforeClass() early in the process of running tests.

In any case, a tweak to the way that the fixture incrementor works appears to resolve the problem. It also solves a developer-facing annoyance, discussed in #35199, which is that a generated user might have user_login "user_5" but user_email "user_6@…". https://core.trac.wordpress.org/attachment/ticket/35199/35199.diff should fix all of this.

[swissspidy]

Thanks a bunch for investigating, @boonebgorges! An improved incrementor in trunk will make the tests more robust.

Since the tests do not pass with 36578.2.diff​ applied, should we

a) only commit 36578.diff​ (without the tests) for 4.5.1 and 36578.2.diff​ for trunk, or
b) move #36616 to the 4.5.1 milestone and implement these together?

[ocean90]

#36626 was marked as a duplicate.

[boonebgorges]

Thinking this over, I think we should be conservative and not rush #35519 for 4.5.1. We can work around the current problem by explicitly providing unique user_login and user_email for the admin user required in these new tests. See 36578.3.diff​.

[swissspidy]

@boonebgorges Thanks for sharing. That actually makes the most sense!

[ocean90]

In 37288:

Media: Remove an extra quote when sending a link of a media file to the editor.

Introduced in [37035].

Props joemcgill, swissspidy, boonebgorges.
Fixes #36578.

[ocean90]

In 37289:

Media: Remove an extra quote when sending a link of a media file to the editor.

Introduced in [37035].

Merge of [37288] to the 4.5 branch.

Props joemcgill, swissspidy, boonebgorges.
Fixes #36578.

[swissspidy]

#36632 was marked as a duplicate.