WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#16655 closed defect (bug) (fixed)

Media Library assign popup capturing incorrect checked radio elements

Reported by: pmenard Owned by: azaozz
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.1
Component: Media Keywords: has-patch dev-feedback
Focuses: Cc:

Description

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.

Attachments (2)

16655.diff (491 bytes) - added by solarissmoke 3 years ago.
16655.patch (626 bytes) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 pmenard3 years ago

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() });

comment:2 follow-up: solarissmoke3 years ago

  • Component changed from General to Media
  • Keywords has-patch added

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

solarissmoke3 years ago

comment:3 in reply to: ↑ 2 ; follow-up: PeterUpfold2 years ago

  • Keywords dev-feedback added

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?

comment:4 in reply to: ↑ 3 helenyhou2 years ago

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.

comment:5 chrisvanpatten2 years ago

  • Cc chris@… added

comment:6 PeterUpfold2 years ago

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.

SergeyBiryukov2 years ago

comment:7 follow-up: SergeyBiryukov2 years ago

  • Milestone changed from Awaiting Review to 3.4

16655.diff still applies cleanly and fixes the issue.

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

comment:8 azaozz2 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In [20338]:

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

comment:9 in reply to: ↑ 7 azaozz2 years ago

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).

Note: See TracTickets for help on using tickets.