Closed Bug 1107388 Opened 5 years ago Closed 5 years ago

No auth prompt is shown when subscribing to CalDAV calendars [domWin.document is null]

Categories

(Calendar :: Provider: CalDAV, defect, blocker)

Lightning 3.8
defect
Not set
blocker

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mmecca, Assigned: mmecca)

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

Subscribing to a non-OAuth CalDAV calendar no longer displays an auth prompt, with the following error logged:

Error: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "domWin.document is null" {file: "resource://gre/modules/SharedPromptUtils.jsm" line: 16}]'[JavaScript Error: "domWin.document is null" {file: "resource://gre/modules/SharedPromptUtils.jsm" line: 16}]' when calling method: [nsIAuthPrompt2::promptAuth]
Source File: resource://calendar/modules/calAuthUtils.jsm
Line: 257
Matt, do you think you could take care of this one? I think you already had a hunch why its not working?
Flags: needinfo?(matthew.mecca)
Regression is likely caused by Bug 1091287 - looks like we need to pass in the real window to the prompt now.
Assignee: nobody → matthew.mecca
Status: NEW → ASSIGNED
Flags: needinfo?(matthew.mecca)
Attached patch auth-prompt.diff (obsolete) β€” β€” Splinter Review
Uses the real window for the auth prompt instead of a LoadContext. To do so we have to defer the prompt until the window is loaded.
Attachment #8532868 - Flags: review?(philipp)
Attachment #8532868 - Flags: approval-calendar-aurora?(philipp)
Comment on attachment 8532868 [details] [diff] [review]
auth-prompt.diff

Review of attachment 8532868 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp with these comments considered:

::: calendar/base/modules/calAuthUtils.jsm
@@ +317,5 @@
>  
> +        function checkWindowState() {
> +            self.mWindow = cal.getCalendarWindow();
> +            if (!self.mWindow || self.mWindow.document.readyState != "complete") {
> +                setTimeout(checkWindowState, 0);

Can you just add an event listener on the "load" or "DOMContentLoaded" event here instead? This way we don't have to hammer the event queue with the checkWindowState call.

::: calendar/base/modules/calProviderUtils.jsm
@@ -185,5 @@
>  };
>  
>  /**
> - * Implements an nsILoadContext that allows auth prompts to avoid using private
> - * browsing without a parent DOM window

I don't remember the details, but with the new code, do we still avoid using private browsing?
Attachment #8532868 - Flags: review?(philipp)
Attachment #8532868 - Flags: review+
Attachment #8532868 - Flags: approval-calendar-aurora?(philipp)
Attachment #8532868 - Flags: approval-calendar-aurora+
Version: unspecified → Lightning 3.8
(In reply to Philipp Kewisch [:Fallen] from comment #4)

> I don't remember the details, but with the new code, do we still avoid using
> private browsing?

Yes, we avoid private browsing using the real window, so we still have the option to save passwords.
Attached patch Fix v2 (obsolete) β€” β€” Splinter Review
Uses the window's load event listener if the window isn't ready yet.
Attachment #8532868 - Attachment is obsolete: true
Attachment #8536563 - Flags: review?(philipp)
Attachment #8536563 - Flags: approval-calendar-aurora?(philipp)
Attached patch Fix v3 β€” β€” Splinter Review
Don't need the Timer.jsm import anymore.
Attachment #8536563 - Attachment is obsolete: true
Attachment #8536563 - Flags: review?(philipp)
Attachment #8536563 - Flags: approval-calendar-aurora?(philipp)
Attachment #8536564 - Flags: review?(philipp)
Attachment #8536564 - Flags: approval-calendar-aurora?(philipp)
Attachment #8536564 - Flags: review?(philipp)
Attachment #8536564 - Flags: review+
Attachment #8536564 - Flags: approval-calendar-aurora?(philipp)
Attachment #8536564 - Flags: approval-calendar-aurora+
Keywords: checkin-needed
http://hg.mozilla.org/comm-central/rev/2944600ee1ef
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.9
You need to log in before you can comment on or make changes to this bug.