Opened 11 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 | Owned by: | wonderboymusic |
---|---|---|---|
Milestone: | 4.0 | Priority: | normal |
Severity: | normal | Version: | 3.9 |
Component: | Media | Keywords: | has-patch |
Focuses: | ui, javascript, administration | Cc: |
Description
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)
Change History (14)
#1
@
11 years ago
- Keywords has-patch added
- Milestone changed from Awaiting Review to 3.9.1
- Type changed from enhancement to defect (bug)
#2
@
11 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
@
11 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
@
11 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.
#5
@
11 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.
#6
@
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
#8
@
10 years ago
attachment:28038.3.diff adds proper instantiation, in case no image editing support is found.
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.