WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#20765 closed defect (bug) (fixed)

get_attachment_fields_to_edit() returns term->name rather than term->slug, causing conflicts between terms

Reported by: eddiemoya Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.3.2
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description

When adding support for taxonomy to attachments, the media page will automatically add a simple text field for entering comma delimited list of terms. This works fine, and appears to handle slugs as well as names entered.

However, once saved and re-opened, the attachment field will output only term names. This leads to conflicts in situations where users have entered two slugs for different terms which share the same name.

This field should be returning slugs or ID's, preferably slugs as they are more human-readable, and unlike the category metaboxes this depends on manual entry.

This bug was found to be the cause of a bug reported on one of my plugins

Attachments (2)

attachment-fields-to-edit.patch (378 bytes) - added by eddiemoya 5 years ago.
Changes term->name to term->slug in attachment_fields_to_edit() function in /wp-admin/includes/media.php
20765.patch (359 bytes) - added by SergeyBiryukov 5 years ago.

Download all attachments as: .zip

Change History (9)

@eddiemoya
5 years ago

Changes term->name to term->slug in attachment_fields_to_edit() function in /wp-admin/includes/media.php

#1 @eddiemoya
5 years ago

To be clear, the conflict is seen and data is lost when two terms of the same name are applied to an attachment and saved, then re-opened, and saved once again. Because the media page renders outputs the term names, it will always assume that the first term is finds with that name is the correct one.

There is no way through javascript or otherwise, to fix this, since there is no way to know which was the originally intended term. I've managed to work around this in my plugin by filtering the function that does this, duplicating all the code, except switching it to slugs instead.

http://plugins.trac.wordpress.org/changeset/549552/media-categories-2/tags/1.3.1/media-categories.php

#2 @eddiemoya
5 years ago

  • Version set to 3.3.2

I've posted about this issue on my blog which includes step-by-step directions on recreating this bug.

#3 @eddiemoya
5 years ago

  • Version changed from 3.3.2 to 3.4.1

#4 follow-up: @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 3.5
  • Version changed from 3.4.1 to 3.3.2

Version number indicates when the bug was initially introduced/reported.

Reproduced with the steps from comment:2. The code was introduced in [6659].

Considering backwards compatibility, would it make more sense to pass the whole $terms array in a separate key?

Related: #21391

#5 in reply to: ↑ 4 @eddiemoya
5 years ago

Sorry about setting the version number wrong, I have not done many patches.

Replying to SergeyBiryukov:

Considering backwards compatibility, would it make more sense to pass the whole $terms array in a separate key?

I dont know if it makes sense in this case to preserve backward compatibility in this particular case - any plugins (including earlier versions of mine) that rely on this would themselves also be broken. We'd be preserving broken behavior for the sake of backward compatibility with plugins that were, by extension, also broken.

If this were an enhancement, or a new feature then yes. However this is just broken - the category field using names can't work as they are, and they never did.

Thanks for looking into this.

#6 @nacin
5 years ago

  • Keywords commit added

Looks good.

#7 @nacin
5 years ago

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

In [21889]:

Return term slugs rather than term names in attachment_fields_to_edit() as that is what we are dealing with. props eddiemoya, fixes #20765.

Note: See TracTickets for help on using tickets.