543701 - Email address of secondary identities not recognized as "You" address book contact

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Andrea Soster]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2) Gecko/20100115 Epiphany/2.22 Firefox/3.6
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.7) Gecko/20100127 Lightning/1.0b2pre Thunderbird/3.0.1

If you add an identity (with a different mail address that the main one) mails received for that mail address are not recognized as 'to "You"' is this expected?

Reproducible: Always

Steps to Reproduce:
1.Create a mail account that receive mails for 2 or more mail address
2.create an identity for the second mail address
3.receive an email for the second mail address
Actual Results:  
The "to" address on the message header is not marked as "You"

Expected Results:  
The "to" address on the message header is marked as "You"

Comment 1

[Jim Porter (:squib)]

I see this too, confirming it. However, fixing this would cause a lot of confusion without the fix from bug 478466.

Comment 2

[Jim Porter (:squib)]

I've looked at this in a little more detail, and it only seems to happen when you have two identities whose domain names are the same.

Comment 3

[Jim Porter (:squib)]

Attachment: Simple patch, no tests yet

Comment 2 may be inaccurate. The code for checking identities was pretty strange. The attached patch is much simpler and should resolve the issue. No tests yet, though.

Comment 4

[Jim Porter (:squib)]

Can anyone who has experienced this tell me what identities they have in Thunderbird? (You can anonymize the addresses of course.) I'd really like to get this fixed, but I'm sure it'll require a test to get checked in, and I don't really know what to test for.

Comment 5

[Jim Porter (:squib)]

Comment on attachment 471421 [details] [diff] [review]
Simple patch, no tests yet

Blake, could you take a look at this? I think this change is sane, but I'm at a loss as to how to test it, and how big a test I really need...

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 471421 [details] [diff] [review]
Simple patch, no tests yet

Couldn't we get essentially the same behaviour with a much smaller change by passing aEmailAddress in to getBestIdentity as the second "optionalHint" parameter?

Thanks,
Blake.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

(P.S. Sorry about the delay in getting back to you on this.  I'll try to do better in the future.)

Comment 8

[Jim Porter (:squib)]

I think that would work, but it seems a little roundabout, and in the back of my head, I worry about the performance, since getBestIdentity does roughly twice as much work as IsIdentity. Maybe that's not a big deal in practice, though. (Of course, it might become a bigger deal after bug 591730 and/or bug 312821 are fixed.)

Comment 9

[Jim Porter (:squib)]

Attachment: The simplest possible patch

This is about as simple as it gets. It probably works (I never did quite figure out a comprehensive list of causes for this problem), and it'll get tested by way of bug 527550.

Much as I'd like an IsIdentity function (it just seems like a sensible thing), it's probably not a big enough deal to worry about, since at best, it would only get used here and in some logic for Reply All.

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 483612 [details] [diff] [review]
The simplest possible patch

(Assigning to me for review, cause otherwise I'll never get around to reviewing it.)

Comment 11

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 483612 [details] [diff] [review]
The simplest possible patch

Apparently it'll take me ages to get around to reviewing it even if I _do_ ask myself for review.  :P

Anyways, I like it.  It seems simple, and reasonably correct.

Thanks,
Blake.

Comment 12

[Blake Winton (:bwinton) (:☕️)]

Pushed as http://hg.mozilla.org/comm-central/rev/8e7458f809d5