Closed Bug 99217 Opened 23 years ago Closed 22 years ago

Edit msg in draft/template moves msg from local to imap folder

Categories

(MailNews Core :: Composition, defect, P3)

x86
Windows 98

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.3alpha

People

(Reporter: sheelar, Assigned: bugzilla)

References

Details

(Whiteboard: have fix)

Attachments

(1 file, 3 obsolete files)

branch build: 2001-09-07-06 I noticed this while running basic functional on the above branch build. When you edit an existing message or saved message in drafts or templates the message moves from the local draft/template folder to the imap template/draft folder. steps: -create a new profile with imap as 1st account and pop as second account -Make sure the 1st account imap copies and folders for drafts and templates points to the drafts/templates on that imap account. -Change the copies and folders of the 2nd account to place the saved draft/template message to local folder draft/template respectively. -compose a new message from the 2nd account -save as draft (file|save as |draft, or toolbar button save) -You will see the message in the local folder drafts -select this message -click on the edit draft button -make changes to this message -save again Actual: The changes are saved and the message from the local folder is moved to the first account imap drafts folder. Expected: The changes are saved to the existing message and that it still resides in the drafts local folder This behavior is also seen when doing the same with templates.
Adding keywords and nominating this bug for emojo.
Keywords: nsbranch, regression
Blocks: 99230
this sounds kind of nasty. Not sure I would stop ship eMojo for it though. nsBranch- now but sending to Cavin for investigation. If he thinks the fix looks easy we'll move this to nsBranch+ status.
Keywords: nsbranchnsbranch-
Target Milestone: --- → mozilla0.9.5
Was this supposed to be reassigned to Cavin as well?
I was able to reproduce the problem using the 6.1 RTM build (20010726) as well so I guess the problem has been there for a while. I didn't have to follow the exact steps as described by Sheela to reproduce the problem, however, and here is what I did: 1. Set up two accounts and folder settings exactly like Sheela described (ie, one IMAP and one POP3). 2. Then I opened the Drafts folder under account Local Folders (ie, neither IMAP nor POP3 account) and launched a new compose window. At this point the Drafts folder had no msgs (since I just created a new profile so everything was empty). 3. Typed in something in the compose window and hit Save. Still no msgs showed up in the Drafts folder I just opened (it should if the program worked). 4. Opened the Drafts folder associated with the IMAP account and the draft msg I just saved showed up there! From here on, every time I hit the Save button in the compose window the new (draft) msgs were saved to the Drafts folder associated with the IMAP account and the one under Local Folders showed no msgs at all. So my guess is that we did not associate the right folder or account info with the compose window launched from the Local Folders account because looks like it always points to other account's set folder (in this case, it's the 1st IMAP account I created). I'm going to start debugging the problem now. JF, if you have any idea about this please let me know.
After looking this further more I realized that Local Folders account does not even have settings for Sent, Drafts and Templates folders and maybe that's why it's linked to those defined in the 1st account (ie, id1). In other words, when you save a draft msg under Local Folders account, where would you put the msg given the fact that no Drafts folder is defined for Local Folders? So I think this works as designed but I need JF or someone to confirm that. In summary, if you: 1. Launch a compose window from Local Folders and save it (as draft), or 2. Launch a msg from the Drafts folder under Local Folders, edit it and save it. then the new draft msgs are saved to the Drafts folder defined in the 1st account. You can reproduce this by only having a single IMAP account in a profile.
Pushing out in light of cavin's research.
Target Milestone: mozilla0.9.5 → mozilla1.0
Jennifer, Can you comment if the above described behavior is how it should work. I did refer to the specs but was not able to conclude anything for this scenario. But the way it appears right now it looks like the message just disappeared from the local draft folder after editing. I did a search for messages and happen to find it in imap draft folder. This could a little bit confusing to the users if they run into it.
I understand from cavin that this has been there always. I removing the regression keyword.
Keywords: regression
We have an older bug about this kind of problem (lack of Identity involved with Local Folders), particularly it was reported for Unsent Messages (since they're stored in Local Folders) and multiple accounts -- that the identity on sending unsent messages will change to the default account. Just FYI, for reference - bug 59548
Sheela says in the original description of the bug that she edited a Draft/Template already saved in the Local Draft/Template folder and when she saved it, it was Not saved to its current location, but instead, disappeared from its current location and was instead saved to the IMAP Draft/Template folder. This seems like the wrong behavior. Updating and saving an existing Draft/Template shouldn't move it, but replace the existing message in the existing location.
Blocks: 107067
Keywords: nsbranch-
Keywords: nsbeta1
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 (you can query for this string to delete spam or retrieve the list of bugs I've moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
This bug still exists. 2001-12-14-06 win98
Keywords: nsbeta1nsbeta1+
Priority: -- → P3
The solution would be to save the identity key with the message, at least when it's saved locally. Edit message would have to read it ans setup the identity correctly.
Status: NEW → ASSIGNED
Blocks: 122274
Keywords: nsbeta1+nsbeta1-
Target Milestone: mozilla1.0.1 → mozilla1.2
No longer blocks: 107067
Depends on: 59548
Jean-Francois, how would you suggest should this saving be done? Add a new internal header line when saving as draft and removing it upon editing?
correct, we already do that for other needed info...
The attached patch fixes the original problem described by Sheela: When saving a file as draft or template, an additional header is stored (named "X-Mozilla-Identity-Key" - better suggestions welcome :), which denotes the key of the identity which was used to compose the message. When opening a draft or template, this header is evaluated, and the respective identity is retrieved and used to initialize the mail composer. Not sure if I missed something or if the patch really solves the problem completely, but it works fine for all scenarios I tried (amongst them the original one described by Sheela). What is not solved by this pice of code is the problem Cavin described in comment 5, point 2.: As there is no identity associated with "Local Folders", and the mail composer seems to need an identity where it retrieves the target folder for saving a draft/template, this sounds not that easy ....
good patch. But we already have code to save the identity key when queuing a message (send later). All you have to do is to use HEADER_X_MOZILLA_IDENTITY_KEY and write out this header when saving a draft/template as well...
oh, missed that ... thanks! What's the difference between mime_generate_headers (where I hooked in) and nsMsgComposeAndSend::MimeDoFCC (where HEADER_X_MOZILLA_IDENTITY_KEY is generated)? Should I prefer one of these locations? From the name, I would expect that writing the header fits better into mime_generate_headers ...
mime_generate_headers seems a better place for doing that. But if the code is alreay in doFCC (the code that copy the message into a folder), please do the same. I thing the historical reason is that maybe, you don't have access to the identity in mime_generate_headers.
using the HEADER_X_MOZILLA_IDENTITY_KEY header makes the patch _much_ smaller indeed :)
Attachment #105031 - Attachment is obsolete: true
Comment on attachment 105223 [details] [diff] [review] revised version using the HEADER_X_MOZILLA_IDENTITY_KEY header Looks great. Good job. R=ducarroz
Attachment #105223 - Flags: review+
Keywords: patch
Whiteboard: have fix
don't we have to free identityKey? Or use an nsXPIDLCString?
correct, we need to free identityKey
Attached patch revised version, one less memory leak ... (obsolete) (deleted) — Splinter Review
oops .... overlooked this leak :( Thanks!
Attachment #105223 - Attachment is obsolete: true
Comment on attachment 105234 [details] [diff] [review] revised version, one less memory leak ... not good, the PR_FREEIF should go outside the if block (in case identityKey is not null but an empty string)! Also, don't need to use the PR_FREEIF macro here, instead use PR_Free even if identityKey is null
Attachment #105234 - Flags: needs-work+
Attached patch corrected the correction (deleted) — Splinter Review
freed identityKey correctly this time (I hope :) (no more patches from my side today, they cause too much disgrace at 11pm)
Attachment #105234 - Attachment is obsolete: true
Comment on attachment 105242 [details] [diff] [review] corrected the correction thx, sr=bienvenu, as long as it's OK with JF.
Attachment #105242 - Flags: superreview+
Comment on attachment 105242 [details] [diff] [review] corrected the correction Viel besser. R=ducarroz Gute Nacht Frank :-)
Attachment #105242 - Flags: superreview+ → review+
Comment on attachment 105242 [details] [diff] [review] corrected the correction per bienvenu's comment, SR=bienvenu
Attachment #105242 - Flags: superreview+
Fix checked in the trunk. Thanks Frank for the patch.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.2alpha → mozilla1.3alpha
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: