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
242.21 KB, text/plain
8.13 MB, video/mp4
46 bytes, text/x-github-pull-request
|Details | Review|
[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
[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
Assignee: nobody → gsvelto
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.
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 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 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.
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/04b6d30f4502fe793cb56e2cccd30442d494dc4b
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".
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
You need to log in before you can comment on or make changes to this bug.