[Contacts] Changing Contact Photo in 'Edit Contact' but not Updating the changes will maintain the new Contact Photo between edit sessions.

VERIFIED FIXED in 2.2 S7 (6mar)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: onelson, Assigned: ferjm)

Tracking

({regression})

unspecified
2.2 S7 (6mar)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [2.2-Daily-Testing p=2], URL)

Attachments

(4 attachments)

(Reporter)

Description

4 years ago
When the user is editing a contact and changes their 'Preview Image', they will observe that if they do not 'Update' their contact with their changes that returning to editing that contact will maintain the changed 'Preview Image'. The image can be saved to the contact if the user changes another field while editing their contact, otherwise the 'Update' button is not available.
The Preview Contact Photo will return to it's previous image if the application is restarted.
This is different behavior from the text fields within a contact, that upon exiting the 'Edit Contact' page without having updated, those changes will be dismissed and the text fields will return to their previous state (current contact details).
   
Repro Steps:
1) Update a Flame device to BuildID: 20141231010205
2) Open 'Contacts' app.
3) Create a new contact: Contact Photo required (additional text fields filled will help demonstrate issue).
4) Save contact.
5) Edit above contact.
6) Change Contact photo (and text fields for clarity).
7) Tap the 'X' in top left to leave edit mode without updating.
8) Edit the same Contact.
9) Observe Contact Photo (and 'modified' text fields).
  
Actual:
Contact Photo maintains the changed image from the previous 'Edit Contact' state, while text fields reset. Preview Contact Photo can be dismissed by restarting the application.
  
Expected: 
Edited Contact fields are reset if user chooses not to update.
  
******************************
Environmental Variables:
Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141231010205
Gaia: 26d479f0fccb7174e06255121e4e938c1b280d67
Gecko: 88037f94b7d7
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (2.2 Master)
Firmware: V18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
******************************
  
This also occurs on Base Image v188-1

Repro frequency: 5/5
See attached: 
video- http://youtu.be/gxs6lyedwH0
logcat
(Reporter)

Comment 1

4 years ago
Issues DOES NOT REPRO on 2.1 flame devices:
Results: Edited Contact fields are reset if user chooses not to update.
Repro Rate: 5/5

********************************************
Environmental Variables:
----------------------------------------------
Device: Flame 2.1
BuildID: 20141231001232
Gaia: 73be51f998031f06db0cd660c0e388fa621c9f4c
Gecko: ea426e47bfc4
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 34.0 (2.1)
Firmware: V18D
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
----------------------------------------------
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Keywords: regression
Whiteboard: [2.2-Daily-Testing]
Functional regression that exhibits unwanted change in data.

Requesting a window.
blocking-b2g: --- → 2.2?
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
triage: regression
blocking-b2g: 2.2? → 2.2+
QA Contact: pcheng
(Assignee)

Updated

4 years ago
Assignee: nobody → ferjmoreno
b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20141120110801
Gaia: e327daf01fac1e5fbe2ba732c26e3a21db32443a
Gecko: a3b72fe67767
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20141120143805
Gaia: d695e7cdcd162e779e15594054931c84dec34a95
Gecko: 353f9d83297b
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First Broken Gaia & Last Working Gecko - issue DOES repro
Gaia: d695e7cdcd162e779e15594054931c84dec34a95
Gecko: a3b72fe67767

First Broken Gecko & Last Working Gaia - issue does NOT repro
Gaia: e327daf01fac1e5fbe2ba732c26e3a21db32443a
Gecko: 353f9d83297b

Gaia pushlog:
https://github.com/mozilla-b2g/gaia/compare/e327daf01fac1e5fbe2ba732c26e3a21db32443a...d695e7cdcd162e779e15594054931c84dec34a95

Caused by the patch to Bug 1098218.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Jose, could you please take a look at this? Looks like the patch for Bug 1090210 caused this issue.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(jmcf)
(Assignee)

Comment 6

4 years ago
I am already working on this
Flags: needinfo?(jmcf)
(Assignee)

Comment 7

4 years ago
Posted patch FixSplinter Review
This fixes the issue for me. I never looked into form.js before though... http://media.topito.com/wp-content/uploads/2013/01/code-29.gif  :_(

I'll add some tests tomorrow.
(Assignee)

Comment 8

4 years ago
Sorry for the late, I hit bug 1061698 while working on the tests.
Attachment #8548233 - Flags: review?(francisco)
Comment on attachment 8548233 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/27351

Hi Fernando, can you rebase please?
Attachment #8548233 - Flags: review?(francisco)
I rebased, but it seems that TBPL is not happy with the new tests.

12:45:07     INFO -  TEST-UNEXPECTED-FAIL | apps/communications/contacts/test/marionette/form_test.js | Contacts > Form > Edit Contact > Discard edited contact photo Select photo and discard it
12:45:07     INFO -  NoSuchElement: (7) Unable to locate element: .thumbnail img

That means that we have no pictures on the gallery on TBPL. Any idea about how to fix this?
Flags: needinfo?(jlorenzo)
Flags: needinfo?(francisco)
Looking at your test, I don't think there is an image in the B2G internal memory. To make sure there are here, you need to push the image to B2G like they do in the Gallery tests: https://github.com/mozilla-b2g/gaia/blob/master/apps/gallery/test/marionette/delete_image_test.js#L21
Flags: needinfo?(jlorenzo)
Flags: needinfo?(francisco)
Fernando do you need a hand with that test?
Flags: needinfo?(ferjmoreno)
Yes, any help is highly appreciated :) Thanks

I just tried another approach. I open the gallery app and wait for it to be populated with the test pictures before starting the test. Again, it works locally, let's see how it behaves in treeherder https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=477a54cf7310
Flags: needinfo?(ferjmoreno)
It failed with the same error.
Jonathan, I am not sure if you are the right person to ask about marionette issues in treeherder, but I was wondering if you could give me a hand here. The issue is that I have this marionette test that takes a photo from the gallery. It works great locally cause b2g desktop takes the pictures from the local disk (at least in OSX). But it fails in treeherder cause we have no pictures there apparently. So to be sure that we have at least one picture I add a test image from the test itself this way [1]. But still it says that it cannot find the photo in the gallery :( Any idea of what I might be doing wrong here?

Thanks in advance for any help. 

[1] https://github.com/ferjm/gaia/blob/bug1116889.contactphoto/apps/communications/contacts/test/marionette/form_test.js#L202
Flags: needinfo?(jgriffin)
Fernando, I'm probably not the best person to help here.  The failing log is linux, not OSX...have you verified it works correctly for you on that platform?

You might ask James Lal to suggest someone who might know what's going on here.
Flags: needinfo?(jgriffin)
Thanks Jonathan. The test works locally on linux as well.
Flags: needinfo?(jlal)
Whiteboard: [2.2-Daily-Testing] → [2.2-Daily-Testing] p=2
Target Milestone: --- → 2.2 S7 (6mar)
Whiteboard: [2.2-Daily-Testing] p=2 → [2.2-Daily-Testing p=2]
Should we land this?
(Assignee)

Updated

4 years ago
Attachment #8548233 - Flags: review?(francisco)
(Assignee)

Updated

4 years ago
Blocks: 1139799
Comment on attachment 8548233 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/27351

Thanks Fernando
Attachment #8548233 - Flags: review?(francisco) → review+
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
This issue is verified fixed in the latest Nightly Flame 3.0 build.  

Actual Results: The changes to the photo are not remembered between edits of the same contact.

Environmental Variables:
Device: Flame 3.0 KK (319 MB) (Full Flash)
BuildID: 20150310010227
Gaia: 2fb09da0cb9cefad9c6e40f57533fafda6d12557
Gecko: 6686aacf006f
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0
Status: RESOLVED → VERIFIED
Flags: needinfo?(ktucker)
Adding verifyme to check 2.2 once uplifted.
Flags: needinfo?(ktucker)
Keywords: verifyme
Please request Gaia v2.2 approval on this patch when you get a chance.
Flags: needinfo?(ferjmoreno)
Comment on attachment 8548233 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/27351

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Contacts edit form
[User impact] if declined: Confusing UX. The user needs to restart the app to revert changes done while editing a contact photo.
[Testing completed]: Manual tests, unit test added.
[Risk to taking this patch] (and alternatives if risky): No risk. The change is pretty specific to the issue.
[String changes made]: None
Flags: needinfo?(ferjmoreno)
Attachment #8548233 - Flags: approval-gaia-v2.2?
(Assignee)

Updated

4 years ago
Flags: needinfo?(jlal)
Attachment #8548233 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
This bug has been verified as "pass" on latest Nightly build of Flame v2.2 by the STR in Comment 0.

Actual results: Edited contact fields are reset as expected when user chooses 'X' icon instead of 'Update' icon.
See attachment: verified_v2.2.3gp
Reproduce rate: 0/10


Device: Flame v2.2 (Verified) 
Build ID               20150630162500
Gaia Revision          bd386f346eb1591fddbc84bf034b22700e7e2a58
Gaia Date              2015-06-30 15:53:15
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/f16c1125b9d6
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150630.200238
Firmware Date          Tue Jun 30 20:02:49 EDT 2015
Bootloader             L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.