WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 7 months ago

Last modified 7 months ago

#11325 closed defect (bug) (fixed)

Image cropping doesn't work for small areas

Reported by: caesarsgrunt Owned by: SergeyBiryukov
Milestone: 4.0 Priority: normal
Severity: normal Version: 2.9
Component: Media Keywords: has-patch commit
Focuses: ui Cc:

Description

Image cropping works for JPEG images, but not for PNGs. The crop button is disabled.

Rev.179738

Attachments (2)

11325.diff (2.0 KB) - added by solarissmoke 4 years ago.
11325.2.diff (1.7 KB) - added by ericlewis 9 months ago.

Download all attachments as: .zip

Change History (24)

comment:1 @Denis-de-Bernardy5 years ago

Works fine with this file...

http://www.semiologic.com/media/2009/11/26/turkey-day/Turkey-Day.png

The crop button is disabled until you actually select the area that you're meaning to crop. Possible UI niggle? How is this done in Flickr?

comment:2 @caesarsgrunt5 years ago

Odd - for me it's disabled even after electing the area. As I say, though, it works fine for jpegs. I'll do some more testing.

comment:3 @Denis-de-Bernardy5 years ago

it might be an 8-bit png vs 24-bit png problem or some weird thing like that.

comment:4 @caesarsgrunt5 years ago

Ok, it seems I misreported the bug. Didn't do enough testing.

The actual bug is that the crop button is disabled if the area selected is smaller than the default thumbnail size set in the Media preferences.

Real-life example of when this is a problem - I want to make small thumbnails from a small area of the image for a couple of the images on my site. The rest want to be big thumbs of the whole image; hence a largish default size.

I can't see any reason for disabling the button for small areas; is there a reason?

comment:5 @caesarsgrunt5 years ago

  • Summary changed from Image cropping doesn't work for PNGs to Image cropping doesn't work for small areas

comment:6 @janeforshort5 years ago

Discussing with Andrew, Mark, Ryan and Peter IRL right now. Popular opinion seems to be that it's generally reasonable to have a minimum size. Potential cases where it would be useful to go smaller would be if you want to insert little accent images like icons or something, but this seems a bit edge case to delay a release for since there's no patch. Moving to 3.0 with intention of addressing earlyish.

Note: Please do not change back to 2.9 unless a) there's a use case in which this is a blocker that we are missing, and b) there is a patch, since we are already in Beta 2.

comment:7 @caesarsgrunt5 years ago

Don't think there's a case in which it's a blocker, though it is very annoying to have a minimum of the standard thumb size.

If it is decided that this should be fixed I can provide a patch instantly; it's just a couple of lines need removing. Perhaps I'll add a patch anyway.

comment:8 @caesarsgrunt5 years ago

Hmm, turns out not to be as easy as I thought. Enabling the button is easy, but saving a small image returns a PHP error message. I'll investigate later.

comment:9 @markjaquith5 years ago

  • Milestone changed from 2.9 to 3.0

comment:10 @nacin5 years ago

  • Milestone changed from 3.0 to 3.1

comment:11 @mikeschinkel5 years ago

Just ran into this problem myself. Took a while to figure out that it wasn't a bug and was designed this way because it gave no feedback.

(Personal pet peeve: "Grayed-out and disabled anything because even though the grayed-out tells the user they can't do something it doesn't tell them why not or how to fix it. I'd much prefer to see nothing grayed-out (or at least nothing disabled) and to instead have it when click to launch a pop-up that explains why the user can't do whatever and that tells them how to fix it. But I digress...)

Anyway, here's a good reference on why this use-case is important (I know, Jane already blessed it, but this is still a good reference):

comment:12 @dossy4 years ago

  • Cc dossy@… added

I just ran into this "bug" as well - not being able to re-crop an image to generate a new thumbnail if the image has either x or y dimensions smaller than the thumbnail size as specified in Media settings.

I'd be happy to submit a patch today to "fix" this if someone authoritative can speak to what functionality would be acceptable.

comment:13 @nacin4 years ago

  • Milestone changed from Awaiting Triage to Future Release

comment:14 follow-up: @solarissmoke4 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

I came across this and it took me ages to work out why I couldn't crop - this really is unintuitive. Here is a proposed patch - it doesn't disable the button but issues an alert telling you what's up. Thoughts?

I'm going to need help with i10n-ifying the Javascript, because my attempts to get that to work failed miserably.

@solarissmoke4 years ago

comment:15 in reply to: ↑ 14 @mikeschinkel4 years ago

Replying to solarissmoke:

I came across this and it took me ages to work out why I couldn't crop - this really is unintuitive. Here is a proposed patch - it doesn't disable the button but issues an alert telling you what's up. Thoughts?

Agreed. I had the same experience.

@ericlewis9 months ago

comment:16 @ericlewis9 months ago

  • Focuses ui added
  • Keywords dev-feedback removed

This sounds completely valid to me and a very simple patch. attachment:11325.2.diff removes any requirement for a minimum image size in the image editor.

comment:17 @SergeyBiryukov9 months ago

  • Milestone changed from Future Release to 4.0

comment:18 follow-up: @helen7 months ago

  • Milestone changed from 4.0 to Future Release

Mostly seems to work in testing, but the image naming doesn't quite work out - I ended up with images named rss-2x-e1405966155275.png, as opposed to rss-2x-15x20.png. Punting, might be able to bring this back if we know what's going on with the naming.

comment:19 in reply to: ↑ 18 @SergeyBiryukov7 months ago

Replying to helen:

I ended up with images named rss-2x-e1405966155275.png, as opposed to rss-2x-15x20.png.

I think that applies to any edited image, and is probably unrelated.

comment:20 @SergeyBiryukov7 months ago

  • Keywords commit added
  • Milestone changed from Future Release to 4.0

Bringing back, per IRC.

comment:21 @SergeyBiryukov7 months ago

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

In 29259:

Remove requirement for a minimum image size in the image editor.

props ericlewis, solarissmoke.
fixes #11325.

comment:22 @ocean907 months ago

In 29269:

Fix jshint errors introduced in [29259]. see #11325.

Note: See TracTickets for help on using tickets.