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

RESOLVED FIXED in Thunderbird 53.0

Status

Thunderbird
General
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: Jorg K (GMT+2), Assigned: Jorg K (GMT+2))

Tracking

({regression})

Trunk
Thunderbird 53.0
regression

Thunderbird Tracking Flags

(thunderbird52 fixed, thunderbird53 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 months ago
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.
Flags: needinfo?(alice0775)
(Assignee)

Comment 1

9 months ago
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
Flags: needinfo?(alice0775)
(Assignee)

Comment 2

9 months ago
Created attachment 8816722 [details] [diff] [review]
1321996-X-Identity-Key.patch

One line tweak with a three line comment ;-)
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8816722 - Flags: review?(mkmelin+mozilla)
(Assignee)

Comment 3

9 months ago
Created attachment 8816741 [details] [diff] [review]
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.
Attachment #8816722 - Attachment is obsolete: true
Attachment #8816722 - Flags: review?(mkmelin+mozilla)
Attachment #8816741 - Flags: review?(mkmelin+mozilla)
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?
(Assignee)

Comment 5

9 months ago
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

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

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

LGTM, r=mkmelin
Attachment #8816741 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 7

9 months ago
https://hg.mozilla.org/comm-central/rev/e3f928d021cd793df5f02b1caf1d3f341af6f40f
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-thunderbird52: --- → affected
status-thunderbird53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
(Assignee)

Comment 8

9 months ago
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.
Attachment #8816741 - Flags: approval-comm-aurora+
(Assignee)

Comment 9

9 months ago
Aurora (TB 52):
https://hg.mozilla.org/releases/comm-aurora/rev/45ff056943000af6f8794eecef4c00cb63ffe038
status-thunderbird52: affected → fixed
You need to log in before you can comment on or make changes to this bug.