Media Library assign popup capturing incorrect checked radio elements

[pmenard]

Been researching a bug which is effecting my plugin, Media-Tags. Please don't stop reading there. This is not a support question.

When on the Media > Library page and you click 'Attach' link on any unassigned row. You get the search popup titled 'Find Posts or Pages'. You fill in something for the search input. Select the 'Posts' or 'Pages' radio buttons.

Here is the defect:

In wp-admin/includes/js/media.js (line 34 of media.dev.js)

$("input[@name='itemSelect[]']:checked").each(function() { selectedItem = $(this).val() });

This above code loops through every single checked input on the page. Meaning the code finds in order the following checked input fields: author, parent, comments, date then either post or page from the assign popup.

This is wrong in that it is inefficient. Mainly because the 'itemSelect[]' does nothing. But also just by chance it ends at the last checked input field. The problem is when a plugin adds other radio input fields then those are picked up instead of the value of the selected radio on the popup.

The code should be changed to limit the jQuery code to the input radios within popup like

$("input[@name='find-posts-what[]']:checked").each(function() { selectedItem = $(this).val() });

The 'find-post-what' in the selector is obvious since the Posts or Pages form radios are in that group.

This is being reports for WP 3.1 but goes back to previous versions.

[pmenard]

Actually the more correct solution for the JavaScript change would be the following. Note the addition of the 'find-box-search' which is the parent container for the radio buttons.

$(".find-box-search input[@name='itemSelect[]']:checked").each(function() { selectedItem = $(this).val() });

[solarissmoke]

Patch attached. PS the use of @ before attributed selectors in jQuery is deprecated.

[PeterUpfold]

I also ran into this problem recently.

I have tested the patch 16655.diff on WordPress 3.3.1, using Safari 5.1.4 on Mac OS X 10.7.3 as the browser. It solves this bug that occurs when findPosts is invoked on pages where there are other selected radio buttons.

Its functionality on upload.php as the 'Attach' window also works normally after the patch.

(The patch is for media.dev.js, but I also had to apply it manually to media.js, the compiled/minified version that is actually sent to and executed by the browser.)

Would it be helpful if I were to also test this against SVN?

Can anyone advise how I can minify the media.dev.js to media.js file after the patch is applied, so that can be tested too?

[helenyhou]

Replying to PeterUpfold:

Would it be helpful if I were to also test this against SVN?

Can anyone advise how I can minify the media.dev.js to media.js file after the patch is applied, so that can be tested too?

Testing and patches should be done against trunk. To use uncompressed versions, set SCRIPT_DEBUG to true in your wp-config.php.

[PeterUpfold]

I have tested patch 16655.diff against WordPress SVN revision 20235. Tested with WP_DEBUG and SCRIPT_DEBUG on.

Tested in:

• Safari/5.1.4 Mac OS X/10.7.3
• Firefox/11.0 Mac OS X/10.7.3
• Internet Explorer/9.0 Windows/7
• Internet Explorer/8.0 Windows/XP
• Google Chrome/17.0.963.79 Windows/XP
• Chromium/19.0.1076.0 Mac OS X/10.7.3

This patch fixes the issue in my testing. The Attach functionality on upload.php also works normally after the patch.

[SergeyBiryukov]

16655.diff​ still applies cleanly and fixes the issue.

16655.patch​ also gives selectedItem a more descriptive name (selectedPostType).

[azaozz]

In [20338]:

Fix getting the checked radio button when attaching orphan attachments, props solarissmoke SergeyBiryukov, fixes #16655

[azaozz]

Replying to SergeyBiryukov:

The JS there seems very old. No need to loop through the radio buttons just to get which one is checked (jQuery helps for that).