Allow the extension to opt-in to sending invitations and responses #19

NeilRashbrook · 2020-11-10T23:00:34Z

This is what I was able to write to allow the extension to grab the MIME content and send it on behalf of Lightning. This assumes that you're going to support scheduling, of course.

Not shown is the code to allow the extension to update the meeting with the attendees. (At the very least I want to get recurring items working.)

The code in sendItems is partly copied and pasted from Lightning code but ideally the provider would have better access to the desired MIME content.

benbucksch · 2020-11-11T17:52:42Z

calendar/api/parent/ext-calendar-provider.js

+  scheme = "mailto";
+
+  async sendItems(aRecipients, aItipItem) {
+    let { jsmime } = ChromeUtils.import("resource:///modules/jsmime.jsm");


Import at the top of the file, not on every invokation. JSMime is going to be loaded at TB start anyway, no point in lazy loading. And, FWIW, lazy load slows down every function call a little bit, even if it's cached.

benbucksch · 2020-11-11T17:53:07Z

calendar/api/parent/ext-calendar-provider.js

+    methodProp.value = method;
+    serializer.addProperty(methodProp);
+    let icsText = serializer.serializeToString();
+    let randomGenerator = Cc["@mozilla.org/security/random-generator;1"].getService(Ci.nsIRandomGenerator);


You don't need cryptographically strong randomness here. Pseudo-Random is sufficient.

Sorry, I was just copying Thunderbird code.

benbucksch · 2020-11-11T17:54:24Z

calendar/api/parent/ext-calendar-provider.js

+    let icsText = serializer.serializeToString();
+    let randomGenerator = Cc["@mozilla.org/security/random-generator;1"].getService(Ci.nsIRandomGenerator);
+    let randomBytes = randomGenerator.generateRandomBytes(12);
+    let boundary = randomBytes.map(byte => byte.toString(16).padStart(2, 0)).join("").toUpperCase();


Doesn't JSMime have functions to generate a multipart MIME structure with boundaries, instead of sticking strings and newlines together?

No, that's still handled by nsMsgSendPart.cpp for now.

I think js message sending is available behind a pref, might be worth looking in to.

benbucksch · 2020-11-11T17:55:47Z

calendar/api/parent/ext-calendar-provider.js

+            name: "calendar.provider.onSend",
+            register: fire => {
+              let listener = (event, calendar, content) => {
+                return fire.async(convertCalendar(context.extension, calendar), content);


Rant (not your fault): Wow, EventManager is mighty ugly code. It's basically all boilerplate code, all of these 15 lines. That should be simpler.

benbucksch · 2020-11-11T18:09:15Z

calendar/api/parent/ext-calendar-provider.js

+    headers.set("Subject", [subject]);
+    headers.set("To", aRecipients.map(attendee => ({ name: attendee.commonName, email: attendee.id.replace(/^mailto:/, "")})));
+    headers.set("Content-Type", ["multipart/mixed; boundary=\"" + boundary + "\""]);
+    let content = jsmime.headeremitter.emitStructuredHeaders(headers, { hardMargin: 800 }) + "\r\n";


Rename to mimeContent.

benbucksch · 2020-11-11T18:11:14Z

calendar/api/parent/ext-calendar-provider.js

@@ -408,6 +449,21 @@ this.calendar_provider = class extends ExtensionAPI {
              };
            },
          }).api(),
+
+          onSend: new EventManager({


Who's calling onSend and why? Please document, e.g.:

When we need to send out an invitation, the API calls the extension to do the actual sending, using the server's protocol (e.g. SMTP, Exchange, etc.).

The extension needs to implement onSend with the following parameters: ...

(onSend isn't a function, it's an event (in extension terminology, not to be confused with DOM events or calendar events).)

If the extension wants to handle sending mail on behalf of the calendar, it can achieve this by registering a listener using messenger.calendar.provider.onSend.addListener. When Lightning wants to send an email, the experiment code will send an extension event, which calls the listener with three parameters, calendar, mimeContent and recipients. (sendItems is not provided with a true calIEvent object, so we can't be more helpful than that.) The extension is then expected to send the mimeContent as an email to the recipients. (Note that the To header is already populated in the mimeContent, so the message can be e.g. dropped into an SMTP pickup folder as-is.)

onSend isn't a function()

Sure. But somebody calls the event handler. I meant just to document this in the code with a comment, so that it's clear to a casual reader what the overall control flow is.

So IIUC, if there are no listeners to onSend, then the base provider class takes over and sends via email. If there is an onSend listener, then it gets called and the extension is expected to do the sending. I think we should provide a use case where the extension gets the details about sending and can then decide if it wants to do the sending itself, or defer to email. I'm imagining this to be a little like webRequest, where the returned object defines how things are handled, and in that case maybe onBeforeSend may make more sense.

But somebody calls the event handler.

Well, the experiment does. And somebody calls the experiment. But that's already documented in Lightning. So I'm not sure what more you're asking for.

@NeilRashbrook: Here's an example code comment that documents what onSend does and how it works. Basically what we're discussing here.

/** * When we need to send out an invitation, the API calls the calendar provider extension * via this listener, allowing the extension to do the actual sending itself, * using the server's protocol (e.g. SMTP, Exchange, etc.). * * There are 3 ways to send the invitation: * - Let Lightning send the invitation, using SMTP and the associated identity in Thunderbird. * This happens, if there are no `onSend` listeners attached, or if all listeners return nothing, * or all return `.didSend = false`. * - The calendar provider extension sends the invitation. For that purpose, the parameters * to the listener include the event and a pre-constructed MIME message, which the extension * can send using a custom server protocol, or it can construct the invitation on its own using * the event data. It needs to return `.didSend = true` to tell Lightning that the sending is done. * - The calendar provider knows that the server will send out or has already sent out the * invitiation on its own, simply by adding the event to the calendar, so there's no need to * send another invitation. In this case, the handler just needs to return `.didSend = true`. * * If the extension can send the invitation only to some recipients (e.g. those on the * same server) and not others, it can set the `.recipients = []` property to those recipients * that got the invitation, and set `.didSend = true`, and Lightning will handle those recipients * not in the list. TODO: Do we need that? Owl doesn't need this. * * The parameters passed to the listener: ... */ onSend: new EventManager({

@NeilRashbrook : Please include the comment to document the API.

@NeilRashbrook : Please include this code comment, to document the API. None of this is obvious, and it's good to have a high-level description.

API documentation should always be in the code. A copy of it should also be on readthedocs.io, but the code docs are the most durable over time, with all website transitions etc. that we've seen.

kewisch · 2020-11-12T13:29:00Z

calendar/api/parent/ext-calendar-provider.js

+    let mimeContent = jsmime.headeremitter.emitStructuredHeaders(headers, { hardMargin: 800 }) + "\r\n";
+    mimeContent += "--" + boundary + "\r\n";
+    mimeContent += "Content-Type: text/plain; charset=UTF-8\r\n\r\n";
+    mimeContent += body + "\r\n";
+    mimeContent += "--" + boundary + "\r\n";
+    mimeContent += "Content-Type: text/calendar; method=" + aItipItem.responseMethod + "; charset=UTF-8\r\nContent-Transfer-Encoding: 8BIT\r\n\r\n";
+    mimeContent += icsText;
+    mimeContent += "--" + boundary + "\r\n";
+    mimeContent += "Content-Type: application/ics; name=\"invite.ics\"\r\nContent-Disposition: attachment; filename=\"invite.ics\"\r\nContent-Transfer-Encoding: 8BIT\r\n\r\n";
+    mimeContent += icsText;
+    mimeContent += "--" + boundary + "--\r\n";
+    await this.extension.emit("calendar.provider.onSend", this, mimeContent, aRecipients.map(attendee => attendee.id.replace(/^mailto:/, "")));


This assumes that the extension will be using email to send the scheduling message. I think we should instead pass either a js object with the specific details such as responseMethod, a MailExtensions calendar item, etc.

Together with below comment on similarities to webRequest, I think the extension can then either:

Decide for itself how to send (e.g. maybe make server requests) and indicate it has handled it

Make some changes to the data and return those, having Thunderbird take care of the send via email

Leave the message unchanged and have Thunderbird take care of the send via email.

I've added a commit to demonstrate that approach as best as I can. I don't know enough about Lightning internals to attempt to change anything other than the list of recipients. The extension is passed useful information about the email, and can return true to indicate that it has processed the message, or an object with a recipients property to show which recipients it has processed. If the extension does not process any recipients, the default Lightning processing then applies.

@kewisch : I can see the value of having the individual properties available. Neil is suggesting to do both, passing the individual properties and the MIME message into the listener, and let the listerer use what it needs.

The problem with leaving the MIME creation to the calendar provider is that creating the MIME message isn't trivial, as you can see from the code here. I actually think that the whole multipart code should eventually also move to JSMime (but that's for another enhancement request later, to provide these APIs). Worse, the code requires JSMime, which is chrome code, so we cannot just copy this code into the extension as it is. Even if that were solved, I don't think we should require every calendar provider to re-construct the MIME message on its own, but let Lightning do that, consistently for all calendar providers, at least optionally, to ensure consistency and to avoid code duplication. It can be optional, but we shouldn't force a calendar provider to re-implement that.

Make some changes to the data

While I can see the theoretical value, we don't have actual use cases for that. Additionally, Neil said that the Lightning APIs don't make that easy. Could we leave that to another PR? Given that we don't actually need that, and this here is already progress?

@kewisch : I can see the value of having the individual properties available. Neil is suggesting to do both, passing the individual properties and the MIME message into the listener, and let the listerer use what it needs.

To me this should be up to the extension to do. If the extension needs a mime message they can put it together with a library. If there is no library then we should make jsmine work without xpcom so people can make use of it. jsmine is available without Thunderbird afaik?

If we always provide both formats then we're doing extra conversion where it may not be needed. I don't actually think every calendar provider will need a mime message.

Make some changes to the data

While I can see the theoretical value, we don't have actual use cases for that. Additionally, Neil said that the Lightning APIs don't make that easy. Could we leave that to another PR? Given that we don't actually need that, and this here is already progress?

We can certainly leave it to another pr, maybe add a TODO comment though. I think there would be valid use cases for this.

Shipping an entire MIME library as part of our ext is not going to work for us. This is inherently the job of the MUA (or calendar app, in this case) and not of the extension.

The problem with leaving the MIME creation to the extension is that creating the MIME message isn't trivial, even if you do have a MIME library available, as you can see from the complexity of the code here. It should be the job of the calendar API to create the notification.

The extension should concentrate on its part of the job: Sending the notification.

The code here, in combination with a library, does not seem overly complex to me. If the extension needs a specific format to send that is their job as well.

I could imagine a similar format as the formats specifier where the developer can request specific formats. This would align with the items api where you can select specific formats. That part isn't ideal either, if we could go with one authoritative format that extensions can convert to their liking that would be my preference there as well.

in combination with a library

We will not ship an MIME library with our extension.

the developer can request specific formats

That's fine with me. Do you want to suggest an API for that? We can then go and implement it.

(What I'd like to see, presumably once this becomes part of Thunderbird, is that the code in Lightning that is used to create the MIME message is refactored so that it can be called directly rather than having to mostly cut and paste it like I did here.)

the developer can request specific formats

I can implement a returnFormat option like you have for the other listeners.

I did have to shuffle things around a bit so that the option could be accessed but now there are several formats:

an array of items, in default, ical or jcal format;

a subject, body and ics text; or

a MIME message

I did have to shuffle things around a bit so that the option could be accessed

Actually, I find that this new version is more readable, so it's a good change.

Good work!

It's nit-picking about names, but: I find that onSend was a better name, because in our case, the onSend() function does the actual sending, so that name is appropriate. If the onSend is not sending, then Lightning uses SMTP as fallback, so that still makes sense in that case. But I leave the final decree to Philipp.

NeilRashbrook · 2021-02-08T14:29:06Z

(Sorry I hadn't noticed the merge conflicts earlier; I've just rebased the branch to fix them up.)

benbucksch suggested changes Nov 11, 2020

View reviewed changes

NeilRashbrook force-pushed the email-opt-in branch from 4ecf70b to f922376 Compare November 12, 2020 12:41

kewisch reviewed Nov 12, 2020

View reviewed changes

NeilRashbrook force-pushed the email-opt-in branch from 049af1b to d74ab49 Compare December 1, 2020 16:09

NeilRashbrook added 3 commits February 8, 2021 14:24

Allow the extension to opt-in to sending invitations and responses

fb5abd3

Switch to onBeforeSend approach

6212d38

Provide returnFormat option to onBeforeSend listener

d4d6d09

NeilRashbrook force-pushed the email-opt-in branch from 59ee5dd to d4d6d09 Compare February 8, 2021 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow the extension to opt-in to sending invitations and responses #19

Allow the extension to opt-in to sending invitations and responses #19

NeilRashbrook commented Nov 10, 2020

benbucksch Nov 11, 2020

benbucksch Nov 11, 2020

NeilRashbrook Nov 12, 2020

benbucksch Nov 11, 2020

NeilRashbrook Nov 12, 2020

kewisch Nov 12, 2020

benbucksch Nov 11, 2020 •

edited

Loading

benbucksch Nov 11, 2020

benbucksch Nov 11, 2020

NeilRashbrook Nov 12, 2020

benbucksch Nov 12, 2020

kewisch Nov 12, 2020

NeilRashbrook Nov 13, 2020 •

edited

Loading

benbucksch Nov 13, 2020 •

edited

Loading

benbucksch Dec 9, 2020

benbucksch Dec 10, 2020 •

edited

Loading

kewisch Nov 12, 2020

NeilRashbrook Nov 13, 2020

benbucksch Nov 13, 2020 •

edited

Loading

kewisch Nov 14, 2020

benbucksch Dec 7, 2020 •

edited

Loading

kewisch Dec 7, 2020

benbucksch Dec 7, 2020 •

edited

Loading

NeilRashbrook Dec 10, 2020

NeilRashbrook Dec 10, 2020

benbucksch Dec 10, 2020

NeilRashbrook commented Feb 8, 2021

Allow the extension to opt-in to sending invitations and responses #19

Are you sure you want to change the base?

Allow the extension to opt-in to sending invitations and responses #19

Conversation

NeilRashbrook commented Nov 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benbucksch Nov 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeilRashbrook Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

benbucksch Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benbucksch Dec 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benbucksch Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benbucksch Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benbucksch Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeilRashbrook commented Feb 8, 2021

benbucksch Nov 11, 2020 •

edited

Loading

NeilRashbrook Nov 13, 2020 •

edited

Loading

benbucksch Nov 13, 2020 •

edited

Loading

benbucksch Dec 10, 2020 •

edited

Loading

benbucksch Nov 13, 2020 •

edited

Loading

benbucksch Dec 7, 2020 •

edited

Loading

benbucksch Dec 7, 2020 •

edited

Loading