Gavin mentioned in bug 387617 that we should do this, as it would be more useful to external callers via XPCOM.

The objective of this bug is to fix many of the throwing statements of directory toolkit/components/passwordmgr (see http://dxr.mozilla.org/mozilla-central/search?q=throw+path%3Atoolkit%2Fcomponents%2Fpasswordmgr&case=true ). Generally, in JavaScript, you can call `throw foo` to throw an error, but if `foo` is not an instance of `Error`, this complicates debugging a lot. So, we want to look at all the calls to `throw foo` in that directory and make sure that we always throw instances of `Error`. - wherever there is `throw "Some text"`, replace it with `throw new Error("Some text")`; - wherever there is `throw Components.results.SOME_CONSTANT` or equivalently `throw Cr.SOME_CONSTANT`, replace it with `throw new Components.Exception(some message, Components.results.SOME_CONSTANT)`. Is this information sufficient to get you started, Rajat? If you have any question, don't hesitate to ask.

Is this open? Interested in taking it.

Hi Jeffrey. Rajat mentioned a few days ago that he was interested in this bug, but I just filed bug 1061521 which is a very similar bug. Would you be interested in working on bug 1061521 instead?

Yes I'm interested in working on bug 1061521. Can you please assign that to me?

I haven't been able to clone mozilla-central due to its size. Any alternatives?

Hey! This is my first time fixing a bug. How do i start?

(In reply to Medhini from comment #6) > Hey! This is my first time fixing a bug. How do i start? See comment 1 and https://developer.mozilla.org/en-US/docs/Introduction

Hi I would like to work on this bug. How do I proceed? Thanks in advance shreyas

(In reply to shreyas from comment #8) > I would like to work on this bug. How do I proceed? Hello shreyas, please see comment 1 and https://developer.mozilla.org/en-US/docs/Introduction. You can also join #introduction on irc.mozilla.org to get live help getting a development environment setup.

Comment on attachment 8565825 [details] [diff] [review] bug394686.diff Review of attachment 8565825 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the patch. There are just some small things to fix then you can request review again. Your commit message should follow the rules at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Commit_message_restrictions (you can include r=MattN in advance to make it easier for checkin later). ::: toolkit/components/passwordmgr/nsLoginManager.js @@ +74,4 @@ > return this; > } > > + throw new Components.Exception("interface not found", Components.results.NS_ERROR_NO_INTERFACE); Good catch that Cr wasn't defined :) Can you add the alias for `Cr` to this file like so: const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components; (replacing the existing Cc, Ci, Cu lines) Also, I think "Interface not available" is a bit clearer. ::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js @@ +231,4 @@ > this.__strBundle = bunService.createBundle( > "chrome://passwordmgr/locale/passwordmgr.properties"); > if (!this.__strBundle) > + throw new error("String bundle for Login Manager not present!"); This should be `Error` (uppercase e). @@ +294,4 @@ > prompt : function (aDialogTitle, aText, aPasswordRealm, > aSavePassword, aDefaultText, aResult) { > if (aSavePassword != Ci.nsIAuthPrompt.SAVE_PASSWORD_NEVER) > + throw new Components.Exception("password not saaved", Components.results.NS_ERROR_NOT_IMPLEMENTED); How about "prompt only supports SAVE_PASSWORD_NEVER"? @@ +318,4 @@ > this.log("===== promptUsernameAndPassword() called ====="); > > if (aSavePassword == Ci.nsIAuthPrompt.SAVE_PASSWORD_FOR_SESSION) > + throw new Components.Exception("password saved", Components.results.NS_ERROR_NOT_IMPLEMENTED); How about "promptUsernameAndPassword doesn't support SAVE_PASSWORD_FOR_SESSION"? @@ +420,4 @@ > this.log("===== promptPassword called() ====="); > > if (aSavePassword == Ci.nsIAuthPrompt.SAVE_PASSWORD_FOR_SESSION) > + throw new Components.Exception("password saved", Components.results.NS_ERROR_NOT_IMPLEMENTED); How about "promptPassword doesn't support SAVE_PASSWORD_FOR_SESSION"? ::: toolkit/components/passwordmgr/storage-mozStorage.js @@ +66,4 @@ > return this._dbConnection; > } > > + throw new Components.Exception("interface not found", Cr.NS_ERROR_NO_INTERFACE); "Interface not available"

Updated the patch

Comment on attachment 8568339 [details] [diff] [review] bug_394686.diff Review of attachment 8568339 [details] [diff] [review]: ----------------------------------------------------------------- I pushed this after fixing a few small issues and making the commit message more descriptive. Good job. You can find some more mentored bugs at http://www.joshmatthews.net/bugsahoy/?internals=1&ff=1

https://hg.mozilla.org/mozilla-central/rev/4c55a705e629 https://hg.mozilla.org/mozilla-central/rev/4e79d19ab9f2