447927 - Additional Email field of address book entries not included in address autocompletion

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect, P4)

Description

[James Teh [:Jamie]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a1pre) Gecko/2008072211 Minefield/3.1a1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.2pre) Gecko/2008072303 Shredder/3.0a2pre

When composing a message and using address autocompletion to select a recipient, addresses listed in the Additional Email field of address book entries are not included in the list of possible autocompletion addresses.

Reproducible: Always

Steps to Reproduce:
0. Ensure that address autocompletion is enabled.
1. Add an entry to the Address Book with the following details:
Name:
  First: abcd
Internet:
  Email: [e-mail]
  Additional Email: [e-mail]
2. Open a new message composition window.
3. In the To: field, enter "zyxw" (without the quotes). Examine the list of possible autocompletion addresses.
4. Clear the content of the To: field.
5. (sanity check) In the To: field, enter "abcd" (without the quotes). Observe that [e-mail] is the only entry in the list of possible autocompletion addresses.
6. Clear the content of the To: field.
7. In the To: field, enter "ijkl" (without the quotes). Examine the list of possible autocompletion addresses.
Actual Results:  
For zyxw (step 3), only [e-mail] is listed.
For ijkl (step 7), autocompletion does not provide any results.

Expected Results:  
[e-mail] should be included in the list of autocompletion addresses for both abcd (step 3) and ijkl (step 7).

Obviously, this test assumes that there are no other matches for abcd, efgh and ijkl in your Address Book.

This is a regression. I'm not sure exactly when this started failing, but it was some time within the last month or two.

Comment 1

[Mark Banner (:standard8)]

Confirming, missed this during the transition to the new toolkit interfaces.

Comment 3

[Mark Banner (:standard8)]

Retriaging according to new policy for flags.
https://wiki.mozilla.org/Thunderbird:Release_Driving
(bugs marked wanted- don't indicate we wouldn't accept patches, but that they're not going to be the focus for release drivers)

Comment 4

[Wayne Mery (:wsmwk)]

should be relnoted if it doesn't make the b1 train

Comment 5

[Wayne Mery (:wsmwk)]

bad eyesight at 6am - this wasn't a blocker so relnote may not be warranted.

Comment 8

[Armond Avanes]

This bug has been marked for 3.0b1 but it's still open! Could we expect it to be fixed before going final?

Comment 9

[Steffen Wilberg]

(In reply to comment #1)
> Confirming, missed this during the transition to the new toolkit interfaces.
Do you mean bug 370306? How hard is it to fix this?

Comment 10

[Mark Banner (:standard8)]

(In reply to comment #9)
> (In reply to comment #1)
> > Confirming, missed this during the transition to the new toolkit interfaces.
> Do you mean bug 370306? How hard is it to fix this?

Yes. Its all js code (nsAbAutoCompleteSearch.js), probably wouldn't be too difficult but there is at least one tricky situations:

- We generally want to add an entry for both primary & secondary, except when the primary email is the part matching the string and the secondary doesn't.

Doing a straight check for this will hurt perf if there are lots of matches, and the existing search mechanism doesn't help us here.

I'm starting to think it may be better to fix bug 455714 first and get ourselves a fast search function returning multiple cards, then:

- Drop primary email from the current search
- Add a two more searches: one for primary, the other for secondary
- Merge the results & drop duplicates.

Not ideal, but it would largely get us around the perf issues and mean we don't have to do extra comparisons working out what matched within a card.

Comment 11

[Steffen Wilberg]

Isn't that also the better option in regard to allowing n email adresses per card (bug 118665)?

Comment 13

[yillyevski]

This bug is still not solved. I am using the latest Shredder nightly build

Comment 14

[yillyevski]

Hello!

This bug is nagging me all the time.
If you write an email to a person from the address book who has two emails.
The first email: [e-mail] and an additional email field [e-mail].

The additional email field is not showing up when you write email.
I am using the latest Shredder 3.0b3

Reproducing the bug:
-You need Multiple e-mail addresses per address book entry (John --> [e-mail] and an additional email field [e-mail])
-For example, start typing in "John"
-the autocomplete dropdown box that displays emails is showing only the first email, the additional email is skipped

Comment 17

[eldeh]

Still open in the nightly.

Performance is an issue, but at the moment, the performance bottle neck is to enter a lot of email adresses by hand.

Comment 18

[wjtriffo]

any comments on when this will be addressed ?

I am trying to avoid switching to Mac Mail b/c of lack of addressbook integration in TB 2 (read-only), but switching to a 3beta with this problem would be crippling - I have tons of contacts with multiple emails. And I often have to pick among them - so if I am not able to even see the second email (I depend a lot on autocomplete in TB2, I rarely have ability to do verbatim memory recall of all my addresses), that is a big deal-breaker.

After having an iPhone for quite a while, not being able to sync with TB, and not being able to write to the Mac Addressbook with TB2 (which has functional autocomplete) is pressuring me to switch clients to Mail (gasp!). If this feature were working, I would be able to use TB3's integration with Mac addressbook and avoid the switch. But I've been checking this bug for months and it does not seem to get addressed. (I can't be the only guy out there with an iPhone who prefers TB and depends upon the autocomplete...)

Comment 19

[Mark Banner (:standard8)]

Attachment: The fix

This adds matching against the additional email back into the address book.

Notes:

- Unfortunately we don't have an ideal function in the address book to search, so we have to search the address book with primary and additional included and then duplicate where necessary so that we get two results listed where a name matches but the email addresses don't.

- As we now have the possibility that we have a result for just a primary or additional email, we have to keep track of that fact - so we use the new interface function getIsPrimaryMatchAt.

- I altered the test file that test_nsAbAutoCompleteSearch1.js was using so that it didn't have an additional email address in its card, rather than try and unbreak other tests I created a new file (autocomplete2.mab).

- This patch passes the existing tests and adds a few more for matching the additional email.

Comment 20

[Ben Bucksch (:BenB)]

Mark, thanks for fixing this.
Wouldn't it be better to structure the API so that it would still work for bug 118665 (more than 2 email addresses per card)?

Comment 21

[Mark Banner (:standard8)]

(In reply to comment #20)
> Mark, thanks for fixing this.
> Wouldn't it be better to structure the API so that it would still work for bug
> 118665 (more than 2 email addresses per card)?

Whilst that would be nice, implementing that in autocomplete would require significant rework anyway. Additionally, we'd probably end up killing SecondEmail and doing it a different way. Given that we're going to focus on contacts in more detail after Thunderbird 3 I don't see any advantage of taking longer to work an unusable more flexible API now that could likely be obsolete before we use it.

Comment 22

[[e-mail]]

Wait, so if I have someone (let's call him Mark) with an primary address of [e-mail] and a secondary address of [e-mail], and I then autocomplete for "mark" then I will never see the bugzilla address, even though "mark" matches his first name...

Comment 23

[Mark Banner (:standard8)]

(In reply to comment #22)
> Wait, so if I have someone (let's call him Mark) with an primary address of
> [e-mail] and a secondary address of [e-mail],
> and I then autocomplete for "mark" then I will never see the bugzilla address,
> even though "mark" matches his first name...

Err, no, you should see:

Mark <bugzilla@....>
Mark <mark.banner@....>

Comment 24

[[e-mail]]

(In reply to comment #23)
> Err, no, you should see:
> 
> Mark <bugzilla@....>
> Mark <mark.banner@....>
But the code will look at the email addresses, notice that the second one matches, sets wasAdded to true, and ignores the first address.

Comment 26

[Mark Banner (:standard8)]

Comment on attachment 404594 [details] [diff] [review]
The fix

Neil's right. I've got a solution in progress but need to work out some missing features in the address book. Shouldn't take too long hopefully.

Comment 27

[Mark Banner (:standard8)]

Attachment: The fix v2

Second attempt. This goes along similar lines to the first patch. However I've now split the main search into two.

The first search does matching against the names and other fields. The second search matches against the email addresses. This gives us the extra flexibility we need to add both email addresses on a card where something other than the email address matches, and to add the appropriate email addresses where the name fields don't match.

Whilst this means two searches through each directory which will hurt perf a bit for large ABs, I think it is the simplest way we have to do this in the current address book without a big rewrite.

The new test has also been updated to include Neil's case he mentioned before and that passes as well.

Comment 28

[[e-mail]]

Comment on attachment 406660 [details] [diff] [review]
The fix v2

>-        lastName.lastIndexOf(fullString, 0) == 0 ||
>-        card.primaryEmail.toLocaleLowerCase().lastIndexOf(fullString, 0) == 0)
>+        lastName.lastIndexOf(fullString, 0) == 0)
>+      return true;
>+
>+    if (emailToUse.toLocaleLowerCase()
>+                  .lastIndexOf(fullString, 0) == 0)
Why not just change card.primaryEmail to emailToUse?

Comment 29

[David :Bienvenu]

Comment on attachment 406660 [details] [diff] [review]
The fix v2

typo:   *                        for duplicates agianst.

_searchWithinEmails starts off using var and then uses let.

_checkEntry comment should have @return since it returns a value.

Comment 30

[Mark Banner (:standard8)]

Comment on attachment 406660 [details] [diff] [review]
The fix v2

a=Standard8 on the basis that this restores functionality compared to TB 2 and has unit tests with good coverage.

Comment 31

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/67c02f46eab4

Comment 33

[Rolf Pedersen]

I see the initially-reported behavior on Seamonkey v.2.0 x86_64.  If a card in Address Book has an additional email (the highest level of complexity that exists for me), only the primary address is displayed in the auto-completion drop-down.  Can/will the fix be applied to Seamonkey mail?  Thanks.

Comment 34

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #33)
> I see the initially-reported behavior on Seamonkey v.2.0 x86_64. (...)
> Can/will the fix be applied to Seamonkey mail?

This should be fixed in SeaMonkey 2.0.1 which has just been released. If you find any issues regarding what has been implemented here, please file a new bug. Thanks.

Comment 35

[Rolf Pedersen]

Just to confirm it's working for all my multiple-email-address-book entries on this morning's local build of 2.0.1 from release sources. :) 
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.6) Gecko/20091216 SeaMonkey/2.0.1
Thanks.

Comment 36

[Michael Graubart]

Just to confirm that with SM 2.0.1 as installed by automatic update from SM 2.0, and on a Mac G4 (PPC) with OS X 10.4.11, it is also fixed.

Comment 37

[Jens Hatlak (:InvisibleSmiley)]

Bug 535528, which is about a crash newly introduced with SM 2.0.1 and happening on autocomplete, seems to be related to the Outlook Express AB. That bug might have been introduced through the fix for this one. Does a similar bug exist for TB?

Comment 38

[Wayne Mery (:wsmwk)]

(In reply to comment #37)
> Bug 535528 ... might have been introduced through the fix for this one. Does a similar bug exist for TB?

there are currently no recent crash bugs filed for thunderbird address autocomplete. which is not to say that such a bug does exist ... a stack and sig for Bug 535528 would help in that regard.

Comment 39

[Mark Banner (:standard8)]

(In reply to comment #37)
> Bug 535528, which is about a crash newly introduced with SM 2.0.1 and happening
> on autocomplete, seems to be related to the Outlook Express AB. That bug might
> have been introduced through the fix for this one. Does a similar bug exist for
> TB?

Thunderbird doesn't enable the outlook express AB by default like SeaMonkey 2.0 does (because it requires outlook express to be default mail client iirc), so unless users know how to enable it (which is on mozillazine), it is unlikely that they would experience an issue.

If this bug did introduce an issue, then it only exposed it - the functionality that this touched was generic to all address books.