1003979 - [B2G][Open_C][Dialer]Contact picture does not appear in the call log

Status: VERIFIED FIXED

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

Description

[Andrew Stole [:astole]]

Attachment: logcat

The image associated with a contact does not show up in the call log or messages timeline. The contact's picture appears in the messages timeline if the app is killed, but does not show up in the call log when the dialer app is killed.

Repro Steps:
1) Update a Open_C to BuildID: 20140430040206
2) Create a contact with a picture associated with it
3) Call contact
4) After call has been completed, navigate to call log

Actual:
The area where the image should be located is a blank circle

Expected:
The image associated with the contact shows up

2.0 Environmental Variables:
Device: Open_C 2.0 Master M-C MOZ
BuildID: 20140430040206
Gaia: badc73ee7f108fa631150bded0cc57e92aad810e
Gecko: e19812f56952
Version: 32.0a1
Firmware Version: P821A10-ENG_20140410

Repro frequency: 100%
See attached: Screenshot, logcat

Comment 1

[Andrew Stole [:astole]]

Attachment: Screenshot

This issue does not occur on the latest 1.4 on Open_C. The visual design is different on 1.4.

1.4 Environmental Variables:
Device: Open_C 1.4 MOZ
BuildID: 20140430000201
Gaia: 81e97c3ca58be0487292011bc59efa4cebab30be
Gecko: 123485e733d5
Version: 30.0
Firmware Version: P821A10-ENG_20140410

Comment 2

[Andrew Stole [:astole]]

This issue does NOT occur on Master M-C build on Buri

2.0 Master M-C Environmental Variables:
Device: Buri 2.0 Master M-C MOZ
BuildID: 20140430040206
Gaia: badc73ee7f108fa631150bded0cc57e92aad810e
Gecko: e19812f56952
Version: 32.0a1
Firmware Version: V1.2-device.cfg

Comment 3

[Noemí Freire (:noemi)]

Adding contact's picture withing call log is part of the work to be done under Visual Refresh for 2.0, adding the corresponding dependencies

Comment 4

[Maria Angeles Oteo (:oteo)]

it seems to be an OpenC specific bug, perhaps adding the new rounded contact's photo in the call log according to the 2.0 Visual Refresh proposal in a device with different resolution (as the OpenC)is causing the error.

Andrew, if you could be so kind of checking in the OpenC (master) if the contact's photo is shown in the thread list screen of the Messaging application? Just to try to figure out if the photos change are affecting to the messaging application too in the OpenC.

Comment 5

[Anthony Ricaud (:rik)]

I believe this is a regression from Building blocks. We haven't touched the call log yet in Dialer. And I'm seeing those weird empty circles in the Message app on my dogfooding phone.

Arnau: Have you landed anything in that area in building blocks?

Comment 6

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Sorry, I cannot reproduce this.
I don't have an open C, but I've followed the steps described in comment 1, and I can see the picture in my Peak, flashed with current master @1.5.

Comment 7

[Andrew Stole [:astole]]

Confirmed, as well as with comment 5, that the contact's photo is not seen in the messages app. However, if the messages app is killed then the picture shows up when the app is reopened. Killing and reopening the dialer app does not make it so the picture shows up.

Comment 8

[Maria Angeles Oteo (:oteo)]

(In reply to Andrew Stole from comment #7)
> Confirmed, as well as with comment 5, that the contact's photo is not seen
> in the messages app. However, if the messages app is killed then the picture
> shows up when the app is reopened. Killing and reopening the dialer app does
> not make it so the picture shows up.

That bug has also been reported in Bug 1003789 - [VR][Messaging] Contact's photo doesn't appear in the message inbox, and it's happennig in buri too.

Andrew, you have you tested it in the OpenC too, haven't you?

Comment 9

[Andrew Stole [:astole]]

Yes, I tested it on OpenC as well and saw it occur.

Comment 10

[Noemí Freire (:noemi)]

Hi,

Tested with today's (5/5) master build:
Device: Hamachi
BuildId: 20140505081601
Gecko: 6fafefc
Gaia: 265062
Platform version: 32.0a1

With the following STRs, the issue always occurs:
1- Tap on Contacts app
2- Create a contact (with image)
3- Tap on the contact created (contacts details) -> Tap on call
4- Tap on "home" button
5- Tap on dialer app
6- Tap on call log -> The contact image is properly shown
7- Being in call log, tap on contacts icon
8- Access to contacts details -> Tap on call
9- Tap on call log icon -> The contact image is not shown

Comment 11

[Chris Kreinbring [:CKreinbring]]

B2G-Inbound Regression window

No repro: 
Build ID: 20140429053002
Gecko: https://hg.mozilla.org/integration/b2g-inbound/rev/0d45d99bba09
Gaia: 1892ba3a857f7e9cd1d2a0cf1c87481f3dcaca2c
Platform Version: 32.0a1
Firmware Version: V1.2-device.cfg

Repro:    
Build ID: 20140429083002
Gecko: https://hg.mozilla.org/integration/b2g-inbound/rev/81dc74c9a5a2
Gaia: feef47764b9ae49fa5cc28a07186db7905481297
Platform Version: 32.0a1
Firmware Version: V1.2-device.cfg

Old Gaia / New Gecko - OK
New Gaia / Old Gecko - Fail
https://github.com/mozilla-b2g/gaia/compare/1892ba3a857f7e9cd1d2a0cf1c87481f3dcaca2c...feef47764b9ae49fa5cc28a07186db7905481297

Comment 12

[Jason Smith [:jsmith]]

Caused by bug 990478.

Comment 13

[Jason Smith [:jsmith]]

Noemi - Can you find someone to look into this?

Comment 14

[Jose Manuel Cantera]

I have my doubts this has to do with bug 990478.

By inspecting the logcat it can be found

E/GeckoConsole( 3068): [JavaScript Error: "IndexedDB UnknownErr: IDBTransaction.cpp:864"]

E/GeckoConsole( 3928): Security Error: Content at app://communications.gaiamobile.org/dialer/index.html#keyboard-view may not load data from blob:11a11751-710b-426a-bdbc-5f59ae1b6cf1.

Comment 15

[Jose Manuel Cantera]

Attachment: 18971.html

the problem was a race condition between URL rovoke operation and the assignment to the background img. I have just put a delay in the URL revoke. please let me know what do you think

thanks

Comment 16

[Etienne Segonzac (:etienne)]

Comment on attachment 8417911 [details]
18971.html

The |URL.revokeObjectURL(this.src);| line is commented, so with this patch we're just never revoking... :/
And I think we should be able to add a test thanks to sinon.useFakeTimers.

But before you make another version of the patch I'd like to ask Julien for recommendation since I recall the SMS app had a very similar issue recently.

Comment 17

[Jose Manuel Cantera]

(In reply to Etienne Segonzac (:etienne) from comment #16)
> Comment on attachment 8417911 [details]
> 18971.html
> 
> The |URL.revokeObjectURL(this.src);| line is commented, so with this patch
> we're just never revoking... :/

Oh yes, my mistake, thanks for spotting 

> And I think we should be able to add a test thanks to sinon.useFakeTimers.

sure

> 
> But before you make another version of the patch I'd like to ask Julien for
> recommendation since I recall the SMS app had a very similar issue recently.

ok, let me know

Comment 18

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Yes, Bug 994553 is what happens in the SMS app.

I had a lengthy discussion with Gecko developers, and here are the important points:

* we need to always revoke the blob url
* but if the blob is not an in-memory blob (eg: backed with a file, or IndexedDB), then it's fine to not revoke it asap, because it does not take much memory.

So I can have 2 suggestions for you if the blob is not in-memory:

* use a setTimeout with a big delay (IMO 1 second is not enough, you can still have the issue on slow or loaded devices; better use eg 60 seconds). There is one issue with that: eventually, Gecko developers want to lazy load the images used as background-image; I mean that if the image is not displayed, the url won't even be fetched. This is not happening right now though.

* or revoke the url when the element is removed from the page (not always easy to do)

If the blob is in-memory, then you should try to see why it's the case, but I think it's not here, right?

Comment 19

[Jose Manuel Cantera]

Etienne,

Please let me know your thoughts about how to continue with this

thanks

Comment 20

[Etienne Segonzac (:etienne)]

(In reply to Jose Manuel Cantera from comment #19)
> Etienne,
> 
> Please let me know your thoughts about how to continue with this

After discussing it with Julien I think our best bet right now is to do the revoke in a long setTimeout and cover it with a test.

Thanks!

Comment 21

[Jose Manuel Cantera]

Comment on attachment 8417911 [details]
18971.html

PR updated with the latest recommendations, including one test

Comment 22

[Etienne Segonzac (:etienne)]

Comment on attachment 8417911 [details]
18971.html

Really cool to see this coming with a test.

Can we just change the order to do onload/onerror _then_ the revoke timeout instead of timeout first then wait for load?

Comment 23

[Jose Manuel Cantera]

Comments adressed

Comment 24

[Etienne Segonzac (:etienne)]

Comment on attachment 8417911 [details]
18971.html

Thanks!

Comment 25

[Jose Manuel Cantera]

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

Comment 26

[Mike Lien[:mlien]]

veirfy with Flame KK v180 + v2.0 gaia/gecko, it's fine
Gaia      7edd3b0b9f65c3dde235c732d270e43e055a1254
Gecko     https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/13e04ab68621
BuildID   20140914162307
Version   32.0