WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 14 months ago

Last modified 14 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 5 years ago.
11325.2.diff (1.7 KB) - added by ericlewis 16 months ago.

Download all attachments as: .zip

Change History (24)

comment:1 @Denis-de-Bernardy6 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 @caesarsgrunt6 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-Bernardy6 years ago

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

comment:4 @caesarsgrunt6 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 @caesarsgrunt6 years ago

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

comment:6 @janeforshort6 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 @caesarsgrunt6 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 @caesarsgrunt6 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 @markjaquith6 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 @dossy5 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 @nacin5 years ago

  • Milestone changed from Awaiting Triage to Future Release

comment:14 follow-up: @solarissmoke5 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.

@solarissmoke5 years ago

comment:15 in reply to: ↑ 14 @mikeschinkel5 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.

@ericlewis16 months ago

comment:16 @ericlewis16 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 @SergeyBiryukov15 months ago

  • Milestone changed from Future Release to 4.0

comment:18 follow-up: @helen14 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 @SergeyBiryukov14 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 @SergeyBiryukov14 months ago

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

Bringing back, per IRC.

comment:21 @SergeyBiryukov14 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 @ocean9014 months ago

In 29269:

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

Note: See TracTickets for help on using tickets.