Closed Bug 1155582 Opened 5 years ago Closed 5 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S11 (1may)
blocking-b2g 2.2+
Tracking Status
b2g-v2.1 --- unaffected
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: yue.zhou, Assigned: gsvelto)

Details

(Keywords: regression, Whiteboard: [planned-sprint])

Attachments

(3 files)

[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
Attached video Bug video: 345.mp4
[Blocking Requested - why for this release]: broken functionality
UX
https://mozilla.app.box.com/s/44utizl9oz4eupyu3fuu/1/2184700727/19414616103/1
blocking-b2g: --- → 2.2?
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
blocking-b2g: 2.2? → 2.2+
Assignee: nobody → gsvelto
Whiteboard: [planned-sprint]
Target Milestone: --- → 2.2 S11 (1may)
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.
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.
Flags: needinfo?(francisco)
Flags: needinfo?(drs)
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.
Flags: needinfo?(francisco)
Flags: needinfo?(drs)
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.
Attachment #8595203 - Flags: review?(francisco)
Attachment #8595203 - Flags: review?(drs)
Status: NEW → ASSIGNED
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.
Attachment #8595203 - Flags: review?(francisco) → review+
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.
Attachment #8595203 - Flags: review?(drs) → review+
Thanks to both, ready to land.
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
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.
Attachment #8595203 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8595203 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
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?
(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?
(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".
Flags: needinfo?(yue.zhou)
Keywords: verifyme
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
Status: RESOLVED → VERIFIED
Flags: needinfo?(yue.zhou)
Per comment 18, clear "verifyme".
Keywords: verifyme
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.