Strict Warning: assignment to undeclared variable logins

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Toolkit
Password Manager
--
minor
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Simon Bünzli, Assigned: Simon Bünzli)

Tracking

(Blocks: 1 bug, {fixed1.9.1})

1.9.1 Branch
mozilla1.9.2a1
fixed1.9.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.23 KB, patch
beltzner
: approval1.9.1+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
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
(Assignee)

Comment 1

9 years ago
Created attachment 373514 [details] [diff] [review]
fixes

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.
Assignee: nobody → zeniko
Attachment #373514 - Flags: review?(dolske)
Comment on attachment 373514 [details] [diff] [review]
fixes

>-            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.
Attachment #373514 - Flags: review?(dolske) → review+
(Assignee)

Comment 3

9 years ago
Created attachment 373519 [details] [diff] [review]
for check-in
Attachment #373514 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/1b7ba9054769
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
(Assignee)

Updated

8 years ago
Attachment #373519 - Flags: approval1.9.1?
Comment on attachment 373519 [details] [diff] [review]
for check-in

a191=beltzner
Attachment #373519 - Flags: approval1.9.1? → approval1.9.1+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ccbaf5729f21
Keywords: checkin-needed → fixed1.9.1
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
http://graphs-new.mozilla.org/graph.html#tests=[{"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
You need to log in before you can comment on or make changes to this bug.