961620 - Pick activity sends the first picture taken instead of the last one

Status: RESOLVED FIXED

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

Description

[:gerard-majax]

On gecko+gaia master running on a Nexus S, when using the pick activity to attach photos, if you decide to retake the picture and then select it, then the first picture that was taken will be sent instead of the last one.

Gecko: 6d2f301
Gaia: 9418328

STR:

0. Open SMS app
1. Tap attachment
2 [review]. Select photo
3. Take one picture
4. Discard it and take another
5. Select this new picture

Expected: Last picture taken appears in the MMS
Actual: First picture taken appears in the MMS

Comment 1

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: unable to reproduce it.

Alexandre,

I cannot reproduce it with the STR mentioned at comment 0. I am using Inari with the master PVT 20140119040203. Is there anything I missed?

And I am not very sure about the step 4. Is the "discard it"  to use "replace image" to reselect one? Or to choose one image in gallery and go back to another? Or to use "remove image" and to add another one?

Comment 2

[:gerard-majax]

It is "replace image".

Comment 3

[Jason Smith [:jsmith]]

QA Wanted - Let's see if someone else can reproduce this.

Comment 4

[:gerard-majax]

You must use the activity exposed by the camera app, not the gallery one. Sorry for the wrong component.

Comment 5

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: retake doesn't work

Ok, I can reproduce it with Camera app. And the "discard" is to press "retake" button inside camera. The attached video shows this issue.

Comment 6

[Sarah Parsons]

This issue reproduces on the buri 1.4 Build ID: 20140121040201

Gaia   e218d17ae7d01a81d48f833cd6fafb4e11b26cd8
SourceStamp cdc0ab2c0cba
BuildID 20140121040201
Version 29.0a1

This issue does not reproduce on the Buri 1.3 Build ID: 20140121004137

BuildID: 20140121004137
Gaia: 47049555282a9a01fb60d1e1421b57e2810c96f5
Gecko: 6f7dfe36ab6c
Version: 28.0a2

Comment 7

[Sarah Parsons]

This issue started to occur on the Buri 1.4 Build ID:20140110040206

Gaia   f400efc804366c7b7cf5476d1d5d325e6651ee71
SourceStamp 37516445a0b5
BuildID 20140110040206
Version 29.0a1

Last working Buri 1.4 Build ID: 20140109040203

Gaia   47206ac66b084c6f6c4503a3b10d0e0760df2b6f
SourceStamp 9409405e0739
BuildID 20140109040203
Version 29.0a1

Comment 8

[Punam Dahiya [:pdahiya]]

Attachment: PR with fix of pick activity sends the first picture instead of last

Debugged and issue is click on select after multiple retakes triggers multiple onSelectClick listeners that sends multiple post results to the activity. 
The previous listeners should be removed on retake. Attaching PR with the fix where redundant select and retake listeners are explicitly removed.

The PR also fixes the below error seen in logs
01-30 13:00:52.169 E/GeckoConsole(  886): [JavaScript Error: "TypeError: camera is undefined" {file: "app://camera.gaiamobile.org/js/main.js" line: 8384}]

Please review

Comment 9

[Diego Marcos [:dmarcos]]

Thanks a lot Punam. Your patch is good but I think our code is not that good. I created an alternative patch that fixes the bug and also implements a small refactor of the confirm view/controller. In the new code I avoid destroying and recreating the confirm view and removing/attaching events at each time that the user retakes a picture/video. I need Wilson's input on this.

Comment 10

[Diego Marcos [:dmarcos]]

Attachment: Alternative pull request

Comment 11

[Wilson Page [:wilsonpage]]

dmarcos: Comments on Github.

Comment 12

[Diego Marcos [:dmarcos]]

Just modified the PR so we lazily create the view, render and attach the events only once during the life of the confirm view/controller.

Comment 13

[Diego Marcos [:dmarcos]]

New PR that avoids modifing evt.js to pass the name of the event to the event handler.

Comment 14

[Punam Dahiya [:pdahiya]]

Thanks Diego for refactoring and fixing this issue, re-assigning the bug to you.

Comment 15

[Diego Marcos [:dmarcos]]

Landed in master:

https://github.com/mozilla-b2g/gaia/commit/cd07ce781c2d8ea6630f606ee8741b6a12b666fc