Serge Gautherie (:sgautherie) Assignee
	Description • 16 years ago

(Noticed while looking into bug 425145.)

	Serge Gautherie (:sgautherie) Assignee
	Comment 1 • 16 years ago

*The code optimization.
*1 comment addition.
*Blank line removals.

	Serge Gautherie (:sgautherie) Assignee
	Comment 2 • 16 years ago

Comment on attachment 327809 [details] [diff] [review]
(Av1) <nsLoginManager.js>

Oh, well: just noticed that  mozilla-central has (2) new (related) checkins...

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

The optimization is no longer relevant on trunk, and I don't think there's good reason to take this on branch.

	Serge Gautherie (:sgautherie) Assignee
	Comment 4 • 16 years ago

Av1, with comment 2 suggestion(s). [Unified diff; no(t) Hg.]

Do we really need |this.log("form not filled, has autocomplete=off");| ?
Otherwise, that code could be optimized too.

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

Comment on attachment 327905 [details] [diff] [review]
(Av2) <nsLoginManager.js>

I don't see the point of this.

	Serge Gautherie (:sgautherie) Assignee
	Comment 6 • 16 years ago

(In reply to comment #5)
> (From update of attachment 327905 [details] [diff] [review])
> I don't see the point of this.

Isn't it easier to read/execute ?

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

No, I don't think so. You've added an early return, shuffled the logic, and made a large pile of unneeded whitespace changes.

	Serge Gautherie (:sgautherie) Assignee
	Comment 8 • 16 years ago

(In reply to comment #7)
> No, I don't think so. You've added an early return, shuffled the logic, and
> made a large pile of unneeded whitespace changes.

Reading a test once instead of three times is easier to me, at least.
Early return: yes, that's the point !
Which logic did I shuffle ?
Blank line removals shouldn't bother anything...

If you prefer, I can:
*use an |if (selectedLogin) { ... }|.
*leave the blank lines as they are.