110454 - javascript strict warnings in password.js

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P3)

Description

[Henrik Gemal]

When selecting Change Master Password

Warning: reference to undefined property params.getString
Source File: chrome://pippki/content/password.js
Line: 42

Warning: assignment to undeclared variable i
Source File: chrome://pippki/content/password.js
Line: 51

build 20011115

Comment 1

[Stephane Saux]

Attachment: proposed patch

Comment 2

[Stephane Saux]

Kai,
part of the patch does what resetpassword.js does. I suspect the old code using
the try catch was only necessary because "arguments" was never checked.

Can you review.

Comment 3

[Kai Engert [:KaiE:]]

It is a good idea to replace the try block with the check you added.

But I think this will not resolve the first strict warning, or have you checked?
I wonder that no ";" is behind "params.getString(1)", I suggest we add one. This
may or may not make that Warning on Line 42 disappear. We should test that.

The "else path" now uses two pairs of brackets {{}}, I think you should remove one.

Comment 4

[Stephane Saux]

Attachment: revised patch.

Changes are:
1) add semicolon to the end of the GetString statement.
2) remove extra curly-braces.

Note that I've tested this. The javascript warning was because the code
read getString instead of GetString.  I checked the idl for the
nsIDialogParamBlock and made the change.

With the previous patch, just as with this one, there are no warnings.

Comment 5

[Kai Engert [:KaiE:]]

Comment on attachment 58709 [details] [diff] [review]
revised patch.

looks good.
r=kaie

Comment 6

[Kai Engert [:KaiE:]]

see also bug 106939.

Comment 7

[kinmoz]

Comment on attachment 58709 [details] [diff] [review]
revised patch.

I think the try/catch was necessary since the QueryInterface() can fail and
throw an NS_ERROR_NO_INTERFACE exception. If it doesn't fail and throw an
exception then params is almost guaranteed not to be null.

It's probably a good idea to restore it back to the way it was originally and
just put in the getString to GetString change.

sr=[e-mail] with the try/catch restored.

Comment 8

[Stephane Saux]

Kaie checked in an equivalent patch (cvs version 1.15)
Marking fixed.

Comment 9

[John Unruh]

Verified per ssaux's comment.