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

RESOLVED FIXED in Thunderbird 3.0rc1

Status

MailNews Core
Address Book
P4
normal
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Jamie, Assigned: standard8)

Tracking

({fixed-seamonkey2.0.1, regression})

Trunk
Thunderbird 3.0rc1
fixed-seamonkey2.0.1, regression
Dependency tree / graph
Bug Flags:
wanted-thunderbird3 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
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.
(Reporter)

Updated

9 years ago
Version: unspecified → Trunk
(Assignee)

Comment 1

9 years ago
Confirming, missed this during the transition to the new toolkit interfaces.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-thunderbird3+
Keywords: regression
(Assignee)

Updated

9 years ago
Assignee: nobody → bugzilla
Priority: -- → P2
(Assignee)

Updated

9 years ago
Duplicate of this bug: 448049
(Assignee)

Comment 3

9 years ago
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)
Flags: blocking-thunderbird3+ → wanted-thunderbird3+
Target Milestone: --- → Thunderbird 3.0b1

Updated

9 years ago
Priority: P2 → P4
should be relnoted if it doesn't make the b1 train
Keywords: relnote
bad eyesight at 6am - this wasn't a blocker so relnote may not be warranted.
(Assignee)

Updated

9 years ago
Target Milestone: Thunderbird 3.0a3 → Thunderbird 3.0b1
(Assignee)

Updated

9 years ago
Duplicate of this bug: 468798
(Assignee)

Updated

9 years ago
Duplicate of this bug: 470589

Comment 8

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

Updated

9 years ago
OS: Windows XP → All
Hardware: x86 → All
Target Milestone: Thunderbird 3.0b1 → Thunderbird 3.0b2

Comment 9

8 years ago
(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?
Blocks: 360648
(Assignee)

Comment 10

8 years ago
(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.
Depends on: 455714

Comment 11

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

Updated

8 years ago
Duplicate of this bug: 498063

Comment 13

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

Comment 14

8 years ago
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
(Assignee)

Updated

8 years ago
Duplicate of this bug: 511691
(Assignee)

Updated

8 years ago
Duplicate of this bug: 513034

Comment 17

8 years ago
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

8 years ago
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...)
(Assignee)

Comment 19

8 years ago
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.
Attachment #404594 - Flags: superreview?(bienvenu)
Attachment #404594 - Flags: review?(neil)
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED
Component: Message Compose Window → Address Book
No longer depends on: 455714
Product: Thunderbird → MailNews Core
QA Contact: message-compose → address-book
Target Milestone: Thunderbird 3.0b2 → Thunderbird 3.0rc1

Comment 20

8 years ago
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)?
(Assignee)

Comment 21

8 years ago
(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.
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...
(Assignee)

Comment 23

8 years ago
(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@....>
(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.
(Assignee)

Updated

8 years ago
Duplicate of this bug: 520829
(Assignee)

Updated

8 years ago
Attachment #404594 - Attachment is obsolete: true
Attachment #404594 - Flags: superreview?(bienvenu)
Attachment #404594 - Flags: review?(neil)
(Assignee)

Comment 26

8 years ago
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.
(Assignee)

Comment 27

8 years ago
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.
Attachment #406660 - Flags: superreview?(bienvenu)
Attachment #406660 - Flags: review?(neil)
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?
Attachment #406660 - Flags: review?(neil) → review+

Comment 29

8 years ago
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.
Attachment #406660 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 30

8 years ago
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.
Attachment #406660 - Flags: approval-thunderbird3+
(Assignee)

Comment 31

8 years ago
Checked in: http://hg.mozilla.org/comm-central/rev/67c02f46eab4
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: relnote
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Flags: in-testsuite+
Duplicate of this bug: 524967

Updated

8 years ago
Keywords: fixed-seamonkey2.0.1

Comment 33

8 years ago
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.
(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

8 years ago
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

8 years ago
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.
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?
(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.
(Assignee)

Comment 39

8 years ago
(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.

Updated

8 years ago
Depends on: 535528
You need to log in before you can comment on or make changes to this bug.