Closed
Bug 848625
Opened 11 years ago
Closed 11 years ago
[OPEN_][Gallery]the crop box is shown dislocation when landscape in edit mode.(617002043345)
Categories
(Firefox OS Graveyard :: Gaia::Gallery, defect)
Tracking
(blocking-b2g:tef+, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 verified)
VERIFIED
FIXED
blocking-b2g | tef+ |
People
(Reporter: Firefox_Mozilla, Assigned: jhylands)
References
Details
Attachments
(3 files, 1 obsolete file)
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.
Reporter | ||
Comment 1•11 years ago
|
||
Comment 2•11 years ago
|
||
I can reproduce it with the newest nightly build 20130306070201.
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Updated•11 years ago
|
blocking-b2g: --- → tef?
Comment 3•11 years ago
|
||
(tef+, as this is a pretty bad functional issue with croping)
blocking-b2g: tef? → tef+
Comment 5•11 years ago
|
||
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.
Assignee: dflanagan → jhylands
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #724023 -
Flags: review?(dflanagan)
Assignee | ||
Comment 7•11 years ago
|
||
Attachment #724023 -
Attachment is obsolete: true
Attachment #724023 -
Flags: review?(dflanagan)
Attachment #725088 -
Flags: review?(dflanagan)
Comment 8•11 years ago
|
||
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!
Attachment #725088 -
Flags: review?(dflanagan) → review-
Assignee | ||
Updated•11 years ago
|
Attachment #725088 -
Attachment description: fix dialer history to work with different AppIDs (rebased off master) → cleaned up formatting, fixed 2 new issues in gallery edit
Attachment #725088 -
Flags: review- → review?(dflanagan)
Comment 9•11 years ago
|
||
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'
Attachment #725088 -
Flags: review?(dflanagan) → review-
Assignee | ||
Comment 10•11 years ago
|
||
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.
Attachment #725088 -
Attachment description: cleaned up formatting, fixed 2 new issues in gallery edit → fixed so crop resize still works after orientation change
Attachment #725088 -
Flags: review- → review?(dflanagan)
Comment 11•11 years ago
|
||
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.
Attachment #725088 -
Flags: review?(dflanagan) → review-
Assignee | ||
Comment 12•11 years ago
|
||
Comment on attachment 725088 [details]
fixed so crop resize still works after orientation change
fixed lint issues
Attachment #725088 -
Flags: review- → review?(dflanagan)
Comment 13•11 years ago
|
||
Landed on gaia master: https://github.com/mozilla-b2g/gaia/commit/3010edd727863a657a3ef784ed7cbce4844e5c4a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 14•11 years ago
|
||
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)
Comment 15•11 years ago
|
||
We won't be able to uplift this until bug 826249 is uplifted.
Comment 17•11 years ago
|
||
Landed on v1-train: https://github.com/mozilla-b2g/gaia/commit/6dbf0b68622aa80d079fee2af660ddc47f7cd7df
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → wontfix
Comment 18•11 years ago
|
||
Landed on v1.0.1: https://github.com/mozilla-b2g/gaia/commit/d40dcdd112f12e2a5a0d1de46451670918fd4369
status-b2g18-v1.0.1:
--- → fixed
Comment 19•11 years ago
|
||
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
Updated•10 years ago
|
Attachment #725088 -
Flags: review?(dflanagan)
You need to log in
before you can comment on or make changes to this bug.
Description
•