Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#28038 closed defect (bug) (fixed)

Only show the Edit Image option in the media modal if applicable

Reported by: clorith's profile Clorith Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.9
Component: Media Keywords: has-patch
Focuses: ui, javascript, administration Cc:


The general media screen filters out the edit option if there's no valid image processing library found. In the media modal this check is never performed so the Edit Image button is available regardless, and thus shows a broken image.

For consistency, I've added a check to not display the edit link if no library is found using wp_image_editor_supports.

I tested with a variety of file types, and couldn't find any other files that would trigger the Edit option in the modal to begin with, so I believe this should be a safe patch.

Attachments (4)

28038.patch (719 bytes) - added by Clorith 10 years ago.
28038.diff (1.1 KB) - added by nacin 10 years ago.
28038.2.diff (1.6 KB) - added by ericlewis 10 years ago.
28038.3.diff (1.6 KB) - added by ericlewis 10 years ago.

Download all attachments as: .zip

Change History (14)

10 years ago

#1 @DrewAPicture
10 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.9.1
  • Type changed from enhancement to defect (bug)

Since we've made the image editor quite a bit more discoverable via the modal, it seems to me that this bug might be more discoverable as well. Moving to 3.9.1 for consideration.

#2 @samuelsidler
10 years ago

  • Summary changed from Only show the Edit Image option in the media modal if aplicable to Only show the Edit Image option in the media modal if applicable

#3 @nacin
10 years ago

  • Keywords needs-patch added; has-patch removed

This patch will break things... The other nonces are used for any kind of type. It's specifically only the 'edit' nonce that needs to be restricted.

_wp_image_editor_choose() is not exactly cheap, so we should cache it inside that function so we're only running it once per mime type. We should also only run it when we're dealing with an image mime type.

#4 @Clorith
10 years ago

My experience with caching is limited to transients, would that be a sufficient approach in this situation (that would leave a DB query, I don't know how they compare in cost to _wp_image_editor_choose()).

Other than that, moving the media check around a little to account for other media types needing other nonces shouldn't be a problem at all.

10 years ago

#5 @nacin
10 years ago

  • Milestone changed from 3.9.1 to 3.9.2

I don't love the static variable, but thinking something like 28038.diff.

10 years ago

#6 @ericlewis
10 years ago

  • Focuses javascript added
  • Keywords has-patch added; needs-patch removed

Riffing a bit, attachment:28038.2.diff sets a property imageEditorSupports on the settings object, which is a flat array of image mime-types the system can handle. Then compare the attachments mime-type at microtemplate compile-time to decide whether to show the Edit Image link or not.

This ticket was mentioned in IRC in #wordpress-dev by ericandrewlewis. View the logs.

10 years ago

10 years ago

#8 @ericlewis
10 years ago

attachment:28038.3.diff adds proper instantiation, in case no image editing support is found.

#9 @helen
10 years ago

  • Milestone changed from 3.9.2 to 4.0

#10 @wonderboymusic
10 years ago

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

In 29529:

Media Modal: don't show the "Edit Image" button for attachments that don't have data.sizes set.

Fixes #28038.

Note: See TracTickets for help on using tickets.