974646 - [B2G][Contacts] Linking a Facebook contact to existing contact shows local image on contacts list, but Facebook picture in contact details

Status: VERIFIED FIXED

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

Description

[Brogan Zumwalt [Inactive]]

Attachment: Screenshot

Description:
Linking an existing contact (with picture set to a locally stored image) to an imported Facebook contact produces a contact where the photo in the contacts list shows the locally stored image, but the contact details shows the Facebook profile picture.

Repro Steps:
Prerequisite 1: Have an existing contact with picture set to a locally stored image
Prerequisite 2: Have an imported Facebook contact with a Facebook profile picture
1) Updated Buri to BuildID: 20140219004003
2) Open Contacts app
3) Select contact from prerequisite 1
4) Press "Link Contact"
5) Choose Facebook contact from prerequisite 2
6) Observe new contact image in details
7) Go back to contacts list and observe contact image thumbnail

Actual:
Contact linked to Facebook contact shows a different image in details than appears in the contact list thumbnail.

Expected:
Single image displayed in both details and list view for a given contact.

Environmental Variables:
Device: Buri v1.3 Mozilla RIL
BuildID: 20140219004003
Gaia: a43904d9646109b48836d62f7aa51e499fbf4b2e
Gecko: 97922b6daad1
Version: 28.0
Firmware Version: v1.2-device.cfg

Notes:
Repro frequency: 3/3, 100%
See attached: screenshot & linked Youtube video clip - http://youtu.be/4JOto1UgOwU

Comment 1

[Jason Smith [:jsmith]]

What happens on 1.1?

Comment 2

[Jose Manuel Cantera]

another regression from bug 967272 (contacts thumbnails). noming it to 1.3+ as it is a regression and happens on v1.3 as well, breaking the essential concept of the FB integration experience

Comment 3

[Jose Manuel Cantera]

Attachment: 16476.html

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 8378880 [details]
16476.html

Looking good, please:
* Expend the comment a bit to show the rationale behind choosing the picture at index 1.
* Add a unit test to cover the change [1].

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/test/unit/contact_photo_helper_test.js

Comment 5

[Jose Manuel Cantera]

Comment on attachment 8378880 [details]
16476.html

Hi Etienne,

My initial patch was incorrect as I haven't understood well your code at first glance :) . Then, I discovered that we miss a code path in thumbnail generation, thus I needed to add that code as well. Last but not least we have tests as well.

many thanks for the reviews

Comment 6

[bhavana bajaj [:bajaj]]

This is a fallout from the checkboarding fixes we took and since a backout is not likely here, blocking on this bug to avoid shipping with this regression as this scenario could be quite confusing to an end user.

Comment 8

[Etienne Segonzac (:etienne)]

Comment on attachment 8378880 [details]
16476.html

Perfect!
(Made a tiny comment on github about an early return but it's optional.)

Comment 9

[Jose Manuel Cantera]

landed in master with the last recommendation from Etienne adressed ;)

thanks!

https://github.com/mozilla-b2g/gaia/commit/1c7fa06880e63164061ecc7427aa514ed19c9eb9

Comment 10

[Jose Manuel Cantera]

Comment on attachment 8378880 [details]
16476.html

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Contacts thumbnails
[User impact] if declined: High, inconsistent UX, and regressing from 1.1 
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Low risk patch
[String changes made]:

Comment 11

[[:fabrice] Fabrice Desré]

(In reply to Jose M. Cantera from comment #10)
> Comment on attachment 8378880 [details]
> 16476.html
> 
> NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to
> better understand the B2G approval process and landings.
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): Contacts thumbnails
> [User impact] if declined: High, inconsistent UX, and regressing from 1.1 
> [Testing completed]: Yes

I'm mostly interested in *how* that is tested. Automated test? verified by QA? by yourself?

Comment 12

[Jose Manuel Cantera]

(In reply to Fabrice Desré [:fabrice] from comment #11)
> (In reply to Jose M. Cantera from comment #10)
> > Comment on attachment 8378880 [details]
> > 16476.html
> > 
> > NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to
> > better understand the B2G approval process and landings.
> > 
> > [Approval Request Comment]
> > [Bug caused by] (feature/regressing bug #): Contacts thumbnails
> > [User impact] if declined: High, inconsistent UX, and regressing from 1.1 
> > [Testing completed]: Yes
> 
> I'm mostly interested in *how* that is tested. Automated test? verified by
> QA? by yourself?

I've tested it myself. Also the patch includes unit tests. would be nice if QA test it as well.

Comment 13

[KTucker [:KTucker][Inactive 3/4/2016]]

This issue does not occur on the latest Buri v 1.4.0 Mozilla RIL

Environmental Variables
Device: Buri v 1.4.0 Mozilla RIL
Build ID: 20140224040201
Gecko: https://hg.mozilla.org/mozilla-central/rev/31113754db3b
Gaia: ffb527b84594396ed611edf0a8a5a130d60a742f
Platform Version: 30.0a1
Firmware Version: v1.2-device.cfg

The photo is the same for the thumbnail and for the contact in the detailed view after linking to a Facebook contact.

Comment 14

[KTucker [:KTucker][Inactive 3/4/2016]]

The locally stored photo is chosen instead of the Facebook contact's photo the contact is being linked to. Is this correct behavior?

Comment 15

[Jason Smith [:jsmith]]

(In reply to ktucker from comment #14)
> The locally stored photo is chosen instead of the Facebook contact's photo
> the contact is being linked to. Is this correct behavior?

I think so.

Comment 16

[Maria Angeles Oteo (:oteo)]

(In reply to ktucker from comment #14)
> The locally stored photo is chosen instead of the Facebook contact's photo
> the contact is being linked to. Is this correct behavior?

Yes, I can confirm you that's the correct behavior.

Comment 17

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 1c7fa06880e63164061ecc7427aa514ed19c9eb9 to:
v1.3: fa7fe9975165f5ae9dc08621323219501f17fd01

Comment 18

[Ross Kuhlman ([e-mail])]

Verified fixed in v1.4. The local profile picture is displayed in the list and on the contact details page.

Environmental Variables:
Device: Buri v1.4 Moz RIL
BuildID: 20140304040200
Gaia: 6df4533f14ec2645bb13d8a803a5151583ca13a8
Gecko: 529b86b92b1d
Version: 30.0a1
Firmware Version: v1.2-device.cfg