374757 - Lightning always wants to send iTIP messages to attendees

Status: VERIFIED FIXED

(Calendar :: Lightning Only, defect)

Description

[Matthew (lilmatt) Willis]

In bug 373380, we landed initial support to send iTIP invitations when you added attendees to the attendee box.  Basically, if we're Lightning, and you have attendees, we try to send email.  You'll see the compose window, and get to edit  or cancel the message before it gets sent, but there's no way (globally, or otherwise) to disable this behaviour.

I forsee bugs once 0.5 ships saying "but I don't _WANT_ to have to dismiss a compose window every time I edit the event".  This is a valid point.

In the long term this may be solved by the Sun event dialog, which already handles many more of the iTIP methods/states.  In the short term, I think we need to address this before 0.5 ships to avoid the above issue.

I propose that we add "[X] Send this meeting to attendees" or something similarly phrased to the event dialog, beneath the attendee box, if we're Lightning.  The checkbox state should persist between event dialogs, so that if someone never wants to send invites or updates, they only need to touch that box once.  All future events they create will have the box unchecked.

This would force us to break string freeze for this entity, but I think this may be an issue which warrants doing so.

Comment 1

[Michael Büttner (no reviews TFN)]

This bug strikes me to be the perfect bag to cover a closely related topic. Some providers (WCAP, google) automatically handle the invitation scenario. In this case it is not necessary to send the ICS mail. One possible solution to this problem is that we introduce the capability-stuff i was talking earlier about. i imagine some kind of flag in calICalendar, which would allow code to query whether or not a specific provider supports a specific feature or not. in this case the feature would be "provider is able to automatically handle invitations". in case we're operating on the storage calendar we simply go with what we currently have (pop up the compose window, etc.) otherwise we simply skip this step. i'm open for other suggestions in order to solve this problem, this is just an idea.

one side note i feel quite important, is the fact that we currently have two solutions in order to solve the invitation problem. one is the iTIP/iMIP processing, the other is the WCAP specific invitation-dialog (see [1] for details). in the long term we definitely should have a single generic solution. i hope that this is commonly agreed. maybe we should use the face2face meeting in order to find what the best strategy is...

I think we should address this issue before 0.5.

[1] http://wiki.mozilla.org/Calendar:Server_Based_Invitation_Handling

Comment 2

[Matthew (lilmatt) Willis]

As discussed in the phone meeting, this is more than just an annoyance for users of the WCAP or GData providers.  Those providers handle the invitations from the server side and don't need to send iTIP invitations, yet we still pop up the compose window.

I think mickey's suggestion of having an attribute on the provider for things like this is a good one, but admittedly I'm not the best person for architecture decisions like this.

What I (quickly, and at 7:41 in the morning) am envisioning is adding something to calIProvider like:
   boolean attribute sendItipInvitations
which we can use to show/hide (or enable/disable -- UI folks can decide) the previously described checkbox.

I do think trying to hash out a common architecture for invites is a very good use of some piece of the F2F time.  I don't think it requires the entire group though so we may want to do it in the evening or something.

Comment 3

[Michael Büttner (no reviews TFN)]

Attachment: sendItipInvitations v1

something like this? the ui-part is missing, this is just a quick'n'dirty patch that introduces the attribute as suggested above...

Comment 4

[Matthew (lilmatt) Willis]

Yes, like that, although you forgot gdata :)

I'd like jminta or mvl to comment on this approach (from an architecture standpoint) before we move forward on it though.

Comment 5

[Michiel van Leeuwen (email: mvl+moz@)]

I can see two use-cases for adding attendees. The first is to actually invite them, using the calendar system. This sounds like a business orientated feature. The other use case is just to keep a list of people you want to invite. But you want to invite them in a more personal manner. That sounds more like a thing for students.
We really must support use-case two. With only case one, our app will send out emails on it's own. That's annoying and likely just spam. So we must have a chekbox 'send invitations for those attendees'
The problem with the wcap way of dealing with this is that it does not allow for use case number two. That might be considered a problem with the providers.
In the future, the way of sending out invitations should be tied to the receiver, not to the calendar.

But in order to solve the problem (for wcap you send out emails that are not needed) in a timeframe for 0.5, I think adding the attribute to calICalendarProvider is a good enough workaround.

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: sendItipInvitations v2

Adding the same for gdata.

Comment 7

[Michiel van Leeuwen (email: mvl+moz@)]

Those patches don't fix the bug as filed. If you want to add some ther fix into the bug, that's workable. But don't forget to fix the original issue. Morphing bugs is bad!
This bug is primary about user options. provider options are only secondary.

Comment 8

[Matthew (lilmatt) Willis]

Attachment: rev3 - Adds checkbox and wires it in

Adds UI as described previously.

Comment 9

[Matthew (lilmatt) Willis]

Attachment: rev3 - screenshot for UI review

Comment 10

[Matthew (lilmatt) Willis]

Comment on attachment 259526 [details]
rev3 - screenshot for UI review

It is disabled, because I've selected the our project Google Calendar in the dropdown list.  The checkbox and label enable/disable when the calendar menulist is changed.

Comment 11

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 259526 [details]
rev3 - screenshot for UI review

ui-review=mvl

Comment 12

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 259525 [details] [diff] [review]
rev3 - Adds checkbox and wires it in

> function checkForAttendees(aItem, aOriginalItem)
> {
>     // iTIP is only supported in Lightning right now
>-    if (!gDataMigrator.isLightning) {
>+    if (isSunbird) {
>         return;
Should this not be isSunbird() ? 

Other than that, it looks good.

Comment 13

[Matthew (lilmatt) Willis]

(In reply to comment #12)
> Should this not be isSunbird() ? 
It should. Good catch. Will fix before checkin.

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 259525 [details] [diff] [review]
rev3 - Adds checkbox and wires it in

>Index: calendar/base/content/calendar-event-dialog.js
>+    document.persist("send-invitations-checkbox", "checked");

I'm not sure if we should persist the value of the checkbox. It's a quite dangerous checkbox, you don't want to have it checked by accident. When you accidently enable it, you are actually annoying other people (or sending out things you did not want to send).
And it's not consistent either: we don't persist the value of any of the other items on the dialog.


>+    if (item.hasProperty("X-MOZ-SEND-INVITATIONS")) {
>+        sendInvitesCheckbox.checked = (item.getProperty("X-MOZ-SEND-INVITATIONS").toLowerCase() == "true");

Then set it as lowercase in the first place, so you don't need to conversion here.

>Index: calendar/base/content/calendar-item-editing.js
>+    if (aItem.getProperty("X-MOZ-SEND-INVITATIONS").toLowerCase() != "true") {

No need to call hasProperty() here? Then you don't need it above. Or it's the other way around, and you do need it here.

>Index: calendar/base/public/calICalendar.idl
>+  /**
>+   * Whether or not iTIP invitations are handled automatically
>+   */
>+  readonly attribute boolean sendItipInvitations;

It's not clear what true and false mean here. The comment indicates that true would mean that the invitations are handled automatically (but by whom?). The attribute doesn't seem to mean anything. And the implementations say that true means that it's not handled automatically.
So, make the comment more clear please (and rename the attribute if needed)


>Index: calendar/providers/caldav/calDavCalendar.js
>     // attribute boolean suppressAlarms;
>     get suppressAlarms() { return false; },
>     set suppressAlarms(aSuppressAlarms) { throw Components.results.NS_ERROR_NOT_IMPLEMENTED; },
> 
>+    get sendItipInvitations() {
>+        return true;
>+    },

Nit: make that one line, to be consistent with the lines above it.

r- for now, to get the defaults fixed, and i want to see the new comment in the idl.

Comment 15

[Matthew (lilmatt) Willis]

Attachment: rev4 - Addresses mvl and xFallen's comments

This addresses the issues laid out in comment 12 and comment 14.
UI is the same as before.

Comment 16

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 259725 [details] [diff] [review]
rev4 - Addresses mvl and xFallen's comments

r2=mvl

Comment 17

[Matthew (lilmatt) Willis]

Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Comment 18

[Marcel Berteler]

TB version 2.0pre (20070326)
lightning Version 0.5pre (2007032807)

No Composition window opening. Not when editing, not when creating an event.

Calendar used are stored locally, no WCAP / Google.

Is it something I did?

Comment 19

[Marcel Berteler]

Did some more testing and got the following:

To create an event, I double click in the day where I want to have the event, I
put a test subject, click on more, type an email address which is not one of my
aliases and select ok.
Expected : an email invitation is send
Result : no email invitation

After this, I edit the event and add a new attendee and click OK
Expected: an email invitation is send
Result : an email invitation is send

Comment 20

[Marcel Berteler]

In bug 373380 the following user-case was presented:

User X: Lightning -> Send Invitation (REQUEST iTIP method)
User Y: Lightning (or other application) -> Replies to Invitation (REPLY iTIP
method)
User X: Lightning -> Receives the REPLY.

Now that it send the invitation as a attachment, my TB/Lightning does not recognise the attachment as an event. Is that because of my virus scanner adding scanning results at the bottom of the mail, or is this a general problem? (And an other bug?)

Comment 21

[klint]

I have a Google-based calendar in Lighting, but the Google connector doesn't seem to handle attendees yet. So how do I still send invitations to these attendes (that are stored on the local copy of the calendar, although not at Google).

In addition, I also have the issue that does not record the attendees when the event is created, but does it when the event is edited.

Comment 22

[Stefan Sitter [:ssitter]]

Marcel, 
I'd like you to not add comments to a bug that handles and fixes a complete different issue. This bug is about adding a checkbox to event dialog to _not_ send invitations. If you want to send invitations and it is not working file a new bug and add your information in the new bug please.

klint,
attendees are not yet supported. See [http://wiki.mozilla.org/Calendar:GDATA_Provider] and Bug 355226.

Comment 23

[klint]

Stefan
I'm sorry but I'm asking again : how do we do to send invitations by mail for google-connected calendars until bug 355226 is fixed ? Would it be possible to identify the gdata provider as enabling mail invitations until then, and disable it when the patch for bug 355226 lands ? Of course this depends on when the latter will land. 
But I think it would be sad to ship 0.5 without being able to invite people for google-based event.
What do you think ?
Thanks

Comment 25

[Martin Schröder [:mschroeder]]

VERIFIED with Tb 2.0.0.4pre & Lightning 0.5pre (2007052205) on Windows XP.