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)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:tef+, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 verified)

VERIFIED FIXED
blocking-b2g tef+
Tracking Status
b2g18 --- verified
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- verified

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.
Attached image screenshot
I can reproduce it with the newest nightly build 20130306070201.
Status: UNCONFIRMED → NEW
Ever confirmed: true
blocking-b2g: --- → tef?
(tef+, as this is a pretty bad functional issue with croping)
blocking-b2g: tef? → tef+
David, are you the best owner here?
Assignee: nobody → dflanagan
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
Attached file new pull request (obsolete) —
Attachment #724023 - Flags: review?(dflanagan)
Attachment #724023 - Attachment is obsolete: true
Attachment #724023 - Flags: review?(dflanagan)
Attachment #725088 - Flags: review?(dflanagan)
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-
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 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-
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 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-
Comment on attachment 725088 [details]
fixed so crop resize still works after orientation change

fixed lint issues
Attachment #725088 - Flags: review- → review?(dflanagan)
Landed on gaia master: https://github.com/mozilla-b2g/gaia/commit/3010edd727863a657a3ef784ed7cbce4844e5c4a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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.
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
Status: RESOLVED → VERIFIED
Attachment #725088 - Flags: review?(dflanagan)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: