Closed
Bug 551851
Opened 14 years ago
Closed 14 years ago
Add Services.jsm goodness to login manager
Categories
(Toolkit :: Password Manager, defect)
Toolkit
Password Manager
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a4
People
(Reporter: Dolske, Assigned: Dolske)
Details
Attachments
(1 file, 1 obsolete file)
26.86 KB,
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
Bug 512784 added Services.jsm, we should use this goodness in login manager code. Untested (waiting for my build to finish) but is simple enough.
Attachment #432024 -
Flags: review?(paul)
Comment 1•14 years ago
|
||
Comment on attachment 432024 [details] [diff] [review] Patch v.1 r=zpao on the password manager part. I assume somebody else needs to give the nod to adding console to services...
Attachment #432024 -
Flags: review?(paul) → review+
Assignee | ||
Comment 2•14 years ago
|
||
Oops. Took out a QueryInterface I shouldn't have. xpcshell and mochitests now all pass. Over to vlad for the change to Services.jsm (bottom of patch).
Assignee: nobody → dolske
Attachment #432024 -
Attachment is obsolete: true
Attachment #432040 -
Flags: review?(vladimir)
Attachment #432040 -
Flags: review?(vladimir) → review+
Comment 3•14 years ago
|
||
Comment on attachment 432040 [details] [diff] [review] Patch v.2 >diff --git a/toolkit/components/passwordmgr/src/crypto-SDR.js b/toolkit/components/passwordmgr/src/crypto-SDR.js > init : function () { > // Connect to the correct preferences branch. > this._prefBranch = Cc["@mozilla.org/preferences-service;1"]. > getService(Ci.nsIPrefService); > this._prefBranch = this._prefBranch.getBranch("signon."); Missed this before - Services.prefs this one too.
Assignee | ||
Comment 4•14 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/b264a7e3c0f5 (with extra change from comment 3)
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a4
Comment 5•14 years ago
|
||
(In reply to comment #2) > Took out a QueryInterface I shouldn't have. Why not? Services.jsm seems to do it already...
Comment 6•14 years ago
|
||
(In reply to comment #5) > (In reply to comment #2) > > Took out a QueryInterface I shouldn't have. > > Why not? Services.jsm seems to do it already... Services.jsm QIs Services.prefs to nsIPrefBranch2, but getBranch returns an nsIPrefBranch, so that needs to be QIed. Why does Services.jsm even QI to nsIPrefBranch2? I don't think that should do anything, let alone work...
Comment 7•14 years ago
|
||
(In reply to comment #6) > Services.jsm QIs Services.prefs to nsIPrefBranch2, but getBranch returns an > nsIPrefBranch, so that needs to be QIed. Right, I missed the getBranch... > Why does Services.jsm even QI to nsIPrefBranch2? I don't think that should do > anything, let alone work... It's there to enable Services.prefs.add/removeObserver, which works fine.
Assignee | ||
Comment 8•14 years ago
|
||
(In reply to comment #7) > Right, I missed the getBranch... I'm glad I wasn't the only one to make this mistake. ;)
You need to log in
before you can comment on or make changes to this bug.
Description
•