Last Comment Bug 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
: Replies to email invitations which have been sent by Lightning 4.0.2 may have...
Status: RESOLVED FIXED
: regression
Product: Calendar
Classification: Client Software
Component: E-mail based Scheduling (iTIP/iMIP) (show other bugs)
: Lightning 4.0.2
: Unspecified Unspecified
-- major (vote)
: 4.0.2.1
Assigned To: [:MakeMyDay]
:
:
Mentors:
: 1190802 (view as bug list)
Depends on:
Blocks: ltn4021 1197320 1199942
  Show dependency treegraph
 
Reported: 2015-09-12 12:06 PDT by [:MakeMyDay]
Modified: 2015-09-29 11:17 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
IgnoreMalformedAttendees-V1.diff (1.89 KB, patch)
2015-09-12 12:16 PDT, [:MakeMyDay]
philipp: feedback+
Details | Diff | Splinter Review
IgnoreMalformedAttendees-V2.diff (9.43 KB, patch)
2015-09-13 05:02 PDT, [:MakeMyDay]
no flags Details | Diff | Splinter Review
IgnoreMalformedAttendees-V3.diff (5.42 KB, patch)
2015-09-13 12:46 PDT, [:MakeMyDay]
no flags Details | Diff | Splinter Review
IgnoreMalformedAttendees-V4.diff (8.61 KB, patch)
2015-09-14 07:45 PDT, Philipp Kewisch [:Fallen]
philipp: review+
philipp: approval‑calendar‑aurora+
philipp: approval‑calendar‑beta+
philipp: approval‑calendar‑esr+
Details | Diff | Splinter Review

Description User image [:MakeMyDay] 2015-09-12 12:06:20 PDT
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 User image [:MakeMyDay] 2015-09-12 12:16:21 PDT
Created attachment 8660346 [details] [diff] [review]
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 User image Philipp Kewisch [:Fallen] 2015-09-12 16:48:43 PDT
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 User image [:MakeMyDay] 2015-09-13 05:02:44 PDT
Created attachment 8660423 [details] [diff] [review]
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 User image Philipp Kewisch [:Fallen] 2015-09-13 05:18:53 PDT
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 User image [:MakeMyDay] 2015-09-13 05:32:20 PDT
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 User image [:MakeMyDay] 2015-09-13 12:46:46 PDT
Created attachment 8660476 [details] [diff] [review]
IgnoreMalformedAttendees-V3.diff

Updated patch with removed debug.log check and the second test removed.
Comment 7 User image Philipp Kewisch [:Fallen] 2015-09-14 07:45:51 PDT
Created attachment 8660748 [details] [diff] [review]
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 User image Philipp Kewisch [:Fallen] 2015-09-16 12:41:01 PDT
Pushed to comm-central changeset 7491867de4b2
Comment 10 User image Philipp Kewisch [:Fallen] 2015-09-29 08:41:53 PDT
*** Bug 1190802 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.