The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in 4.0.2.1

Status

Calendar
E-mail based Scheduling (iTIP/iMIP)
--
major
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: MakeMyDay, Assigned: MakeMyDay)

Tracking

({regression})

Lightning 4.0.2
4.0.2.1
regression
Dependency tree / graph

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

2 years ago
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.
(Assignee)

Comment 1

2 years ago
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.
Attachment #8660346 - Flags: feedback?(philipp)
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.
Attachment #8660346 - Flags: review?(philipp)
Attachment #8660346 - Flags: feedback?(philipp)
Attachment #8660346 - Flags: feedback+
(Assignee)

Comment 3

2 years ago
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
Attachment #8660346 - Attachment is obsolete: true
Attachment #8660346 - Flags: review?(philipp)
Attachment #8660423 - Flags: review?(philipp)
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
(Assignee)

Comment 5

2 years ago
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.
(Assignee)

Comment 6

2 years ago
Created attachment 8660476 [details] [diff] [review]
IgnoreMalformedAttendees-V3.diff

Updated patch with removed debug.log check and the second test removed.
Attachment #8660423 - Attachment is obsolete: true
Attachment #8660423 - Flags: review?(philipp)
Attachment #8660476 - Flags: review?(philipp)
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.
Attachment #8660476 - Attachment is obsolete: true
Attachment #8660476 - Flags: review?(philipp)
Attachment #8660748 - Flags: review+
(Assignee)

Updated

2 years ago
Attachment #8660748 - Flags: approval-calendar-release?(philipp)
Attachment #8660748 - Flags: approval-calendar-beta?(philipp)
Attachment #8660748 - Flags: approval-calendar-aurora?(philipp)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Attachment #8660748 - Flags: approval-calendar-release?(philipp)
Attachment #8660748 - Flags: approval-calendar-release+
Attachment #8660748 - Flags: approval-calendar-beta?(philipp)
Attachment #8660748 - Flags: approval-calendar-beta+
Attachment #8660748 - Flags: approval-calendar-aurora?(philipp)
Attachment #8660748 - Flags: approval-calendar-aurora+
Pushed to comm-central changeset 7491867de4b2
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 4.5
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
Target Milestone: 4.5 → 4.0.3
Duplicate of this bug: 1190802
You need to log in before you can comment on or make changes to this bug.