[wp-trac] [WordPress Trac] #31050: Better PDF Upload Management

[WordPress Trac]

#31050: Better PDF Upload Management
-------------------------------------------------+-------------------------
 Reporter:  celloexpressions                     |       Owner:
     Type:  feature request                      |  mikeschroder
 Priority:  normal                               |      Status:  assigned
Component:  Media                                |   Milestone:  4.7
 Severity:  normal                               |     Version:
 Keywords:  has-patch needs-unit-tests needs-    |  Resolution:
  testing                                        |     Focuses:  ui
-------------------------------------------------+-------------------------

Comment (by mikeschroder):

 Replying to [comment:93 joemcgill]:
 > 2. I fixed two issues with the way orientations were being calculated.
 First, in `wp_prepare_attachment_for_js()` when we use the sizes fallback
 to set the full size data we were calculating the orientation in reverse.
 While it's simple to do this calculation, I'm not against adding a helper
 function as @markoheijnen suggested on Slack. I also fixed a typo which
 meant we were failing to set the orientation, height, and width attributes
 for the response.

 Also not opposed. Good catch!

 > 3. The only other change is to go back to the default size and crop
 attributes for image sizes in `wp_generate_attachment_metadata()`, which I
 don't feel strongly about if there is a good reason to hard code those
 values in.

 This was done to make the thumbnail size more useful. By disabling crop
 for the thumbnail size, this makes the entire page visible, and avoids
 cropping out of the middle of a page (which allows visible headers in the
 thumbnail). I've left it the way you changed it, but please test with both
 and see which one you prefer (it's easiest to test by uploading various
 PDFs with each patch, then looking in the Media list view).

 > Remaining issues I see:
 > 1. On the edit media screen `wp-admin/post.php?action=edit` the
 thumbnail is not showing up for PDFs because it's attempting to use the
 attachment file and not the thumbnail image.

 This was caused by the removal of the bits in `image_downsize()` that
 replaced the PDF's filename with the "full image" filename.

 I've restored these in the [attachment:31050.5.diff most recent patch],
 but am definitely open to a different or cleaner way of doing it.

 > 2. I'm not confident that adding the `attachment_fallback_mimetypes`
 filter is needed here since any mimes that are added to that array might
 need its own process for creating a fallback image rather than using the
 same process we use for PDFs.

 I'm not opposed to pulling this for now, and adding it back if there is a
 call for it.

 > I'm going to keep testing, but otherwise this looks good.

 Great to hear! I noticed that the 'full' size is still in there. I've been
 assuming you want to see it pulled before a commit, rather than
 standardize on it, but want to be sure we're on the same page here, since
 there's a bit of doing to do so. :)

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