832824 - [Email] Contact name is not displayed when we receive a mail from existing saved contact

Status: RESOLVED DUPLICATE of bug 848266

(Firefox OS Graveyard :: Gaia::E-Mail, defect, P1)

Description

[Leo]

1. Title : Contact name is not displayed when we receive a mail from existing saved contact
2. Precondition : Email account has to be configured.Run contacts app and save one contact with email address
3. Tester's Action : Receive an email from the email address which is saved in contacts
4. Detailed Symptom (ENG.) : The email Cannot be displayed with the name saved in the contact
5. Expected :When recieved an email from the email address which is saved in the contact,the name saved in the contact should be displayed 
6.Reproducibility: Y
1)Frequency Rate : 100%
7. Gaia Revision: a03f7b532e9998e646d55f93a0fc03a04d7ca7d9

Comment 1

[Leo]

Attachment: Pointer to pull request

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 717091 [details]
Pointer to pull request

Review feedback provided on the pull request.  Clearing review flag until next rev of the patch.

Comment 3

[Leo]

@ Andrew,

I have replied with my points at pull request, please suggest me how to proceed further

Thanks.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

I thought about this more after reviewing and decided that now may be the right time to just have the MailAPI automatically resolve e-mail addresses to contacts before telling the view-slices about the message headers.  We have a bunch of functionality that wants to know if an address is a contact, etc., so I think it's probably best to do that now, especially since we're seeing high reflow costs, etc.

If it's okay with you, I'd like to quickly try and implement the contact lookup with caching in the back-end and the minor front-end change and then have you review it to make sure it does what you want and works like you want it to.  (I'm expecting the most complex part is the back-end unit tests, and since I think you want this for v1.1, it's probably fastest for me to whip that up.)  I can have that later today.

Context:
Bug 838011 is about our UI for adding e-mail addresses to contacts where we want to know if an e-mail address is already covered by a contact
Bug 840372 is about e-mail FPS rate
Bug 799707 is about showing contact photos on the right side of the contact list

Comment 5

[Leo]

Hi Andrew,
As you suggested, please do the contacts look up in the backend. at UI side we will verify and see if it meets our requirement. In case if it doesn't meet our requirement we can discuss again.

Thanks,
Leo

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

Will do as soon as possible.  Sorry about the delay; busy week and then travel/jet-lag.

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Leo, I've got a patch up for gaia-email-libs-and-more on bug 848266.  If you want to be able to see the changes in the gaia repo, check out the pull request from bug 799707 which demonstrates I think what you want here plus showing contact pictures:
https://github.com/mozilla-b2g/gaia/pull/8490

Comment 8

[Leo]

Andrew,
I will check it out.

Thanks
Leo

Comment 9

[Leo]

Since Mr.Andrew is worked on this issue , Resetting the Assignee.

Comment 10

[Leo]

Hi Andrew,

I am on latest gaia revision today. When I tried to apply the patch https://github.com/mozilla-b2g/gaia/pull/8490 it is giving errors.
=================================================================
error: while searching for:

    if (this.selectedMessages.length === 0)
      return;

    var dialog = msgNodes['delete-confirm'].cloneNode(true);
    var content = dialog.getElementsByTagName('p')[0];
    content.textContent = mozL10n.get('message-multiedit-delete-confirm',

error: patch failed: apps/email/js/message-cards.js:802
error: apps/email/js/message-cards.js: patch does not apply
Checking patch apps/email/style/message-cards.css...
Checking patch apps/email/js/ext/mailapi/activesync/configurator.js...
error: while searching for:
/* Copyright 2012 Mozilla Foundation
 *
 * Licensed under the Apache License, Version 2.0 (the "License");

error: patch failed: apps/email/js/ext/mailapi/activesync/configurator.js:1
error: apps/email/js/ext/mailapi/activesync/configurator.js: patch does not apply
Checking patch apps/email/js/ext/mailapi/composite/configurator.js...
error: while searching for:
/**
 * Make our TCPSocket implementation look like node's net library.
 *

error: patch failed: apps/email/js/ext/mailapi/composite/configurator.js:1
error: apps/email/js/ext/mailapi/composite/configurator.js: patch does not apply
Checking patch apps/email/js/ext/mailapi/fake/configurator.js...
error: while searching for:
/**
 * Implements a fake account type for UI testing/playing only.
 **/

error: patch failed: apps/email/js/ext/mailapi/fake/configurator.js:1
error: apps/email/js/ext/mailapi/fake/configurator.js: patch does not apply
Checking patch apps/email/js/ext/mailapi/same-frame-setup.js...
error: while searching for:
/**
 * Look like node's Buffer implementation as far as our current callers require
 * using typed arrays.  Derived from the node.js implementation as copied out of

error: patch failed: apps/email/js/ext/mailapi/same-frame-setup.js:1
error: apps/email/js/ext/mailapi/same-frame-setup.js: patch does not apply
==================================================================================
Please let me know how to check the above patch

Thanks
Leo

Comment 11

[Andrew Sutherland [:asuth] (he/him)]

I'm not sure I understand why you would need to apply a patch.  You can just check out the branch that the pull request comes from.

If you don't have my repo already as a remote:
  git remote add asutherland git://github.com/asutherland/gaia.git

Then fetch the state of my repo:
  git fetch asutherland

Then check out the branch
  git checkout asutherland/display-contact-pics

Comment 12

[Leo]

Hi Andrew, 
sorry for the wrong way of doing it.
I will check the way you mentioned.

Thanks,
Leo

Comment 13

[Joe Cheng [:jcheng] (please needinfo)]

is this a bug or a new feature request? thanks

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

It's not a regression, so let's call it a feature request.  Note that bug 848266 is where this problem would be fixed, and that is already leo+.

Comment 15

[Wayne Chang [:wchang]]

Do we need any work from the mail app once the API in bug 848266 lands?

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

If this bug 'just works' when bug 848266 lands, then we win, but otherwise this is a feature request and v1.1 is past Feature Complete so not blocking.

Comment 17

[Leo]

Attachment: Image for Contacts with Images.png

Image for Contacts with Images.png

Comment 18

[Leo]

Attachment: Image for Contact with just name.png

Image for Contact with just name.png

Comment 19

[Leo]

Hello Andrew,

I have verified this issue at the branch asutherland/display-contact-pics. 
Please find the attached images for the same.

Contact Name looks fine. Contact with Images UI layout is not proper.

Requesting you to land the Contact Name part in V1-Train.

Thanks

Comment 20

[Leo]

Attachment: Contact with Name

Comment 21

[Leo]

Attachment: Image attachment: Contact with Name and Picture

Comment 22

[Leo]

Attachment: Image attachment: Contact with Name

Comment 23

[Jim Porter (:squib)]

Is there anything left to do here? We have bug 799707 for contact pictures, and bug 848266 has hooked us up to the mozContacts database for the contact names. I suggest we either resolve this as FIXED or dupe it to bug 799707.

Comment 24

[Andrew Sutherland [:asuth] (he/him)]

Let's declare this a dupe of bug 848266 since we're doing the one-patch-per-bug thing.