Last Comment Bug 994417 - [B2G][System]Missed call notification from a contact does not display the contact's name
: [B2G][System]Missed call notification from a contact does not display the con...
Status: RESOLVED FIXED
: regression
Product: Firefox OS
Classification: Client Software
Component: Gaia::Dialer (show other bugs)
: unspecified
: x86 Linux
-- normal (vote)
: 2.0 S1 (9may)
Assigned To: Zibi Braniecki [:gandalf][:zibi]
: Matthew Vaughan
:
Mentors:
Depends on: 1086676
Blocks: 914414
  Show dependency treegraph
 
Reported: 2014-04-09 17:06 PDT by Andrew Stole [:astole]
Modified: 2014-11-03 09:47 PST (History)
21 users (show)
jlorenzo: in‑qa‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
2.0+
unaffected
fixed


Attachments
logcat (38.80 KB, text/plain)
2014-04-09 17:06 PDT, Andrew Stole [:astole]
no flags Details
Screenshot (151.73 KB, image/png)
2014-04-09 17:09 PDT, Andrew Stole [:astole]
no flags Details
pull request (46 bytes, text/x-github-pull-request)
2014-04-16 15:21 PDT, Zibi Braniecki [:gandalf][:zibi]
anthony: review+
Details | Review | Splinter Review

Description User image Andrew Stole [:astole] 2014-04-09 17:06:06 PDT
Created attachment 8404347 [details]
logcat

Instead of showing the contact's name in a missed call notification, the notification says 'Missed call From {{ contact }}'

Repro Steps:
1) Update a Buri to BuildID: 20140409130909
2) Created a contact with a valid phone number
3) Call the DUT with the number attached to the contact in step 2
4) End the call from the device making the call
5) Slide down notification bar

Actual:
The Missed call notification does not match the contact's information

Expected:
The name from the missed call should match the contact's information

1.5 Environmental Variables:
Device: Buri 1.5 MOZ
BuildID: 20140409130909
Gaia: 9d0b1bdf746823a94b13e6574c1d8304dc584763
Gecko: 5a5ed08df529
Version: 31.0a1
Firmware Version: V1.2-device.cfg

Repro frequency: 100%
See attached: logcat, screenshot
Comment 1 User image Andrew Stole [:astole] 2014-04-09 17:09:03 PDT
Created attachment 8404349 [details]
Screenshot
Comment 2 User image Jason Smith [:jsmith] 2014-04-09 19:03:21 PDT
Can we check to see if this happens on 1.4?
Comment 3 User image Gregor Wagner [:gwagner] 2014-04-10 01:25:25 PDT
(In reply to Andrew Stole from comment #0)
> Created attachment 8404347 [details]
> logcat
> 
> Instead of showing the contact's name in a missed call notification, the
> notification says 'Missed call From {{ contact }}'
> 
> Repro Steps:
> 1) Update a Buri to BuildID: 20140409130909
> 2) Created a contact with a valid phone number

What fields did you fill out when you created the contact?
Comment 4 User image Marcia Knous [:marcia - use ni] 2014-04-10 16:08:00 PDT
I also saw this bug today when testing Master, but I haven't been able to reproduce it. I thought initially that I had a missed call from a contact and then tried to add the number without filling out any fields, but I tried those steps and wasn't able to repro again. Will keep trying to get some better steps - in the meantime also adding Andrew for ni.
Comment 5 User image Dietrich Ayala (:dietrich) 2014-04-10 18:01:43 PDT
STR:

1. import contacts from Google
2. receive a call from a Google contact, and miss it
3. see notification
4. cry tears of sadness
Comment 6 User image Dietrich Ayala (:dietrich) 2014-04-10 18:02:33 PDT
Re comment #5:

* master + m-c, 4/10
* Nexus 4
Comment 7 User image Gregor Wagner [:gwagner] 2014-04-11 02:41:16 PDT
(In reply to Dietrich Ayala (:dietrich) from comment #5)
> STR:
> 
> 1. import contacts from Google
> 2. receive a call from a Google contact, and miss it
> 3. see notification
> 4. cry tears of sadness

Can you let me know how the contact looks like in the contact app? Are the name fields correct?
Comment 8 User image Gregor Wagner [:gwagner] 2014-04-11 02:42:08 PDT
(In reply to Dietrich Ayala (:dietrich) from comment #5)
> STR:
> 
> 4. cry tears of sadness

There is an easy fix for this. Next time, just pick up the phone so you don't have to cry :)
Comment 9 User image Matthew Vaughan 2014-04-11 17:01:13 PDT
(In reply to Jason Smith [:jsmith] from comment #2)
> Can we check to see if this happens on 1.4?

This does not reproduce for me on the 04/11/14 1.4 build on a Buri.

Device: Buri v1.4 MOZ RIL
BuildID: 20140411000202
Gaia: 6c50349f41d40ba175ea0fc0c2c2cbd739ba7170
Gecko: 28b419f0e857
Version: 30.0a2
Firmware Version: v1.2-device.cfg


(In reply to Marcia Knous [:marcia - use needinfo] from comment #4)
> I also saw this bug today when testing Master, but I haven't been able to
> reproduce it. I thought initially that I had a missed call from a contact
> and then tried to add the number without filling out any fields, but I tried
> those steps and wasn't able to repro again. Will keep trying to get some
> better steps - in the meantime also adding Andrew for ni.

I was able to reproduce the issue using the STR from comment 0 consistently. It appears that so long as the contact has either a first or last name, or both, the notification will display 'Missed call From {{ contact }}'. If the contact only has a phone number, it will display 'Missed call From <contact number>' where "contact number" actually shows the phone number from the contact who is calling the DUT.
Comment 10 User image Marcela Oniga 2014-04-14 05:30:16 PDT
Same issue on my Keon device,1.5.0.0 version, build identifier: 20140413025413
Comment 11 User image Alexandre LISSY :gerard-majax 2014-04-15 07:15:03 PDT
100% reproducing too.
Comment 12 User image Matthew Vaughan 2014-04-15 10:43:03 PDT
TINDERBOX:
This looks to be a gaia issue.

last working gaia/first broken gecko = NO REPRO
Gaia  650e8c2c611ed07495d3bf3769f44a0efd88a492
Gecko 7160658c4be3

first broken gaia/last working gecko = REPRO
Gaia  9d0b1bdf746823a94b13e6574c1d8304dc584763
Gecko e84d0f010637

B2G INBOUND:
- Last Working -
Device: Buri ENG Master (2.0) MOZ RIL
BuildID: 20140409004946
Gaia: 650e8c2c611ed07495d3bf3769f44a0efd88a492
Gecko: 2d76999fe595
Version: 31.0a1
Firmware Version: v1.2-device.cfg

- First Broken -
Device: Buri ENG Master (2.0) MOZ RIL
BuildID: 20140409010346
Gaia: 9d0b1bdf746823a94b13e6574c1d8304dc584763
Gecko: 1497cb76b145
Version: 31.0a1
Firmware Version: v1.2-device.cfg

Push log: https://github.com/mozilla-b2g/gaia/compare/650e8c2c611ed07495d3bf3769f44a0efd88a492...9d0b1bdf746823a94b13e6574c1d8304dc584763


Please note: The gaias for the Tinderbox and Inbound last working and first broken builds match. This is an odd situation, but I double checked to make sure this window was correct.
Comment 13 User image Jason Smith [:jsmith] 2014-04-15 13:47:08 PDT
The window here can be done deeper with b2g-inbound builds.
Comment 14 User image Matthew Vaughan 2014-04-15 16:35:24 PDT
(In reply to Jason Smith [:jsmith] from comment #13)
> The window here can be done deeper with b2g-inbound builds.

I can confirm the above regression window was found using B2G Inbound builds. I double checked by downloading the two builds directly from the PVT website and matching the gaia and geckos for both builds. If there is another area I can go to get a more refined window, please let me know.
Comment 15 User image bhavana bajaj [:bajaj] 2014-04-15 19:45:19 PDT
Starting with a NI on :rik from dialer here to get some direction as this is 100% reproducible. :rik, anything obvious form the long regression window we have here or something obviously wrong going by the logcat ?
Comment 16 User image Anthony Ricaud (:rik) 2014-04-16 07:21:22 PDT
This is a regression of the new l10n.js library. https://github.com/mozilla-b2g/gaia/commit/fd2d573db38fa4ff5e607e62a35a23cb15c025d7
Comment 17 User image Zibi Braniecki [:gandalf][:zibi] 2014-04-16 11:12:15 PDT
taking
Comment 18 User image Zibi Braniecki [:gandalf][:zibi] 2014-04-16 15:21:37 PDT
Created attachment 8407864 [details] [review]
pull request

Ok, it's a trivial fix.

So, the refactor is a bit more strict about the type of the variable you can pass as placeable.

The old code was just implicitly stringifying it while hte new code has a check to reject anything but strings and numbers.

https://github.com/mozilla-b2g/gaia/blob/master/shared/js/l10n.js#L784-L788

Fixing this by just explicitly stringifying the param passed to mozL10n.get.
Comment 19 User image Anthony Ricaud (:rik) 2014-04-16 15:28:14 PDT
Comment on attachment 8407864 [details] [review]
pull request

(Please ask for review to the module peers: https://wiki.mozilla.org/Modules/FirefoxOS)

I don't think this is the proper fix. We probably have other places of the code base relying on this old behaviour so the new lib should respect it imho.
Comment 20 User image Zibi Braniecki [:gandalf][:zibi] 2014-04-16 15:34:36 PDT
(In reply to Anthony Ricaud (:rik) from comment #19)
> Comment on attachment 8407864 [details] [review]
> pull request
> 
> (Please ask for review to the module peers:
> https://wiki.mozilla.org/Modules/FirefoxOS)
> 
> I don't think this is the proper fix. We probably have other places of the
> code base relying on this old behaviour so the new lib should respect it
> imho.

The API in the future will enable passing objects as context data like this:

`mozL10n.get('id', {'user': {'name': 'Zibi', 'gender': 'male'});`

and allow localizers to refer to 'user.name' or 'user.gender' as needed.
and we do not want to implicitly stringify that.
Comment 21 User image Anthony Ricaud (:rik) 2014-04-17 08:27:17 PDT
Alright. Since we're early in the cycle, I guess we can keep that check and fix all call sites. But late in the cycle, we might have to be less strict if we're still seeing such regressions.

So let's go with this approach. But we still need new tests to verify that we're sending a string to the l10n lib.
Comment 22 User image Zibi Braniecki [:gandalf][:zibi] 2014-04-17 09:57:31 PDT
Cool, thanks. 

I agree with this approach.

Can you help me design the test you'd like to see?

We're talking about this code: https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/dialer.js#L64-L80

Since I want to run toString() on primaryInfo in that scenario, it doesn't make much sense - it will be a string.
Comment 23 User image Zibi Braniecki [:gandalf][:zibi] 2014-04-18 13:34:11 PDT
Comment on attachment 8407864 [details] [review]
pull request

updated the patch to add the tests. re-requesting r=
Comment 24 User image Anthony Ricaud (:rik) 2014-04-18 14:19:21 PDT
Comment on attachment 8407864 [details] [review]
pull request

Reviewed via IRC.

Thanks!
Comment 26 User image Johan Lorenzo [:jlorenzo] 2014-11-03 09:47:33 PST
Added in bug 1086676

Note You need to log in before you can comment on or make changes to this bug.