Closed Bug 723003 Opened 8 years ago Closed 7 years ago
Login Manager .js uses global Private Browsing state to make decisions
Global PB state is going away. Instead, it should query the window's docshell.
As noted in bug 723004 comment 5... What should we do when there's no obvious window at hand? This is a component, after all, and the APIs here don't have a way to allow a caller to pass in a window reference...
APIs are being changed for other components such as the Download Manager to allow for a context, or default to non-private if none is provided. Can we do the same here?
@Josh,@Ehsan,@Justin Is there an agreement upon what needs to be done then ? Wouldn't the normal solution (of converting singletons to instances) work in this case ?
No, singletons to instances won't work, since this is a component implementing the nsILoginManager interface.
(In reply to Saurabh Anand [:sawrubh] from comment #3) > @Josh,@Ehsan,@Justin Is there an agreement upon what needs to be done then ? > Wouldn't the normal solution (of converting singletons to instances) work in > this case ? What Josh said in comment 2.
I'm interested in working on this.
Assignee: nobody → saurabhanandiit
From the comments of Justin and Josh, I think we wouldn't have a window, then how am I supposed to QI to get the nsILoadContext ? I have still done some QI'ing in this patch, using nsILoadContext. Can you guys tell me whether this is somewhat correct or not.
You should turn the getter into a method that takes a window argument. In the two places that use _inPrivateBrowsing, one can be moved down after the definition of the win variable, and the other can pass doc.defaultView.
Passes all the concerned tests.
Comment on attachment 643821 [details] [diff] [review] Patch v2 Review of attachment 643821 [details] [diff] [review]: ----------------------------------------------------------------- Looks great, thanks!
Attachment #643821 - Flags: review?(ehsan) → review+
Target Milestone: --- → mozilla17
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.