663500 - Consider using ECMAScript 5 strict mode

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Markus Stange [:mstange]]

This might catch some bugs faster (e.g. bug 663498).

Comment 1

[Arpad Borsos [:Swatinem]]

Are there any arguments against using it?

Comment 2

[Markus Stange [:mstange]]

Attachment: storage -> window.storage

We set storage without declaring it with "var". I'm not sure what's better, adding var or using window.storage to make it clear that we're adding a global variable... what do you think?

Comment 3

[Markus Stange [:mstange]]

Attachment: delete window.tinderbox_data

Comment 4

[Arpad Borsos [:Swatinem]]

Are there any arguments against using it?

Comment 5

[Markus Stange [:mstange]]

Attachment: add "use strict"; everywhere and move inTag function to top level

Other than the fact that this might break something because I haven't tested all code paths, there are not many arguments against doing it :)

Comment 6

[Arpad Borsos [:Swatinem]]

Comment on attachment 538579 [details] [diff] [review]
storage -> window.storage

Review of attachment 538579 [details] [diff] [review]:
-----------------------------------------------------------------

Using var makes it only valid in the containing .js file, right? I think window is the way forward. As it is a workaround for window.localStorage.

Comment 7

[Markus Stange [:mstange]]

Comment on attachment 538581 [details] [diff] [review]
add "use strict"; everywhere and move inTag function to top level

I just realized that I didn't separate the inTag change. Strict mode only allows function definitions at the top (function) level, not nested in a loop.

Comment 8

[Arpad Borsos [:Swatinem]]

Comment on attachment 538580 [details] [diff] [review]
delete window.tinderbox_data

Review of attachment 538580 [details] [diff] [review]:
-----------------------------------------------------------------

Damn midair collisions :-D

Comment 9

[Arpad Borsos [:Swatinem]]

Comment on attachment 538581 [details] [diff] [review]
add "use strict"; everywhere and move inTag function to top level

Review of attachment 538581 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 10

[Markus Stange [:mstange]]

(In reply to comment #6)
> Using var makes it only valid in the containing .js file, right?

No, it's actually the same as setting the property on the global object (window), as far as I know. I don't have a JS spec link ready to prove it, though...

Comment 11

[Markus Stange [:mstange]]

http://hg.mozilla.org/users/mstange_themasta.com/tbpl-pending-infrasec-review/rev/22775f15cd6c
http://hg.mozilla.org/users/mstange_themasta.com/tbpl-pending-infrasec-review/rev/2f87b04629ae
http://hg.mozilla.org/users/mstange_themasta.com/tbpl-pending-infrasec-review/rev/f91669f4b809
http://hg.mozilla.org/users/mstange_themasta.com/tbpl-pending-infrasec-review/rev/f6acf8739140