449177 - Support free-busy querying for the GData Provider

Status: RESOLVED FIXED

(Calendar :: Provider: GData, enhancement)

Description

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

Attachment: GData Freebusy - v1

This bug adds each Google Session as a provider for freebusy. This way gmail and hosted users can be queried for freebusy using the invitations dialog.

This patch also fixes bug 429284.

Comment 1

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

Sorry about the mixed up paths, one file is misaligned by one directory component, but it should still apply.

Comment 2

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

Attachment: GData Freebusy - v1

I also missed some files, here we go again

Comment 3

[Daniel Boelzle [:dbo]]

Comment on attachment 332316 [details] [diff] [review]
GData Freebusy - v1

>+
>+function fbListener(row, binding) {
>+    this.row = row;
>+    this.binding = binding;
>+}

I'd prefer a more verbose name.

>     /**
>      * @see nsIInterfaceRequestor
>      */
>-    getInterface: function cGR_getInterface(aIID) {
>-        // Support Auth Prompt Interfaces
>-        if (aIID.equals(Components.interfaces.nsIAuthPrompt) ||
...
>-    },
>+    getInterface: calInterfaceRequestor,
calInterfaceRequestor sounds like an object/interface to me, not like a function to call.

>+/**
>+ * getInterface method for providers. This should be called in the context of
>+ * the respective provider, i.e
>+ *
>+ * return calInterfaceRequestor.apply(this, arguments);
>+ *
>+ * or
>+ * ...
>+ * getInterface: calInterfaceRequestor,
>+ * ...
>+ *
>+ * @param aIID      The interface ID to return
>+ */
>+function calInterfaceRequestor(aIID) {
see above

>+function getFreeBusyService() {
>+    if (getFreeBusyService.mService === undefined) {
>+        getFreeBusyService.mService = 
>+            Components.classes["@mozilla.org/calendar/freebusy-service;1"]
>+                      .getService(Components.interfaces.calIFreeBusyService);
>+    }
>+    return getFreeBusyService.mService;
>+}
We should move this to calUtils.js, so client code could use it (attendee pane).

>+function createFreeBusyInterval() {
Please call it calFreeBusyInterval and allow passing arguments (calId, interval, fbType).


patch untested, though looks good; r=dbo

Comment 4

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

Attachment: GData Freebusy - v2

(In reply to comment #3)
> calInterfaceRequestor sounds like an object/interface to me, not like a
> function to call.
I don't think it makes sense as an object. The function is special, it uses |this| without an explicit context. if you call the function as it is, it will kind of fail, since calInterfaceRequestor.QueryInterface is not a function (the try will catch though). The way I've put this into the gdata provider (and possibly others, followup to test if things work out), the object context is given through calling it on the calGoogleRequest object, which makes |this| be the calGoogleRequest. If this were an object, I don't think we can do it like that, since I don't think you can call an object as a function that is not a function.


> We should move this to calUtils.js, so client code could use it (attendee
> pane).
Done, also adopted client code to use it.

> 
> >+function createFreeBusyInterval() {
> Please call it calFreeBusyInterval and allow passing arguments (calId,
> interval, fbType).
I'd rather keep createFreeBusyInterval, since this is not an object/function prototype, but I added the arguments and also used.

I've also made caldav,wcap and ics use calInterfaceRequestor and createFreeBusyInterval now.

Comment 5

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

Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED

Comment 6

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

Attachment: [checked in] Extra changes

I now correctly understood some of dbo's comments. This patch results from an irc discussion and extends his review comments.