Closed Bug 989361 Opened 7 years ago Closed 7 years ago

[Camera] [Madai] [Preview] Conflict between photo panning and swipe to next/previous picture gestures

Categories

(Firefox OS Graveyard :: Gaia::Camera, defect)

PowerPC
Gonk (Firefox OS)
defect
Not set
blocker

Tracking

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
1.4 S6 (25apr)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: dmarcos, Assigned: djf)

References

()

Details

(Whiteboard: [m+])

Attachments

(2 files)

Attached video paddingBug.MOV
When I zoom in an image and swipe my fingers to pan the gesture is interpreted as next/previous image gesture. See attached video.
Blocks: 983405
Youngjun - please assign to hyuna or anyone else you can take this up from your team
Severity: normal → blocker
Flags: needinfo?(jjoons79)
Whiteboard: [m+]
Dear Hyuna,

Please check it.
Assignee: nobody → hyuna.cho82
Flags: needinfo?(jjoons79)
Related to 987569. I'll try to fix this as part of that bug.
Assignee: hyuna.cho82 → dflanagan
blocking-b2g: --- → 1.4+
Wow. That's pretty bad, isn't it.  An earlier version was velocity-sensitive, only switching left or right if the swipe was fast enough. Somewhere in the translation of the preview code we lost the velocity test, so we're switching images on any left or right pan.

I think this is pretty easy to fix, and instead of relying on velocity, I think we can do better and only switch images if the image is already panned all the way to the edge, which is how we do it in gallery.
Attached file link to pull request
Wilson,

This bug changes the way we decide when to switch between images in preview mode. It relies on the return value of MediaFrame.prototype.pan() which tells how many pixels beyond the edge we are.

It also cleans up event names a bit and gets an event out of lib/panzoom.js
Attachment #8402998 - Flags: review?(wilsonpage)
Attachment #8402998 - Flags: ui-review?(amlee)
Comment on attachment 8402998 [details] [review]
link to pull request

Looks like a good patch, lots of simplification :)

I'm r-'ing because the patch breaks existing unit-tests and has not not added any additional unit-tests to prove the bug has been fixed.
Attachment #8402998 - Flags: review?(wilsonpage) → review-
Comment on attachment 8402998 [details] [review]
link to pull request

Hi David, 

Can you rebase your patch? The camera app crashes each time I open it from the homescreen. I had the same issue with Wilson's patch but it was fixed once it was rebased. Thanks!
Flags: needinfo?(dflanagan)
Attachment #8402998 - Flags: ui-review?(tshakespeare)
Amy,

This patch is actually superseded by the patch in 987569, so I'm just going to clear the review request here instead of rebasing this.
Flags: needinfo?(dflanagan)
Attachment #8402998 - Flags: ui-review?(tshakespeare)
Attachment #8402998 - Flags: ui-review?(amlee)
This is fixed by the patch that just landed in bug 987569.  This is a 1.4+ bug, so that patch in the other bug will need to be uplifted.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S6 (25apr)
Flags: in-moztrap?(ychung)
New test case needs to be added. There is no existing test case.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Test case has been added in moztrap:

https://moztrap.mozilla.org/manage/case/14350/
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(ychung)
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.