288702 - saving an edited template should replace existing template, and keep same message-id

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[David :Bienvenu]

Editing a template and saving as template should replace the existing template,
and keep the message-id the same (this will help with reply with template filter
actions).

Comment 1

[David :Bienvenu]

Attachment: proposed fix

remember message-id when editing templates, but clear it when sending or
sending later, so that we won't re-use the message-id at send time.

Comment 2

[zug_treno]

Could this patch have caused Suite bug 308037/Thunderbird bug 292568?

Comment 3

[Thomas D. (:thomas8)]

Coming from bug 1389062 (Implement "Edit Template" command), I've been trying to dissect what this bug was all about.

I think whatever this bug was trying to do has been thoroughly undone, because it was wrong.
Unfortunately description is unclear.
To this day, there's nothing in the UI to "Edit Template" as comment 0 suggests, but now there's bug 1389062 to introduce that command, and we have already introduced "Edit draft" command in bug 1106412.
So I believe this bug was tweaking "Edit Message as New" in wrong ways, but has since been reverted.

1) fields->SetMessageId(id);
Was introduced by this bug, and has since been made conditional:

https://dxr.mozilla.org/comm-central/source/mailnews/mime/src/mimedrft.cpp#1251-1256

    // Keep the same message id when editing a draft unless we're
    // editing a message "as new message" (template) or forwarding inline.
    if (mdd->format_out != nsMimeOutput::nsMimeMessageEditorTemplate &&
        fields && !forward_inline) {
      fields->SetMessageId(id);
    }

2) fields->SetDraftId(mdd->url_name);
I believe as introduced by this bug this was also removed by bug 321355, attachment 8727043 [details] [diff] [review].
It was removed from this condition:
> if (mdd->format_out == nsMimeOutput::nsMimeMessageEditorTemplate).

It has remained for some other highly conditional cases:
https://dxr.mozilla.org/comm-central/source/mailnews/mime/src/mimedrft.cpp#1502
https://dxr.mozilla.org/comm-central/source/mailnews/mime/src/mimedrft.cpp#1536

3) It looks as if bug 292568 also contributed to undoing this bug, and there's an enlightening comment from assignee of both bugs:
(In reply to David :Bienvenu from bug 292568 comment #11)
> Created attachment 195749 [details] [diff] [review]
> proposed fix
> 
> only remove original message if it was a draft or template. This isn't
> perfect,
> since in theory when you do a edit message as new in the drafts or templates
> folder, you should get a new message, but we don't have a compType that
> enables
> us to distinguish between edit draft/template and edit message as new.

More than a decade later, we're now in the process of introducing that missing compType in bug 731688, attachment 8895053 [details] [diff] [review]:
> Components.interfaces.nsIMsgCompType.EditAsNew

It's quite unfortunate that it wasn't introduced at that time when those structural problems were realized, ignoring respective advice from users (cf. Peter Lairo's 292568 comment 14).

Comment 4

[Thomas D. (:thomas8)]

So in bug 1389062, we might end up reintroducing something like what this bug was doing, only this time under the correct label, "Edit Template", not "Edit as New Message"...