The default bug view has changed. See this FAQ.

wire up nsIPromptService to CalDAV

RESOLVED FIXED in Lightning 0.3

Status

Calendar
Provider: CalDAV
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: dmose, Assigned: Gary van der Merwe)

Tracking

Trunk
Lightning 0.3

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
Right now the only way to authenticate is to specify username and password in
the URI.
(Reporter)

Updated

12 years ago
Blocks: 298366
(Reporter)

Updated

11 years ago
No longer blocks: 298366
(Reporter)

Updated

11 years ago
Target Milestone: --- → Lightning 0.2
(Reporter)

Updated

11 years ago
Whiteboard: [cal relnote]

Comment 1

11 years ago
*** Bug 322437 has been marked as a duplicate of this bug. ***
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
(Assignee)

Updated

11 years ago
Assignee: dmose → garyvdm
(Assignee)

Comment 3

11 years ago
Created attachment 218675 [details] [diff] [review]
Patch rev0
Attachment #218675 - Flags: first-review?(dmose)
(Reporter)

Comment 4

11 years ago
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?
Attachment #218675 - Flags: first-review?(dmose) → first-review-
(Assignee)

Comment 5

11 years ago
Created attachment 218881 [details] [diff] [review]
Patch rev1

As per discussion, pass notificationCallbacks as parameter to each method.
Attachment #218675 - Attachment is obsolete: true
Attachment #218881 - Flags: first-review?(dmose)
(Assignee)

Updated

11 years ago
Attachment #218881 - Attachment description: Pach rev1 → Patch rev1
(Assignee)

Updated

11 years ago
Blocks: 323952
(Reporter)

Comment 6

11 years ago
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|.
Attachment #218881 - Flags: first-review?(dmose) → first-review-
(Assignee)

Comment 7

11 years ago
Created attachment 219526 [details] [diff] [review]
Patch rev2

Address nits :-)
Attachment #218881 - Attachment is obsolete: true
Attachment #219526 - Flags: first-review?(dmose)
(Reporter)

Updated

11 years ago
Attachment #219526 - Flags: first-review?(dmose) → first-review+
(Reporter)

Comment 8

11 years ago
Landed on both trunk and branch; thanks for the patch, Gary!
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: [cal relnote]
You need to log in before you can comment on or make changes to this bug.