Last Comment Bug 447927 - Additional Email field of address book entries not included in address autocompletion
: Additional Email field of address book entries not included in address autoco...
Status: RESOLVED FIXED
: fixed-seamonkey2.0.1, regression
Product: MailNews Core
Classification: Components
Component: Address Book (show other bugs)
: Trunk
: All All
: P4 normal with 11 votes (vote)
: Thunderbird 3.0rc1
Assigned To: Mark Banner (:standard8)
:
Mentors:
: 448049 468798 470589 498063 511691 513034 520829 524967 (view as bug list)
Depends on: 535528
Blocks: 360648
  Show dependency treegraph
 
Reported: 2008-07-24 22:29 PDT by James Teh [:Jamie]
Modified: 2009-12-28 15:16 PST (History)
38 users (show)
standard8: wanted‑thunderbird3+
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
The fix (16.28 KB, patch)
2009-10-05 05:52 PDT, Mark Banner (:standard8)
no flags Details | Diff | Splinter Review
The fix v2 (23.99 KB, patch)
2009-10-16 04:03 PDT, Mark Banner (:standard8)
neil: review+
mozilla: superreview+
standard8: approval‑thunderbird3+
Details | Diff | Splinter Review

Description James Teh [:Jamie] 2008-07-24 22:29:08 PDT
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: abcd@efgh.com
  Additional Email: ijkl@mnop.com
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 abcd@efgh.com 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 abcd@efgh.com is listed.
For ijkl (step 7), autocompletion does not provide any results.

Expected Results:  
ijkl@mnop.com 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) 2008-07-25 00:53:02 PDT
Confirming, missed this during the transition to the new toolkit interfaces.
Comment 2 Mark Banner (:standard8) 2008-07-26 00:10:39 PDT
*** Bug 448049 has been marked as a duplicate of this bug. ***
Comment 3 Mark Banner (:standard8) 2008-08-21 11:34:06 PDT
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, NI for questions) 2008-09-30 03:28:20 PDT
should be relnoted if it doesn't make the b1 train
Comment 5 Wayne Mery (:wsmwk, NI for questions) 2008-09-30 08:43:44 PDT
bad eyesight at 6am - this wasn't a blocker so relnote may not be warranted.
Comment 6 Mark Banner (:standard8) 2008-12-10 03:47:52 PST
*** Bug 468798 has been marked as a duplicate of this bug. ***
Comment 7 Mark Banner (:standard8) 2008-12-20 14:25:32 PST
*** Bug 470589 has been marked as a duplicate of this bug. ***
Comment 8 Armond Avanes 2008-12-20 23:25:31 PST
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 2009-05-07 06:29:46 PDT
(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) 2009-05-07 07:46:15 PDT
(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 2009-05-07 08:55:44 PDT
Isn't that also the better option in regard to allowing n email adresses per card (bug 118665)?
Comment 12 Mark Banner (:standard8) 2009-06-13 01:33:46 PDT
*** Bug 498063 has been marked as a duplicate of this bug. ***
Comment 13 yillyevski 2009-06-17 14:28:23 PDT
This bug is still not solved. I am using the latest Shredder nightly build
Comment 14 yillyevski 2009-06-24 13:23:47 PDT
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: john@athome.com and an additional email field john@atwork.com.

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 --> john@athome.com and an additional email field john@atwork.com)
-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 15 Mark Banner (:standard8) 2009-08-20 11:06:47 PDT
*** Bug 511691 has been marked as a duplicate of this bug. ***
Comment 16 Mark Banner (:standard8) 2009-08-27 11:01:36 PDT
*** Bug 513034 has been marked as a duplicate of this bug. ***
Comment 17 eldeh 2009-08-28 16:33:58 PDT
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 2009-09-25 20:09:32 PDT
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) 2009-10-05 05:52:09 PDT
Created attachment 404594 [details] [diff] [review]
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) 2009-10-05 06:05:52 PDT
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) 2009-10-05 07:08:22 PDT
(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 neil@parkwaycc.co.uk 2009-10-05 15:41:40 PDT
Wait, so if I have someone (let's call him Mark) with an primary address of bugzilla@standard8.plus.com and a secondary address of mark.banner@gmail.com, 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) 2009-10-06 00:03:29 PDT
(In reply to comment #22)
> Wait, so if I have someone (let's call him Mark) with an primary address of
> bugzilla@standard8.plus.com and a secondary address of mark.banner@gmail.com,
> 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 neil@parkwaycc.co.uk 2009-10-06 02:35:44 PDT
(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 25 Mark Banner (:standard8) 2009-10-06 11:31:52 PDT
*** Bug 520829 has been marked as a duplicate of this bug. ***
Comment 26 Mark Banner (:standard8) 2009-10-07 06:14:42 PDT
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) 2009-10-16 04:03:52 PDT
Created attachment 406660 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2009-10-16 13:59:23 PDT
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 2009-10-16 14:40:52 PDT
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) 2009-10-19 05:26:17 PDT
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) 2009-10-19 05:31:51 PDT
Checked in: http://hg.mozilla.org/comm-central/rev/67c02f46eab4
Comment 32 Wayne Mery (:wsmwk, NI for questions) 2009-10-30 12:11:26 PDT
*** Bug 524967 has been marked as a duplicate of this bug. ***
Comment 33 Rolf Pedersen 2009-12-14 07:01:28 PST
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) 2009-12-16 11:13:42 PST
(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 2009-12-16 11:57:02 PST
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 2009-12-16 12:34:15 PST
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) 2009-12-17 13:48:07 PST
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, NI for questions) 2009-12-17 14:10:20 PST
(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) 2009-12-17 14:17:10 PST
(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.

Note You need to log in before you can comment on or make changes to this bug.