In-Reply-To: and References: should be removed from mail saved as Template

RESOLVED FIXED in Thunderbird 50.0

Status

defect
RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: World, Assigned: jorgk)

Tracking

(Blocks 1 bug)

Trunk
Thunderbird 50.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird47 wontfix, thunderbird48 wontfix, thunderbird49 fixed, thunderbird50 fixed, thunderbird_esr4549+ fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

In-Reply-To: and References: should be removed from mail saved as Template.

(1) "Edit As New" of an existent mail which has In-Reply-To: and References:.
(2) Save As Template, Exit composition window.
    => In-Reply-To: and References: of original mail is kept in template mail.
(3) Double click of the saved template mail (==Edit As New),
    modify mail, Send Later.
    => In-Reply-To: and References: of original mail still exists
       in mail which will be sent.

If "Edit As New" + "Save As Draft", above is correct action, because purpose is modified version of previous reply mail when the existent mail is reply mail.
However, I believe use case and user's expectation of "Edit As New" + "Save As Template" is never creation of template of reply mail which is same reply as the existent mail.
User's purpose of "Edit As New" + "Save As Template" is re-use of mail data such as To:, CC:, part of message text of the mail which he sent or received, with minimum modification.
Shouldn't it always remove those for "edit as new"? Not only for save as draft/template.
(In reply to Magnus Melin from comment #1)
> Shouldn't it always remove those for "edit as new"? 

No. "Edit As New" is meant more like "write this mail again from the same starting poing".
As Karsten Düsterloh says, "Edit As New" + "Save As Draft"(needless to say +Send/Send Later too) should keep Reply-To: and References:.
This bug is for "Edit As New" on mail with In-Reply-To:/Refereces: + "Save As Template" only.

Updated

3 years ago
Blocks: 273277
I keep getting bitten by this, so wondering if I can interest you in this :)
Flags: needinfo?(acelists)

Comment 5

3 years ago
Seems reasonable. I think Jorg now got practice in adding/removing email headers:)
Flags: needinfo?(acelists) → needinfo?(mozilla)
Assignee

Comment 6

3 years ago
I'll take a look.
Flags: needinfo?(mozilla)
Assignee

Comment 7

3 years ago
Wayne: The idea is of course that you only save a *new* message as a template. As a workaround: If you create a template from a message which has unwanted headers, you'd need to save the message and remove the headers in a text editor. How often to you generate templates?
Assignee

Comment 8

3 years ago
Posted patch Proposed solution (v1a). (obsolete) — Splinter Review
Is there more to it than this?
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attachment #8771902 - Flags: review?(acelists)
Assignee

Comment 9

3 years ago
I'll fix the trailing space, don't worry.

Comment 10

3 years ago
Comment on attachment 8771902 [details] [diff] [review]
Proposed solution (v1a).

If we save as template, do we stay in "template-mode" so we do not need the references back? It does not look like that. Even after you save as template, you can still send out the current composition. So maybe you need to restore the .references back.
Attachment #8771902 - Flags: review?(acelists)
Assignee

Comment 11

3 years ago
Posted patch Proposed solution (v2a). (obsolete) — Splinter Review
Now restoring references.

Maybe you won't believe me, but I was going to do this anyway. You beat me to it.
Attachment #8771902 - Attachment is obsolete: true
Attachment #8771917 - Flags: review?(acelists)
Assignee

Comment 12

3 years ago
Maybe s/oldPreferences/savedPreferences/.
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #7)
> How often to you generate templates?

1-2 times a month. And always from an existing email.

thanks for taking this on.

Comment 14

3 years ago
Comment on attachment 8771917 [details] [diff] [review]
Proposed solution (v2a).

Review of attachment 8771917 [details] [diff] [review]:
-----------------------------------------------------------------

Works great. Change the variable name as you proposed. Thanks.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +3086,5 @@
>    gAutoSaveKickedIn = false;
>    gEditingDraft = false;
>  
> +  let oldReferences = null;
> +  if (gMsgCompose && gMsgCompose.compFields) {

I would add a comment why we are doing this removing.
Attachment #8771917 - Flags: review?(acelists) → review+
Assignee

Comment 15

3 years ago
Changed variable name and extra comment.
Attachment #8771917 - Attachment is obsolete: true
Attachment #8772184 - Flags: review+
Assignee

Comment 16

3 years ago
https://hg.mozilla.org/comm-central/rev/bbdd29586adf
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
Assignee

Updated

3 years ago
Component: Composition → Message Compose Window
Product: MailNews Core → Thunderbird
Assignee

Comment 17

3 years ago
Moved to Product Thunderbird since fix was in MsgComposeCommands.js.

rsx11m, you might be interested to port this to SM.
Flags: needinfo?(rsx11m.pub)
Assignee

Comment 18

3 years ago
Comment on attachment 8772184 [details] [diff] [review]
Proposed solution (v2b).

Let's uplift this low risk patch to Aurora since it fixes an ancient bug.
Attachment #8772184 - Flags: approval-comm-aurora+

Comment 19

3 years ago
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #17)
> rsx11m, you might be interested to port this to SM.

Thanks!
Flags: needinfo?(rsx11m.pub)
See Also: → 1287753
Assignee

Comment 20

3 years ago
Comment on attachment 8772184 [details] [diff] [review]
Proposed solution (v2b).

[Approval Request Comment]
Regression caused by (bug #): No regression, was always wrong.
User impact if declined: Invalid references header in message based on template.
Testing completed (on c-c, etc.): Manual testing.
Risk to taking this patch (and alternatives if risky):
Low, a few lines of JS to clear and restore references before/after saving the template. Besides, that's not a common code path ;-)
Attachment #8772184 - Flags: approval-comm-esr45?

Updated

3 years ago
Depends on: 1303552
Comment on attachment 8772184 [details] [diff] [review]
Proposed solution (v2b).

http://hg.mozilla.org/releases/comm-esr45/rev/0b0c555b35cb
Attachment #8772184 - Flags: approval-comm-esr45? → approval-comm-esr45+
Ouch!  I just ran into this change and thought it was a bug, not a fix!  I depended on saving the In-reply-to: and References: headers in templates because I need to generate repeated replies to the same newsgroup post.  Now I have to save a draft instead, and then move it from my drafts folder to my templates folder.
You need to log in before you can comment on or make changes to this bug.