Steps to reproduce: 1.Go into Gallery and choose an image to view; 2.tap “edit” button and select crop; 3.landscape Expected results: 1.the image an the crop box are shown normally. Actual results: 1.the image is distorted and the crop box is over the image.
Created attachment 722090 [details] Screenshot of crop error I can reproduce it with the newest nightly build 20130306070201.
(tef+, as this is a pretty bad functional issue with croping)
David, are you the best owner here?
Reassigning this to Jon, as he's been working on this in bug 842700. Since this bug is tef+, however, we'll move Jon's work here rather than there. Note that the editing function has never worked at all in landscape mode. Jon is making it work, but his patch will involve more than cropping. It doesn't make sense to fix just the cropping because editing would still be unusable in landscape mode in that case.
Created attachment 725088 [details] fixed so crop resize still works after orientation change
Comment on attachment 725088 [details] fixed so crop resize still works after orientation change In testing, this works well. There are a couple of things that would be nice to fix (but not required) 1) You mentioned you were getting two resize events. When I rotate the phone, I see the image change sizes twice. Can we hide or prevent the first one? 2) If I edit and image by sliding the exposure slider, then the next time I enter edit mode, I see the exposure slider start off at its old location and then move to 0. Maybe you can reset it to zero when exiting edit mode so it starts off in the right place. 3) Here's a weird corner case. For cropping, the crop handles aren't allowed to get smaller than 100x100 so that the touch regions don't overlap each other. If you start in portrait mode, and do a freeform crop to as small as possible then switch to landscape mode, the crop frame becomes smaller than 100x100 and now it won't let me drag to enlarge it. This isn't something we'd block on because it is easy to work around. But it might be easy to add the necessary logic to prevent making the handles smaller than 100x100 but always allowing them to be enlarged (but not allowing them to be enlarged more than the image size, of course). There's a meta issue: because of your git troubles, you now have a new commit message that does not include the bug number or describe your fix. So you need to update the message. (Do a git rebase -i master and use the 'reword' option to edit it.) I've commented on the code on github. There are a couple of issues to fix before we can land it. Overall, though, this is really nice work. Landscape editing works!
Comment on attachment 725088 [details] fixed so crop resize still works after orientation change Jon, Just one more round of fixes. See my comment on github about dealing with the too-small crop regions after rotation. Thanks for fixing the non-blocking stuff. This is going to be good! Did getting rid of the extra event handler make the flash on rotation go away? Finally, I'd still like you to alter your commit message. Convention seems to be to include the word 'Bug' before the number, and describe the overall effect of the patch, not just the most recent commit. Something like: 'Bug 848625 - make image editing work in landscape orientation'
Comment on attachment 725088 [details] fixed so crop resize still works after orientation change Modified the move() function so that it only allows the user to resize the crop region bigger if it is smaller than the minimum allowed size.
Comment on attachment 725088 [details] fixed so crop resize still works after orientation change r- for lint errors in the travis build: https://travis-ci.org/mozilla-b2g/gaia/builds/5593977#L63 Looks like just some too-long lines.
Comment on attachment 725088 [details] fixed so crop resize still works after orientation change fixed lint issues
Landed on gaia master: https://github.com/mozilla-b2g/gaia/commit/3010edd727863a657a3ef784ed7cbce4844e5c4a
I was not able to uplift this bug to v1-train and v1.0.1. If this bug has dependencies which are not marked in this bug, please comment on this bug. If this bug depends on patches that aren't approved for v1-train and v1.0.1, we need to re-evaluate the approval. Otherwise, if this is just a merge conflict, you might be able to resolve it with: git checkout v1-train git cherry-pick -x -m1 3010edd727863a657a3ef784ed7cbce4844e5c4a <RESOLVE MERGE CONFLICTS> git commit git checkout v1.0.1 git cherry-pick -x $(git log -n1 v1-train)
We won't be able to uplift this until bug 826249 is uplifted.
Landed on v1-train: https://github.com/mozilla-b2g/gaia/commit/6dbf0b68622aa80d079fee2af660ddc47f7cd7df
Landed on v1.0.1: https://github.com/mozilla-b2g/gaia/commit/d40dcdd112f12e2a5a0d1de46451670918fd4369
This issue no longer reproduces on Unagi device Build ID: 20130329070203 Kernel Date: Dec 5 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/56c922308fd1 Gaia: 0a9f78bffafda93a159c1f502e8b110c2f49a500 Also, issue appears fixed on Build ID: 20130329070203 Kernel Date: Dec 5 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/5cc5df16447a Gaia: 26b463f14caa11e0fc64fda09a17054da4bea68b The image in the crop box is shown normally, after switching to landscape mode. Verified Fixed