195716 - Custom header data (e.g. mail.compose.other.header;Supersedes) silently discarded when editing saved draft

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect, P2)

Description

[James Nord]

mozilla winXP CVS 01/Mar/03

Mails with custom header set (see bug.cgi 16925 ) loose their custom headers if
the mail is saved as draft then edited.

Steps to reproduce.

1) edit prefs.js and add
user_pref("mail.compose.other.header","Approved,Supersedes,Expires");
2) start mozilla
3) compose a new message and add a custom header
4) save the message as draft
5) edit the draft message

actual results
1) custom headers are lost.  (they are in the saved msg - verify with view source)

expected results,
1) the headers that match our custom headers should be saved / filled in in the UI

Comment 1

[Patrick]

i think this really is another manifestation of the issue covered by bug 137701

*** This bug has been marked as a duplicate of 137701 ***

Comment 2

[James Nord]

No.

Comment 3

[James Nord]

Sorry, this is not the same.  (esp seeing as that bug is WFM)

Comment 4

[Patrick]

Strange, something seems to have fixed bug 137701.

Comment 5

[Wayne Mery (:wsmwk)]

can you test this?
bug 16925  is gone.

Comment 6

[ovidiu]

Yep. Confirming. tb 2-3
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081020 Lightning/1.0pre Shredder/3.0b1pre ID:20081020030833

The precise behavior is:
It saves the headers in draft. When you open it to edit it does not show them in addresses lines, but it does in source/ all headers view. Custom headers lost from this point if save or send the draft. There is no way to recover since saved as draft, but to reenter ..

Same case for edit as new. From draft, sent or from a received. That may be considered another bug.
Not the case for send later. Ok there (different stuff)

Comment 7

[Wayne Mery (:wsmwk)]

xref 83412 Save as Draft or Template won't save the Options|Format selected 
setting depends in case strongly related

Comment 8

[Thomas D. (:thomas8)]

Bug still seen in TB 78.6.0 (64-bit) exactly as described.

Wow, so this renders using mail.compose.other.header a no-op if users happen to save messages with custom headers as draft and re-edit later. That's dataloss and pretty bad, also considering the work which we've invested recently into making raw custom headers work again with the new addressing area.

(In reply to James Nord from comment #0)

Actual results

• custom headers are lost. (they are in the saved msg - verify with view
  source)

Expected results

• the headers that match our custom headers should be saved / filled in in
  the UI

+1, exactly that: For "Edit draft", we want to show exactly and only those custom headers from draft which are defined in mail.compose.other.header.

Comment 9

[tacliat]

Possibly related in 78.6.1 mail with custom headers won't send, the send button clicks but nothing at all happens, remove the contents of the custom header and then it sends without problems.

Comment 10

[Markus Weyhen]

Wow, the draft/re-edit bug has been reported 18 years ago and nothig has been done to fix it.
Why report bugs if no one cares?

And now we have new bugs with custom header (see tacliat's posting above).
However, this new bug is not exactly as tacliat writes. It's even worse:

If you open a custom header field and enter some content and then click
send nothing happens. Removing the content of the custom header field
does not change anything ... the e-mail cannot be sent.
However, after removing the custom header field itself (so the field is not
shown at all) the e-mail can be sent.
BUT, the e-mail is then sent with the custom header including the content!!
So you think that you have removed the custom header at all but it is
secretly sent.

That's all very very bad.

Comment 11

[Thomas D. (:thomas8)]

(In reply to Markus Weyhen from comment #10)

Hello Markus, first of all, my sincere apologies for the inconvenience faced!
Secondly, thank you for alerting us to the bugs in this area!

Wow, the draft/re-edit bug has been reported 18 years ago and nothig has been done to fix it.
Why report bugs if no one cares?

I surely understand the frustration, alas it's not exactly that no one cares, we do care much. However, legacy bugs might be something else given that there was an extended period of volunteer maintenance before Thunderbird was able to hire staff again. There's loads of issues including minor/alleged/non-reproducible/poorly-defined/outdated etc. on record, but current manpower is still modest, so fixing everything at once isn't quite possible. Custom headers certainly aren't everyone's use case... (should work nevertheless, yes!). Every single bug, no matter how small, might require several manhours, mandays or even more of work - and in a several million lines codebase, it's not trivial to find and understand the broken spot. Our code is open source, so you are most welcome to contribute code analysis or patches!

That said, let's move on to get this fixed!

And now we have new bugs with custom header (see tacliat's posting above).
However, this new bug is not exactly as tacliat writes. It's even worse:

If you open a custom header field and enter some content and then click
send nothing happens. Removing the content of the custom header field
does not change anything ... the e-mail cannot be sent.

Yes, that's on record as Bug 1688336, there's a console error which clearly identifies the broken spot. Ctrl+Shift+I shows devtools if you want to check out the console error yourself. Errors in console are very helpful to get things fixed.
The good news is, following your comment I have analysed this, and I think we can fix it fast.

However, after removing the custom header field itself (so the field is not
shown at all) the e-mail can be sent.
BUT, the e-mail is then sent with the custom header including the content!!
So you think that you have removed the custom header at all but it is
secretly sent.
That's all very very bad.

Indeed, that's nasty!!! So apart from Bug 1688336, you have identified another problem, and now that we know, we can look into that, too. We need to get that filed with bullet steps.

Comment 12

[Alexander Bergmann]

While I was testing the new Compose API to set and get custom headers via messenger.compose.getComposeDetails() and messenger.compose.setComposeDetails() - see bug 1749198 - I also discovered this bug.

Unfortunately it is still reproducible with the steps mentioned in comment #1 and affects messages both saved as a draft and saved as a template:

1. Configure a custom header "X-Test" via "mail.compose.other.header"

2. Create a new message and add the custom header

3. Save the message as draft or template
   -> The custom header is correctly saved

4. Open the saved message
   -> The custom header is silently discarded

Maybe - after all those years - this can get fixed in time for the upcoming Thunderbird 102?

Comment 14

[Thomas D. (:thomas8)]

Alex, John - do you think we can make a plan for custom headers to survive "Edit draft"?
Allowing custom headers looks important, but if they are not reliable, the whole feature becomes pretty much no-op.

(In reply to Thomas D. (:thomas8) from comment #8)

Bug still seen in TB 78.6.0 (64-bit) exactly as described.
Wow, so this renders using mail.compose.other.header a no-op if users happen to save messages with custom headers as draft and re-edit later. That's dataloss and pretty bad, also considering the work which we've invested recently into making raw custom headers work again with the new addressing area.

Comment 15

[Thomas D. (:thomas8)]

According to this bug list (25 bugs atm), users also seem to struggle a lot with filtering custom headers, which sort of hints at their most popular purpose and continued usage.

Comment 16

[Masashi FUJITA]

Just to add severity, users may want to use a custom header for special filtering, and for that purpose in many cases user may want to save the email as a template to avoid setting custom header value every time.
But since 'save as template' and re-open it for editing discard the custom header, user has to set the value every time he send the email even it is always same.

Comment 17

[Masashi FUJITA]

I found related bug. I think this is new for 1XX.X .

Filtering/Searching custom header setting disappear after you close Thunderbird.
https://bugzilla.mozilla.org/show_bug.cgi?id=1785028

Comment 18

[Alessandro Castellani [:aleca]]

Open the saved message
-> The custom header is silently discarded

Is the header discarded also if there's text in the field?
If this happens only if the field is empty, it seems to be somewhat by design, but nonetheless we should keep those fields visible if users saved drafts or templates with those extra headers visible, no matter if they have content or not.

Asking for the data retention in case we need to escalate this bug.
Removing John NI since this is not an issue of the composer API but a core design flaw.

Comment 19

[Thomas D. (:thomas8)]

(In reply to Alessandro Castellani [:aleca] from comment #18)

Open the saved message
-> The custom header is silently discarded

Is the header discarded also if there's text in the field?

Yes, that's what this is all about - it's really dataloss - just retested on 102.2.0 (64-bit).

If this happens only if the field is empty, it seems to be somewhat by design, but nonetheless we should keep those fields visible if users saved drafts or templates with those extra headers visible, no matter if they have content or not.

Maybe. Otoh, we've never saved empty fields.

Comment 20

[Alessandro Castellani [:aleca]]

Nicolai, can you take care of this, please?
Comment 12 gives a good STR.

Comment 21

[u695164]

The custom headers are not yet readable.
For the compose window it seems like that nsMsgCompFields have to be extended to use the pref and have an attribute for custom headers.
mimedrft.cpp should fill the nsMsgCompFields for the custom headers.

The items are correctly built on the UI in ComposeLoad

Currently I'm unsure where the custom headers should be filled.

Furthermore it seems that for the non compose view nsIMsgDBHdr needs to be extended aswell. I couldn't figure out if they value is present with the extension of nsMsgCompFields. This is important for custom headers in a non compose view.

Comment 22

[u695164]

Attachment: Bug 195716 - Handle custom header data (e.g. mail.compose.other.header;Supersedes) when restoring saved draft. r=mkmelin

Comment 23

[Kevin Frost]

Is this the reason that thunderbird 102.2.1 Linux install doesn't seem to action adding this X header as per these instructions on:
http://www.gnuterrypratchett.com/#thunderbird
Or should I create a new bug?

Comment 24

[Masashi FUJITA]

No intention to rush, but may I have update if any?
Thanks.

Comment 25

[Alessandro Castellani [:aleca]]

Magnus, is this something you could take on and bring to completion?

Comment 26

[Magnus Melin [:mkmelin]]

Sure. Shouldn't be much left to do.

Comment 27

[Magnus Melin [:mkmelin]]

Addressed the review comments. Ready for checkin.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b50001f541ef
Handle custom header data (e.g. mail.compose.other.header;Supersedes) when restoring saved draft. r=mkmelin

Comment 29

[b3]

Accidental white-space change here: https://hg.mozilla.org/comm-central/rev/b50001f541ef#l4.30

Comment 30

[Magnus Melin [:mkmelin]]

Actually that was intended, to make sure such fat-fingered spacing also works.

Comment 31

[Masashi FUJITA]

Thanks so much guys for taking care of his ticket.
Appreciated.

Many Thanks,
-FUJITA

Comment 32

[Magnus Melin [:mkmelin]]

Comment on attachment 9291958 [details]
Bug 195716 - Handle custom header data (e.g. mail.compose.other.header;Supersedes) when restoring saved draft. r=mkmelin

[Approval Request Comment]
User impact if declined: per bug summary
Testing completed (on c-c, etc.): c-c, beta
Risk to taking this patch (and alternatives if risky): not a super trivial change, but fairly confined to the functionality involved

Comment 33

[Wayne Mery (:wsmwk)]

Comment on attachment 9291958 [details]
Bug 195716 - Handle custom header data (e.g. mail.compose.other.header;Supersedes) when restoring saved draft. r=mkmelin

[Triage Comment]
Approved for esr102

Comment 34

[Rob Lemley [:rjl]]

Thunderbird 102.4.1:
https://hg.mozilla.org/releases/comm-esr102/rev/62e76ac82f2e

Comment 35

[Masashi FUJITA]

It works!
No intention to disturb you guys by notification, but let me say big Thanks!