1204255 - Replies to email invitations which have been sent by Lightning 4.0.2 may have email attendees with and without mailto prefix for the same email address

Status: RESOLVED FIXED

(Calendar :: E-mail based Scheduling (iTIP/iMIP), defect)

Description

[[:MakeMyDay]]

STR:

1) install 4.0.2, invite an attendee to a meeting
2) attendee accepts invitation, which will create a new attendee on the invitation that doesn't have the mailto prefix
    - the actual attendee still has NEEDS-ACTION
    - the new attendee has CONFIRMED
3) attendee upgrades to 4.0.2.1 (rc)

This is another regression of bug 1048035. This gets unvailed once patch for bug 1199942 and bug 1197320 are applied by resetting partStat of attendees back to NEEDS-ACTION.

The problem is that an organizer on Lightning 4.0.2 sends out an invitation to attendees without having prefixed the id with mailto. If a recipient then replies, he will do with a prefixed id (even if send form Lightning 4.0.2) - technically this is not a reply of the invited attendee but the party crasher scenario. In the end, the organizer and the attendee have two attendees in the attendee list for the meeting. Now, once updated to 4.0.2.1 (rc), both attendees have the same id, which results in the above described scenario.

Comment 1

[[:MakeMyDay]]

Attachment: IgnoreMalformedAttendees-V1.diff

The patch adds a duplicate check to addAttendee(). I would have preferred to add the migration code into setItemBaseFromICS(), but this would not work out for storage and cache calendars.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8660346 [details] [diff] [review]
IgnoreMalformedAttendees-V1.diff

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

Generally I think this is a good idea. A unit test wouldn't be bad, but I could live without it. I'll create some test builds tomorrow and give them a whirl before I r+. One minor comment:

::: calendar/base/src/calItemBase.js
@@ +558,5 @@
>      addAttendee: function cIB_addAttendee(attendee) {
> +        // the duplicate check is migration code for bug 1204255
> +        let exists = this.getAttendeeById(attendee.id);
> +        if (exists) {
> +            cal.LOG("Ignoring attendee duplicate: " + exists.id);

I could imagine this might spam the console a bit. If you'd like to keep the message, I'd suggest including the event name. Otherwise I'd probably just remove it.

Comment 3

[[:MakeMyDay]]

Attachment: IgnoreMalformedAttendees-V2.diff

Updated patch with a test and the logging applicable only if calendar.debug.log is enabled.

I've started a try build for the test:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=6f65779f144d

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Note the extra check for calendar.debug.log is not needed, as it only logs in that case anyway:
http://mxr.mozilla.org/comm-central/source/calendar/base/src/calUtils.js#902

Comment 5

[[:MakeMyDay]]

Argh, I missed the .verbose - do you prefer to have this limited to verbose debug mode or get removed the pref check? I would prefer to have a logging here.

Comment 6

[[:MakeMyDay]]

Attachment: IgnoreMalformedAttendees-V3.diff

Updated patch with removed debug.log check and the second test removed.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: IgnoreMalformedAttendees-V4.diff

It turns out your code is right, but the testcase was wrong. For user2, the lines were flipped in the fromICS test in contrast to the newAttendee test. I've corrected this and done some formatting changes to my liking. This is r+, I will do some more testing now and then create new 4.0.2.1 test packages.

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Pushed to comm-central changeset 7491867de4b2

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

https://hg.mozilla.org/releases/comm-aurora/rev/4928be463a71
https://hg.mozilla.org/releases/comm-beta/rev/67fc554bd823
https://hg.mozilla.org/releases/comm-esr38/rev/ac689553a190