1155582 - [Flame][Dialer]Go to "Call log" view to view the call info of a contact with email , the phone number will not show.

Status: VERIFIED FIXED

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

Description

[Boon]

Attachment: Bug log: logcat_345.txt

[1.Description]:
[Flame][v2.2&v3.0][Dialer]Go to "Call log" view to view the call info of a contact with email , the phone number will not show.
Attachment: logcat_345.txt and 345.mp4
Happen time: 3:45 PM

[2.Testing Steps]: 
Pre-condition: Contact A has phone number, but no email address; Contact B has phone number and email address.
1. Launch Dialer
2. Call contact A and B
3. Switch to Call log
4. Check the call information of the two call logs

[3.Expected Result]: 
4. It should show the phone information in call log no matter whether there is an email address.

[4.Actual Result]: 
4. It doesn't show the phone information in call log if there is email address in the contact.

[5.Reproduction build]: 
Device: Flame 2.2 (affected)
Build ID               20150415162504
Gaia Revision          89a94a8b9c9087da916751697a61ba15bc0688c3
Gaia Date              2015-04-15 20:05:38
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/20e92b626530
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150415.201243
Firmware Date          Wed Apr 15 20:12:52 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (affected)
Build ID               20150415160205
Gaia Revision          777d01f4a2c7b41c4b02e3cf87715714ccc0590b
Gaia Date              2015-04-15 17:20:09
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/53ceefb0e1c8
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150415.192055
Firmware Date          Wed Apr 15 19:21:04 EDT 2015
Bootloader             L1TC000118D0

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test

Comment 1

[Boon]

Attachment: Bug video: 345.mp4

Comment 2

[Eric Chang [:ericcc] [:echang]]

[Blocking Requested - why for this release]: broken functionality
UX
https://mozilla.app.box.com/s/44utizl9oz4eupyu3fuu/1/2184700727/19414616103/1

Comment 3

[Johan Lorenzo [:jlorenzo]]

I don't repro on 2.1:

Build ID               20150417001203
Gaia Revision          bbe983b4e8bebfec26b3726b79568a22d667223c
Gaia Date              2015-04-09 13:52:48
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/c54aa1be51d6
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20141120.194707
Firmware Date          Thu Nov 20 19:47:17 EST 2014
Bootloader             L1TC00011880

Comment 4

[Gabriele Svelto [:gsvelto]]

This is caused by L10n.get failing in the shared contacts code and preventing the panel from being properly populated. I'll post a patch shortly.

Comment 5

[Gabriele Svelto [:gsvelto]]

Actually there's two issues at work here. The first one is what I mentioned above and it's present in multiple elements but it's not what's causing the actual problem here so we can probably delay fixing it. The second part is that the code in contacts_buttons.js expect the fragment it's used upon to have at least one send-sms-to-email-button-* field:

https://github.com/mozilla-b2g/gaia/blob/095a9ce5f184de598d191f097bd47554e0e03530/shared/js/contacts/contacts_buttons.js#L167

This field however is present only in the details.html template:

https://github.com/mozilla-b2g/gaia/blob/f18d0fc0c12ab51f8a606c934a179266dcaf3e18/apps/communications/contacts/elements/details.html#L89

Not in the email_details.html template:

https://github.com/mozilla-b2g/gaia/blob/0953f7a4e071fbc6d1cbca691188a8fb34c4dae6/apps/communications/contacts/elements/email_details.html

I can either make the DOM in email_details.html the same as in details.html or simply skip the button in the JS code if it's not present. Doug, Francisco do you have some thoughts about this? Both solutions work for me, I just don't know which one to pick.

Comment 6

[Doug Sherk (:drs) (inactive)]

See the comment in email_details.html:
<!-- bug 1060080: taken from details.html, should be unified -->

Thus the solution here seems pretty clear to me. Let's get them back in sync again. Could you also check phone_details.html while you're here?

I'm clearing Francisco's ni as I don't think there's any need for more discussion here.

Comment 7

[GitHub Autolander Bot]

Attachment: [gaia] gabrielesvelto:bug-1155582-sync-phone-email-contact-details > mozilla-b2g:master

Comment 8

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8595203 [details] [review]
[gaia] gabrielesvelto:bug-1155582-sync-phone-email-contact-details > mozilla-b2g:master

The changes are straightforward: I've sync'd the HTML fragments with the full details.html file and added the missing l10n strings to the dialer. Note that those are not strictly required for fixing the issue here (without them we just get a bunch of warnings) so if we can't land l10n changes anymore on v2.2 those can be excluded from the uplift. I've looked into our landing instructions and I couldn't figure out if l10n changes are still possible or not. r? drs for the dialer and francisco for the contacts.

Comment 9

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8595203 [details] [review]
[gaia] gabrielesvelto:bug-1155582-sync-phone-email-contact-details > mozilla-b2g:master

LGTM, left a tiny comment on github.

Comment 10

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8595203 [details] [review]
[gaia] gabrielesvelto:bug-1155582-sync-phone-email-contact-details > mozilla-b2g:master

Looks good, thanks. Sorry about the delay. I wanted to make sure that re-creating the existing strings in another file is the correct way to do this.

Comment 11

[Gabriele Svelto [:gsvelto]]

Thanks to both, ready to land.

Comment 12

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/04b6d30f4502fe793cb56e2cccd30442d494dc4b

Comment 13

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8595203 [details] [review]
[gaia] gabrielesvelto:bug-1155582-sync-phone-email-contact-details > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 997547
[User impact] if declined: Call information of contacts with an e-mail address isn't shown
[Testing completed]: Tested on a device and on the emulator
[Risk to taking this patch] (and alternatives if risky): Practically no risk, this is just a matter of synchronizing two HTML files so that they contain all the elements the associated JS code expects.
[String changes made]: Added the composeEmail.ariaLabel, emailDetail.ariaLabel, phoneDetail.ariaLabel, send-sms-button.ariaLabel and separator strings to  apps/communications/dialer/locales/dialer.en-US.properties. This aren't strictly needed to make the patch work, leaving them out will affect the screen reader though.

Comment 14

[Francesco Lodolo [:flod]]

Just to clarify

composeEmail.ariaLabel    = Email
emailDetail.ariaLabel     = Email

The first is a verb, the second is a noun? Is it confusing even for English?

Comment 15

[Gabriele Svelto [:gsvelto]]

(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #14)
> Just to clarify
> 
> composeEmail.ariaLabel    = Email
> emailDetail.ariaLabel     = Email
> 
> The first is a verb, the second is a noun? Is it confusing even for English?

I've taken the strings unmodified from the contacts app, should I file a follow-up for improving them?

Comment 16

[Francesco Lodolo [:flod]]

(In reply to Gabriele Svelto [:gsvelto] from comment #15)
> I've taken the strings unmodified from the contacts app, should I file a
> follow-up for improving them?

I think it would make sense (not for 2.2 like this bug), especially if the first one means "Compose email".

Comment 17

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/d3402eb3800f7889f55b9a2eaa8040c651331a80

Comment 18

[Boon]

This isssue has been succsessfully verified on flame2.2&3.0
Reproduce rate: 0/5
Device:Flame 2.2[Verified]
Build ID               20150503002500
Gaia Revision          8d14361337e608c8cdf165ea5034db5eda23b618
Gaia Date              2015-05-01 18:23:46
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cb7cb6597c91
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150503.040203
Firmware Date          Sun May  3 04:02:15 EDT 2015
Bootloader             L1TC000118D0

Device:Flame 3.0[Verified]
Build ID               20150503160200
Gaia Revision          e18cce173840d6ff07fb6f1f0e0ffb58b99aab3e
Gaia Date              2015-05-02 04:27:01
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/dc5f85980a82
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150503.193941
Firmware Date          Sun May  3 19:39:52 EDT 2015
Bootloader             L1TC000118D0

Comment 19

[Shally Li（leave from Mozilla）]

Per comment 18, clear "verifyme".