680593 - Audit Sync code for strict mode violations

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

We should ensure that new Sync code is strict mode-safe, and includes "use strict" as a directive.

We should also exploit strict mode to find problems in the rest of the Sync codebase -- including tests -- but we feel that it's not a good cost-benefit tradeoff to distribute old files with strict mode enabled at this point in time: there isn't enough test coverage to be confident, and there's negligible benefit to be had.

Comment 1

[Richard Newman [:rnewman]]

Attachment: Trivial makefile change to exercise strict mode in Nightly build

Got distracted by this at 1am. Here are my findings.

* Trivial build with "use strict"; prepended to each output file. (See attachment.)
* Verified output:

  $ head -n 3 obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/modules/services-sync/engines/bookmarks.js
  "use strict";
  /* ***** BEGIN LICENSE BLOCK *****
   * Version: MPL 1.1/GPL 2.0/LGPL 2.1

* Ran Nightly. Added bookmark, history, favicon; synced.

The only observed console error was:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "property name byteArrayToString appears more than once in object literal" {file: "resource://services-sync/util.js" line: 562 column: 2 source: "  },
"}]' when calling method: [nsITimerCallback::notify]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "<unknown>"  data: yes]
************************************************************

which is kinda embarrassing.

This is far from a thorough treatment of this bug, but it's a start.

Comment 2

[Richard Newman [:rnewman]]

After fix for Bug 680678, we apparently get the next in the chain:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource://services-sync/status.js :: checkSetup :: line 93"  data: no]
************************************************************

Caused by:

JS Component Loader: ERROR resource://services-sync/resource.js:412
                     SyntaxError: redefining delete is deprecated

?

Comment 3

[Philipp von Weitershausen [:philikon]]

(In reply to Richard Newman [:rnewman] from comment #2)
> After fix for Bug 680678, we apparently get the next in the chain:

Filed bug 682449

Comment 4

[Philipp von Weitershausen [:philikon]]

Comment on attachment 554642 [details] [diff] [review]
Trivial makefile change to exercise strict mode in Nightly build

Just FYI

>+		echo "\"use strict;\"" > $$dest; \

This won't have the desired effect. It needs to be

  "use strict";

not

  "use strict;"

Comment 5

[Richard Newman [:rnewman]]

(In reply to Philipp von Weitershausen [:philikon] from comment #4)

> This won't have the desired effect. It needs to be...

Yeah, I apparently uploaded prior to qref. My local version is fixed. *sigh*