Closed Bug 974646 Opened 7 years ago Closed 7 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 verified)

VERIFIED FIXED
1.4 S2 (28feb)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- verified

People

(Reporter: bzumwalt, Assigned: jmcf)

References

()

Details

(Keywords: regression, verifyme)

Attachments

(2 files)

Attached image 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
What happens on 1.1?
Keywords: qawanted
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
blocking-b2g: --- → 1.3?
Attached file 16476.html
Attachment #8378880 - Flags: review?(etienne)
Assignee: nobody → jmcf
Keywords: qawantedregression
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
Attachment #8378880 - Flags: review?(etienne)
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
Attachment #8378880 - Flags: review?(etienne)
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.
blocking-b2g: 1.3? → 1.3+
Target Milestone: --- → 1.4 S2 (28feb)
Duplicate of this bug: 975150
Comment on attachment 8378880 [details]
16476.html

Perfect!
(Made a tiny comment on github about an early return but it's optional.)
Attachment #8378880 - Flags: review?(etienne) → review+
landed in master with the last recommendation from Etienne adressed ;)

thanks!

https://github.com/mozilla-b2g/gaia/commit/1c7fa06880e63164061ecc7427aa514ed19c9eb9
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
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]:
Attachment #8378880 - Flags: approval-gaia-v1.3?(fabrice)
(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?
Keywords: verifyme
(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.
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.
The locally stored photo is chosen instead of the Facebook contact's photo the contact is being linked to. Is this correct behavior?
Status: RESOLVED → VERIFIED
(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.
(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.
Attachment #8378880 - Flags: approval-gaia-v1.3?(fabrice) → approval-gaia-v1.3+
Uplifted 1c7fa06880e63164061ecc7427aa514ed19c9eb9 to:
v1.3: fa7fe9975165f5ae9dc08621323219501f17fd01
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
You need to log in before you can comment on or make changes to this bug.