Last Comment Bug 489026 - Strict Warning: assignment to undeclared variable logins
: Strict Warning: assignment to undeclared variable logins
: fixed1.9.1
Product: Toolkit
Classification: Components
Component: Password Manager (show other bugs)
: 1.9.1 Branch
: All All
-- minor (vote)
: mozilla1.9.2a1
Assigned To: Simon Bünzli
: Matthew N. [:MattN] (PM if requests are blocking you)
Depends on:
Blocks: 296661
  Show dependency treegraph
Reported: 2009-04-18 15:01 PDT by Simon Bünzli
Modified: 2009-05-06 21:28 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fixes (1.86 KB, patch)
2009-04-18 15:16 PDT, Simon Bünzli
dolske: review+
Details | Diff | Splinter Review
for check-in (2.23 KB, patch)
2009-04-18 16:20 PDT, Simon Bünzli
mbeltzner: approval1.9.1+
Details | Diff | Splinter Review

Description User image Simon Bünzli 2009-04-18 15:01:08 PDT
Warnung: assignment to undeclared variable logins
Quelldatei: file:///C:/Programme/Mozilla%20Firefox/components/nsLoginManager.js
Zeile: 1066

Bonus strict warning:

Warnung: reference to undefined property form.elements[i].type
Quelldatei: file:///C:/Programme/Mozilla%20Firefox/components/nsLoginManager.js
Zeile: 643
Comment 1 User image Simon Bünzli 2009-04-18 15:16:27 PDT
Created attachment 373514 [details] [diff] [review]

Looks like we leak one variable to the global scope; and (at least) HTMLFieldSetElement elements are considered form elements but don't have a "type" attribute.
Comment 2 User image Justin Dolske [:Dolske] 2009-04-18 15:51:21 PDT
Comment on attachment 373514 [details] [diff] [review]

>-            if (form.elements[i].type != "password")
>+            if (!(form.elements[i] instanceof Ci.nsIDOMHTMLInputElement) ||
>+                form.elements[i].type != "password") {

Boo. Stupid HTML. It makes me sad to do this, but guess it removes any ambiguity over having something that isn't a <input type="password"> slip through.

But this now means there are 4 uses of form.elements[i] in the loop. Add a "var element = form.elements[i];" to the top of the loop and fix up the rest to use |element|. r+ with that.
Comment 3 User image Simon Bünzli 2009-04-18 16:20:21 PDT
Created attachment 373519 [details] [diff] [review]
for check-in
Comment 4 User image Dão Gottwald [:dao] 2009-04-19 18:13:50 PDT
Comment 5 User image Mike Beltzner [:beltzner, not reading bugmail] 2009-05-06 10:47:31 PDT
Comment on attachment 373519 [details] [diff] [review]
for check-in

Comment 7 User image Mike Beltzner [:beltzner, not reading bugmail] 2009-05-06 21:28:22 PDT
This checkin was in a range identified with a Ts Shutdown regression on Windows:

Regression: Ts Shutdown increase 27.64% on WINNT 5.1 Firefox3.5
   Previous results:
       357.263 from build 20090506145316 of revision 486b76052a94 at 2009-05-06 14:53:00
   New results:
       456.0 from build 20090506155401 of revision 7aa4483585bd at 2009-05-06 15:54:00[{"machine":32,"test":36,"branch":3},{"machine":33,"test":36,"branch":3},{"machine":34,"test":36,"branch":3},{"machine":35,"test":36,"branch":3},{"machine":48,"test":36,"branch":3}]&sel=1241564040,1241736840

Note You need to log in before you can comment on or make changes to this bug.