Make WordPress Core

Opened 6 years ago

Closed 12 months ago

Last modified 4 months ago

#44789 closed defect (bug) (duplicate)

REST API: Improved media titles when created from filename

Reported by: icaleb's profile iCaleb Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch needs-unit-tests
Focuses: rest-api Cc:

Description (last modified by SergeyBiryukov)

Issue first discovered here: It's very similar to this previous core ticket #37989

When you upload an image via the REST API, the image's automatically created title attribute is sub-optimal. As an example, if you upload image name.png through the REST API - the title will become image-name. If you do the same through WP core's media gallery though, the image title will be image name. More explanation on this in the above two tickets.

To get the rest api's behavior more inline with the rest of WP core, syncing up WP_REST_Attachments_Controller::create_item media_handle_upload() with media_handle_upload() seems like the best option.

I've attached a patch doing just this, however as a disclaimer, I'm fairly unfamiliar with the process of uploading an image via raw POST data - so I've just kind of left that path untouched and it will act the same as it currently does. Also, as an added benefit to making titles more friendly by default, using wp_basename() is i18n friendly.

Some concerns off the top of my head:

  1. Will pathinfo() in this scenario always be okay? It is used in media_handle_upload() but not in media_handle_sideload() where the regex is used instead to remove the extension.
  2. Are there cases where the "name" index in $files['file']['name'] won't be available when uploading a file?
  3. Any ideas to make the image title more friendly when going through the raw POST data path of the conditional?

Attachments (1)

44789.diff (1.5 KB) - added by iCaleb 6 years ago.

Download all attachments as: .zip

Change History (13)

6 years ago

#1 @SergeyBiryukov
6 years ago

  • Description modified (diff)
  • Keywords has-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.9.9

#2 @pento
6 years ago

  • Milestone changed from 4.9.9 to 5.0
  • Version trunk deleted

#3 @dryanpress
6 years ago

I've been testing this patch this morning.. wasn't immediately clear Gutenberg had been shim'd to alter this behavior, so I spun wheels trying to/being unable to recreate via Gutenberg 🤦‍♂️

Anyways, started to work through unit tests but brief update (I'll upload a new patch when complete):

  • Within else condition, passed $file param to basename() needs to be
    based on returned associative keys from wp_handle_sideload().
  • Still tracing origin, but $this->upload_from_data is returning a WP_Error instead of array after changing to $filefile?
Last edited 6 years ago by dryanpress (previous) (diff)

This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.

6 years ago

This ticket was mentioned in Slack in #core-restapi by dryanpress. View the logs.

6 years ago

This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.

6 years ago

This ticket was mentioned in Slack in #core-restapi by danielbachhuber. View the logs.

6 years ago

#8 @danielbachhuber
6 years ago

  • Milestone changed from 5.0 to Future Release

Given the timeline for shipping 5.0, it's unlikely this particular enhancement will make it for this release. We can consider it for a future release when there's a patch with tests to consider.

#9 @abitofmind
12 months ago

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #57957.

#10 @abitofmind
12 months ago

In #57957 there is already a fix for this. But the unit tests for the fix are still outstanding.

So there is some hope this will get resolved.

#11 @abitofmind
12 months ago

Anyone willing to fix the unit tests could expedite this! Help appreciated! Am no dev myself.

#12 @swissspidy
4 months ago

  • Milestone Future Release deleted
Note: See TracTickets for help on using tickets.