Closed Bug 424098 Opened 16 years ago Closed 16 years ago

One mail identity that is a substring of another mail identity will make thunderbird fail to idenify the sender of a reply or forward mail

Categories

(Thunderbird :: Account Manager, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3

People

(Reporter: touraine, Assigned: touraine)

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.12) Gecko/20080222 Firefox/2.0.0.12
Build Identifier: 2.0.0.12

If the user has two identity : aaa@bbb.ccc and ddd.aaa@bbb.ccc.
I.e.: the first identity is a substring of the second one.
Whenever the user want to forward or reply to a mail that has been sent to the second address (ddd.aaa@bbb.ccc), the first address (aaa@bbb.ccc) will also match the search and will be use as the default identity.

Reproducible: Always

Steps to Reproduce:
1.Create two identities (for example : aaa@bbb.ccc and ddd.aaa@bbb.ccc) such that aaa@bbb.ccc is the first in the list of identities.
2.Send a mail to ddd.aaa@bbb.ccc
3.Try to reply to this mail and the From field will be aaa@bbb.ccc.



Actually, I tracked the bug and managed to find that the function GetBestIdentity() of the file mozilla/mail/base/content/mailCommands.js was trying to look for the first identity that matches the optionalHint, that is the concatenation of To: and Cc: fields of the original mail.
I have managed to solve this bug on my installation :
diff -ru mozilla/mail/base/content/mailCommands.js mozilla/mail/base/content/mailCommands.js
--- mozilla/mail/base/content/mailCommands.js	2007-03-09 22:16:44.000000000 +0100
+++ mozilla/mail/base/content/mailCommands.js	2008-03-20 12:00:22.159234395 +0100
@@ -105,11 +105,17 @@
       // iterate over all of the identities
       var tempID;
 
+      var stringLength = 0;
       for (id = 0; id < identitiesCount; ++id) {
         tempID = identities.GetElementAt(id).QueryInterface(Components.interfaces.nsIMsgIdentity);
+	// dump (optionalHint + " ?= " + tempID.email + "\n");
         if (optionalHint.indexOf(tempID.email.toLowerCase()) >= 0) {
-          identity = tempID;
-          break;
+	  // Be carefull the user can have several adresses with the same postfix : aaa.bbb@ccc.ddd and bbb@ccc.ddd
+	  // So we get the longest identity founded
+	  if (stringLength < tempID.email.length) {
+            identity = tempID;
+            stringLength = tempID.email.length;
+	  }
         }
       }
 

Conversely to the previous version, I don't stop on the first match, but I'm looking for the match that has the longest answer (I use the var stringLength). Thus, I'm sure that if two identites matches, that is the most relevant one that is chosen.
Attached patch Patch to solve the bug 424098 (obsolete) — Splinter Review
Conversely to the previous version, I don't stop on the first match, but I'm
looking for the match that has the longest answer (I use the var stringLength).
Thus, I'm sure that if two identites matches, that is the most relevant one
that is chosen.
What exactly is the optionalHint there?

If it's just the e-mail, and possibly some trailing params, wouldn't it be easier to just test for optionalHint.indexOf(tempID.email.toLowerCase()) == 0 instead of optionalHint.indexOf(tempID.email.toLowerCase()) >= 0
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → touraine
The OptionalHint seems to be a kind of concatenation of the To: and Cc: fields.
It is not just the mail, but can also contain the full name with the email inside <> ...
So, this solution is not possible.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
FIXED is only used when a patch fixed some bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Sorry, I didn't know ...
But I submitted a patch to solve this problem (see comment #1). So, what is the process for this patch to be validated ?
See <http://developer.mozilla.org/en/docs/Getting_your_patch_in_the_tree>

Please fix a few things with the patch, and then ask for review.
 - we don't use tabs, convert them to spaces instead
 - remove the uncommented dump statements
 - keep line lenghts at 80 chars
 - change the var name of stringLength to something more describing, like lengthOfLongestMatchingEmail
Here is the correct patch considering the remarks of Magnus Melin.

So could you please review it and include it inside the main trunk ?

Regards,
Damien Touraine
Attachment #310743 - Attachment is obsolete: true
Comment on attachment 316189 [details] [diff] [review]
Correct patch to submit for reviewing ...

This patch is reversed, can you make a new one? 

Also fix spelling, should be "careful, ". Fix the comment sentence "So we get the longest identity founded" to be "Make sure we get the longest match."

Didn't notice earlier, but while you're at it, I think 

tempID.email.length > lengthOfLongestMatchingEmail

is clearer in this context than

lengthOfLongestMatchingEmail < tempID.email.length

Other than that, I think it looks good!
Reverse the patch and correct the cosmetic suggestions ....
Attachment #316189 - Attachment is obsolete: true
Comment on attachment 316201 [details] [diff] [review]
Mistake correcion inside the previous patch

You dropped the comma and dot ;)
Anyway, r=mkmelin with that fixed. You still need superreview for the /mailnews part. 

To ask for superreview, go to the attachment details and set superreview? to a suitable super reviewer. neil@httl.net perhaps?
Attachment #316201 - Attachment is patch: true
Attachment #316201 - Attachment mime type: application/octet-stream → text/plain
Attachment #316201 - Flags: review+
Attachment #316201 - Flags: superreview?(neil)
Attachment #316201 - Flags: superreview?(neil) → superreview+
Fix checked in with the comment fixed. Thx for the patch, Damien!

Checking in mailnews/base/resources/content/mailCommands.js;
/cvsroot/mozilla/mailnews/base/resources/content/mailCommands.js,v  <--  mailCommands.js
new revision: 1.111; previous revision: 1.110
done
Checking in mail/base/content/mailCommands.js;
/cvsroot/mozilla/mail/base/content/mailCommands.js,v  <--  mailCommands.js
new revision: 1.44; previous revision: 1.43
done

->FIXED
Status: REOPENED → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: