Closed Bug 670023 Opened 9 years ago Closed 9 years ago

Use ES5's strict mode in frontend and backend code

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla8

People

(Reporter: Unfocused, Assigned: Unfocused)

References

Details

Attachments

(1 file)

ES5's strict mode lets us catch some bad code patterns and other things that we want to avoid - such as accidentally assigning to an undeclared variable (thus creating a global variable).
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
Attached patch Patch v1Splinter Review
Add strict-mode to all .js and .jsm files, and fix up everything that breaks. Caught a few assignments to undeclared variables, couple of uses of arguments.callee (which is bad for performance), and some functions declared within if(){} blocks.

Does not make the XBL bindings use strict mode - I'm waiting until bug 669190 is fixed, which will mean entire bindings can be made to use strict mode, rather than doing it per-function. Filed bug 670029 for the XBL bindings.
Attachment #544636 - Flags: review?(dtownsend)
Comment on attachment 544636 [details] [diff] [review]
Patch v1

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

::: toolkit/mozapps/extensions/addonManager.js
@@ +147,5 @@
> +          }
> +          catch (e) {
> +            Components.utils.reportError(e);
> +          }
> +        }

newline after this

@@ +155,1 @@
>  

no need for this newline
Attachment #544636 - Flags: review?(dtownsend) → review+
https://hg.mozilla.org/integration/fx-team/rev/a115d824218c
Flags: in-testsuite-
Flags: in-litmus-
Whiteboard: [fixed-in-fx-team]
Backed out because of m-oth permaorange:

http://tbpl.mozilla.org/?tree=Fx-Team&rev=a115d824218c

s: talos-r3-xp-004
29888 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/spatial-navigation/tests/test_snav_tightlinks.xul | Move DOWN to select_element. Found start
29897 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/widget/tests/test_bug343416.xul | Move DOWN to 5. Found 1
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug562797.js | canGoBack should be correct - Got false, expected true
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug562797.js | canGoForward should be correct - Got true, expected false
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug562797.js | Back button should have the right state - Got false, expected true
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug562797.js | Forward button should have the right state - Got true, expected false
Whiteboard: [fixed-in-fx-team]
I'm a dumbass.

Fixed: http://hg.mozilla.org/integration/fx-team/rev/0f11ca4d1733
Whiteboard: [fixed-in-fx-team]
http://hg.mozilla.org/mozilla-central/rev/0f11ca4d1733
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla8
Backed out on m-c and fx-team due to xpcshell test failures:
http://hg.mozilla.org/mozilla-central/rev/19348341366b

http://tinderbox.mozilla.org/showlog.cgi?log=Fx-Team/1311294758.1311296713.27518.gz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
3rd time's a charm!

Last landing was bounced thanks to strict mode now catching assignments to undeclared variables in the initial expression of a for(...) {} loop. That caught two additional undeclared variables.

http://hg.mozilla.org/integration/fx-team/rev/5f3776437298
Whiteboard: [fixed-in-fx-team]
http://hg.mozilla.org/mozilla-central/rev/5f3776437298
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Verified fixed by landing and no test failures.
Status: RESOLVED → VERIFIED
Depends on: 677646
Depends on: 691389
You need to log in before you can comment on or make changes to this bug.