[wp-trac] [WordPress Trac] #39883: Code hooking on `image_downsize` can no longer assume the file is an image

[WordPress Trac]

#39883: Code hooking on `image_downsize` can no longer assume the file is an image
-----------------------------------+------------------------
 Reporter:  stephdau               |       Owner:  joemcgill
     Type:  defect (bug)           |      Status:  assigned
 Priority:  normal                 |   Milestone:  4.8
Component:  Media                  |     Version:  4.7
 Severity:  normal                 |  Resolution:
 Keywords:  has-patch 2nd-opinion  |     Focuses:
-----------------------------------+------------------------
Changes (by joemcgill):

 * keywords:  needs-patch => has-patch 2nd-opinion


Comment:

 As I understand it, there were essentially two main problems introduced in
 r38949:
 1. Non-image attachment IDs are now passed to the `image_downsize` filter,
 where they previously weren't.
 2. Non-supported attachment types trigger unnecessary DB calls when
 attempting to get `_wp_attached_file` and `_wp_attachment_metadata` types.

 Of those two, the second seems to be the most important to address, since
 there's no way for a developer to avoid this on their own, as @stephdau
 helpfully explained in the description of this issue.

 [attachment:39883.diff] resolves the second problem and partially
 addresses the first by only passing supported types to the filter. Here
 are my current assumptions:

 * `wp_get_attachment_image_src()` (and related functions) should be able
 to take a post ID for a PDF and produce image attributes (Since 4.7).
 * Only supported attachment types (i.e., image and pdf) should pass to
 `image_downsize` filter.
 * `image_downsize()` should bail early when the attachment type isn't a
 supported type so we can avoid extra DB hits when getting post meta for
 `_wp_attached_file` or `_wp_attachment_metadata`.

 For now, I think it makes sense to continue passing PDFs to the
 `image_downsize` filter so integrators can still rewrite URLs for PDF
 previews with CDN addresses (which seems to be a common use case for this
 filter). In the future, It makes sense to consider splitting all PDF
 functionality into a separate set of functions to avoid confusion with the
 image functions, as described in #39980. Additionally, we should avoid
 adding additional filters for PDF handling until we can consider
 approaches to #39980.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/39883#comment:23>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform