WordPress.org

Make WordPress Core

Opened 14 months ago

Last modified 12 months ago

#44789 new defect (bug)

REST API: Improved media titles when created from filename

Reported by: iCaleb Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch needs-unit-tests
Focuses: rest-api Cc:
PR Number:

Description (last modified by SergeyBiryukov)

Issue first discovered here: https://github.com/WordPress/gutenberg/issues/8902. 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 14 months ago.

Download all attachments as: .zip

Change History (9)

@iCaleb
14 months ago

#1 @SergeyBiryukov
14 months ago

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

#2 @pento
12 months ago

  • Milestone changed from 4.9.9 to 5.0
  • Version trunk deleted

#3 @dryanpress
12 months 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
    $file['file']
    
    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 12 months ago by dryanpress (previous) (diff)

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


12 months ago

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


12 months ago

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


12 months ago

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


12 months ago

#8 @danielbachhuber
12 months 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.

Note: See TracTickets for help on using tickets.