308567 - wire up nsIPromptService to CalDAV

Status: RESOLVED FIXED

(Calendar :: Provider: CalDAV, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Right now the only way to authenticate is to specify username and password in
the URI.

Comment 1

[Bruno Browning]

*** Bug 322437 has been marked as a duplicate of this bug. ***

Comment 2

[Jack Bates [:nottheoilrig]]

I also experience this problem.  Have tried authenticating to my CalDAV server using Basic & Negotiate authentication.  Both work in Firefox, but not in Sunbird

Many thanks! - Jack

Comment 3

[Gary van der Merwe]

Attachment: Patch rev0

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 218675 [details] [diff] [review]
Patch rev0

There's a problem here, which is that with this setup, only one code-path is really supported for the callbacks at a time.  If someone else wanted to use the service concurrently with the CalDAV code, there'd be unavoidable races.

I'm not what the right design here is.  The most obvious one would be to add another parameter to all the various method calls on the interface.

That would make the parameter list kinda long on some of the calls, but that's not a new problem, and it could certainly be fixed some other time.

I'm open to other suggestions about ways to do this, however.  shaver, do you have any thoughts here?

Comment 5

[Gary van der Merwe]

Attachment: Patch rev1

As per discussion, pass notificationCallbacks as parameter to each method.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 218881 [details] [diff] [review]
Patch rev1

This looks excellent.  Just a few nits, and then it will be ready to land.

* Please add your name and email address to the license boilerplate in each file that you're touching.

>     /**
>      * @param closure   caller-private data returned via listener
>      */
>     void lockResources(in PRUint32 count,
>                        [array, size_is(count)]
>                        in nsIWebDAVResource resources,
>                        in nsIWebDAVOperationListener listener,
>+                       in nsIInterfaceRequestor notificationCallbacks,
>                        in nsISupports closure);

Please add an @param doxygen comment for the notificationCallbacks param similar to the one for the closure parameter in each place that you're adding it.


>+    
>+    // nsIInterfaceRequestor impl
>+    getInterface: function(iid, instance) {

|instance| doesn't need to be listed as a parameter here since it is actually the |retval|.

>+        Components.returnCode = Components.results.NS_ERROR_NO_INTERFACE;
>+        return null;

In general, the only time it's necessary to touch Components.returnCode directly is if you're returning a success code other than NS_OK.  Since this is a failure code, the XPConnect way to do that is just to |throw| it directly instead of the set and |return|.

Comment 7

[Gary van der Merwe]

Attachment: Patch rev2

Address nits :-)

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Landed on both trunk and branch; thanks for the patch, Gary!