1321996 - X-Identity-Key is repeated when saved draft is opened and saved again

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

STR:

Create a new message. Save draft. Close compose window.
Inspect message, one header: X-Identity-Key

Edit the draft using the "Edit" button. Save again. Close compose window.
Inspect message, two headers: X-Identity-Key

I can't see this in TB 51 beta, but it's there in TB 52 Earlybird (and Daily 53), so the regression shouldn't be too hard to find.

Alice, can you give us a hand here?

BTW, many thanks for your help in bug 1321816. Your help was absolutely essential to fixing this quickly.

Comment 1

[Jorg K (CEST = GMT+2)]

If I'm not mistaken, the regression range should the period of Mozilla52, so Sept. 19th to Nov. 14th, 2016. Looking at my work in this period, I guess I broke it in bug 1287268:
https://hg.mozilla.org/comm-central/rev/1b6364120f38

Right, that touched X-Identity-Key:
https://hg.mozilla.org/comm-central/rev/1b6364120f38#l3.14

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1321996-X-Identity-Key.patch

One line tweak with a three line comment ;-)

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1321996-X-Identity-Key.patch (v2)

There is already provision for not writing headers in the box ;-)

To test this, dump out params.composeFields.creatorIdentityKey in MsgComposeCommands.js to see that it is still populated when editing a draft.

That's due to:
https://hg.mozilla.org/comm-central/rev/1b6364120f38#l5.15

But this:
https://hg.mozilla.org/comm-central/rev/1b6364120f38#l3.14
was wrong and I'm correcting it now.

Comment 4

[WADA:World Anti-bad-Duping Agency]

Comment on attachment 8816741 [details] [diff] [review]
1321996-X-Identity-Key.patch (v2)

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

> static HeaderInfo kHeaders[] = {
>    { nullptr, false }, // CHARACTER_SET
> -  { "X-Identity-Key", false }
> +  { nullptr, false } // CREATOR IDENTITY KEY

Why double entries are needed? Source comment is important? Or position has meaning?
Or two "nullptr" is different object in this case?

Comment 5

[Jorg K (CEST = GMT+2)]

Turns out that if you put a header string there instead of 'nullptr', this header is written which is undesired in this case. Yes, the position has a meaning, see:
https://dxr.mozilla.org/comm-central/rev/5bb2d288ae3ed011048a8a405eadad245c2448be/mailnews/compose/src/nsMsgCompFields.cpp#32
https://dxr.mozilla.org/comm-central/rev/5bb2d288ae3ed011048a8a405eadad245c2448be/mailnews/compose/src/nsMsgCompFields.cpp#59

and more importantly
https://dxr.mozilla.org/comm-central/rev/5bb2d288ae3ed011048a8a405eadad245c2448be/mailnews/compose/src/nsMsgCompFields.h#45

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 8816741 [details] [diff] [review]
1321996-X-Identity-Key.patch (v2)

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

LGTM, r=mkmelin

Comment 7

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/e3f928d021cd793df5f02b1caf1d3f341af6f40f

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 8816741 [details] [diff] [review]
1321996-X-Identity-Key.patch (v2)

This bug was introduced in TB 52, so we should fix it there.

Comment 9

[Jorg K (CEST = GMT+2)]

Aurora (TB 52):
https://hg.mozilla.org/releases/comm-aurora/rev/45ff056943000af6f8794eecef4c00cb63ffe038