Closed Bug 889410 Opened 11 years ago Closed 10 years ago

Wrong identity chosen when replying to my own sent e-mails

Categories

(SeaMonkey :: MailNews: Composition, defect)

SeaMonkey 2.19 Branch
x86_64
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.26

People

(Reporter: xfox.mozilla, Assigned: mz+bugzilla)

Details

Attachments

(1 file, 3 obsolete files)

I noticed a strange behaviour when replying to my own sent e-mails stored in the Sent folder.
I have an IMAP account with a default identity <myname@mac.com> and a second identity <myname.mozilla@me.com>.
When I reply to an e-mail message I sent from my default identity <myname@mac.com> to an address such as <myfriend@me.com>, the identity automatically picked by the compose window and used for the From: field is <myname.mozilla@me.com>.
IMO that's wrong, it should be the default identity <myname@mac.com>.

I reproduced this issue with SeaMonkey 2.18b4 and 2.19b2 on Mac OS X 10.7.5.
I searched Bugzilla a lot and it seems bug 397975 to me but that bug is marked RESOLVED FIXED… could it be that the fix landed only for Thunderbird?
> I searched Bugzilla a lot and it seems bug 397975 to me but that bug is
> marked RESOLVED FIXED… could it be that the fix landed only for Thunderbird?

Yeah, that was a thunderbird-only bug.
http://www.cs.iptcom.net/debug/Mail.7z
Test if this build behave in way you want and there are no regressions while general use.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(xfox.mozilla)
Attached patch Simple copy-paste patch (obsolete) — Splinter Review
Aligned with current TB source code
Attachment #811564 - Flags: review?(mnyromyr)
Possibly related to (or duplicate of) bug 608465?
Comment on attachment 811564 [details] [diff] [review]
Simple copy-paste patch

Seems Mnyromyr is busy...
Attachment #811564 - Flags: review?(neil)
Attachment #811564 - Flags: review?(iann_bugzilla)
(In reply to Phoenix from comment #2)
> http://www.cs.iptcom.net/debug/Mail.7z
> Test if this build behave in way you want and there are no regressions while
> general use.

I'm sorry I'm so late with this.
I've downloaded the 7z archive but both SuffIt Destinations 15.0.7 and ForkLift 2.5.7[2] fail to extract it. StuffIt simply stalls but ForkLift tells me "Wrong checksum". The MD5 hash of the Mail.7z file I downloaded (two times) is 5cc8495ab19502830fe614f923e3211a.

[1] <http://www.stuffit.com/mac-stuffit.html>
[2] <https://itunes.apple.com/app/forklift-file-manager-ftp/id412448059?mt=12>
Flags: needinfo?(xfox.mozilla)
Comment on attachment 811564 [details] [diff] [review]
Simple copy-paste patch

That archive was created with original 7z, don't know why those programs can't open it.
Anyway, that build is not working one, I found two minor mistakes in my patch, but after fixing them Reply function still doesn't working, I'll try too look on in closer on weekend...
Attachment #811564 - Flags: review?(neil)
Attachment #811564 - Flags: review?(mnyromyr)
Attachment #811564 - Flags: review?(iann_bugzilla)
(In reply to Phoenix from comment #7)
> That archive was created with original 7z, don't know why those programs
> can't open it.

I've managed to open it with another application (Keka for OS X), thanks.

> Anyway, that build is not working one, I found two minor mistakes in my
> patch, but after fixing them Reply function still doesn't working, I'll try
> too look on in closer on weekend...

However I can now see that you packaged a Windows build but I'm on Mac and I have no access to Windows boxes for testing. On another thought, I only tested this bug on Mac but if you reproduced this bug on Windows, too, maybe you should change the platform value from Mac OS X to all.
Attached patch Simple copy-paste patch v2 (obsolete) — Splinter Review
(In reply to Andrea Govoni from comment #8)
> However I can now see that you packaged a Windows build but I'm on Mac and I
> have no access to Windows boxes for testing. On another thought, I only
> tested this bug on Mac but if you reproduced this bug on Windows, too, maybe
> you should change the platform value from Mac OS X to all.
I don't have any multi-identity account, so can't test patch by myself, maybe Stefan can make Mac test build for you, or you need to find Windows box to test mine build (I will upload it later, if needed).

Attaching fixed patch, setting two reviewers whoever get to this patch first.
Assignee: nobody → pppx
Attachment #811564 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #831136 - Flags: review?(mnyromyr)
Attachment #831136 - Flags: review?(iann_bugzilla)
Flags: needinfo?(stefanh)
Comment on attachment 831136 [details] [diff] [review]
Simple copy-paste patch v2

>+  // If we have more than one identity and a hint to help us pick one.
>+  if (identityCount > 1 && optionalHint) {
>+    // Normalize case on the optional hint to improve our chances of
>+    // finding a match.
>+    optionalHint = optionalHint.toLowerCase();
This line is not required, as it is done again below.
>+    let hints = optionalHint.toLowerCase().split(",");

>+  // Still no matches? Give up and pick the first one.
This comment is not 100% correct, as the identityCount could be 1.

>+  return identities.queryElementAt(0, Components.interfaces.nsIMsgIdentity);

r=me with those fixed.
Attachment #831136 - Flags: review?(mnyromyr)
Attachment #831136 - Flags: review?(iann_bugzilla)
Attachment #831136 - Flags: review+
Attached patch Simple copy patch v3 (obsolete) — Splinter Review
Fixed review comments, carrying r+
Attachment #831136 - Attachment is obsolete: true
Attachment #8351690 - Flags: review+
Flags: needinfo?(stefanh)
Keywords: checkin-needed
(In reply to Phoenix from comment #11)
> Created attachment 8351690 [details] [diff] [review]
> Simple copy patch v3
> 
> Fixed review comments, carrying r+

I would have preferred a fixed comment rather than no comment.
Something like
// Return only found identity or pick the first one from list if no matches found.
?
(In reply to Phoenix from comment #13)
> Something like
> // Return only found identity or pick the first one from list if no matches
> found.
> ?

Yes, sounds good.
Done
Attachment #8351690 - Attachment is obsolete: true
Attachment #8354981 - Flags: review+
https://hg.mozilla.org/comm-central/rev/7b60380cdd8f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.26
You need to log in before you can comment on or make changes to this bug.