(no longer active) Assignee
	Description • 16 years ago

Followup from bug 460086 comment 26:

(In reply to comment bug 460086 #23)
> We really don't expect them to throw, and
> if they do, we should propagate that exception.

Well, getAllLogins() will throw if it triggers a master password prompt and the
user clicks cancels. That's probably something that should be caught and
ignored.



Dolske: is there a better way to detect this exception rather than this hacky comparison?

	Shawn Wilsher :sdwilsh
	Comment 1 • 16 years ago

We should probably add a test for this too

	Justin Dolske [:Dolske]
	Comment 2 • 16 years ago

(In reply to comment #0)

> Dolske: is there a better way to detect this exception rather than this hacky
> comparison?

Not really... You could just unconditionally catch and ignore exceptions from it, doesn't seem like a big deal either way.

It might be syntactically cleaner to use |catch (e if e == "blah")| instead of having to rethrow the exception, though.

	Shawn Wilsher :sdwilsh
	Comment 3 • 16 years ago

I wish password manager threw error codes like everything else :(

	(no longer active) Assignee
	Comment 4 • 16 years ago

(In reply to comment #1)
> We should probably add a test for this too

Justin, can we force this exception in a unit test?

If not, we'd have to resort to catching the master password dialog and pressing cancel from the test (ugh).

	(no longer active) Assignee
	Comment 5 • 16 years ago

(In reply to comment #2)
> Not really... You could just unconditionally catch and ignore exceptions from
> it, doesn't seem like a big deal either way.

Hmm, Shawn mentioned that the plan is to let removeDataFromDomain pass along any possible exception.  If that's not an issue, I can do this, Shawn.

> It might be syntactically cleaner to use |catch (e if e == "blah")| instead of
> having to rethrow the exception, though.

Oh, didn't know that syntax existed.  Will do based on Shawn's respone.

	Shawn Wilsher :sdwilsh
	Comment 6 • 16 years ago

We should throw other exceptions.

I imagine the master password is prompted with something like nsIPrompt?  If so, you can create your own implementation of it, register it, and have it manage the canceling all in xpcshell.  An example of this is here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/tests/chrome/test_bug_412360.xul#69

	Mike Connor [:mconnor]
	Comment 7 • 16 years ago

Comment on attachment 346683 [details] [diff] [review]
Patch (v1)

we should get this in, since this would break the clear action in some cases.

	(no longer active) Assignee
	Comment 8 • 16 years ago

Aaron: are you willing to consider writing a test for this?

	(no longer active) Assignee
	Comment 10 • 16 years ago

http://hg.mozilla.org/mozilla-central/rev/b520f255b3a9