Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 302389 - Make setTimeout()/setInterval() give a JavaScript strict warning when the first arg is a string (or other non-function object)
: Make setTimeout()/setInterval() give a JavaScript strict warning when the fir...
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: PowerPC Mac OS X
: -- enhancement (vote)
: ---
Assigned To: general
: Hixie (not reading bugmail)
: Jet Villegas (:jet)
Depends on:
Blocks: 296661
  Show dependency treegraph
Reported: 2005-07-27 14:02 PDT by Jesse Ruderman
Modified: 2008-03-31 13:49 PDT (History)
8 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

reporting a warning for all but functions in setTimeout and setInterval (1.27 KB, patch)
2007-04-19 19:21 PDT, Brian Crowder
no flags Details | Diff | Splinter Review
check cx for JSOPTION_STRICT (1.35 KB, patch)
2007-04-21 08:53 PDT, Brian Crowder
no flags Details | Diff | Splinter Review

Description Jesse Ruderman 2005-07-27 14:02:51 PDT
setTimeout with a string argument is bad style, a little slow, and can and has
lead to security holes.  I think it should be deprecated, starting by making it
a JavaScript strict warning (like for other dangerous scope things) or a
JavaScript console information message (like for document.getSelection()).  This
will help encourage Firefox and extension developers to use the function forms
of setTimeout, which are safer.

Related information:
Comment 1 timeless 2005-07-27 14:15:18 PDT
would this be for chrome only, or for everyone?
Comment 2 Jesse Ruderman 2005-07-27 14:17:02 PDT
I assume it would be for everyone (including Greasemonkey scripts and web
pages), like other JavaScript strict warnings.
Comment 3 Brian Crowder 2007-04-19 19:21:06 PDT
Created attachment 262204 [details] [diff] [review]
reporting a warning for all but functions in setTimeout and setInterval

Not sure how to do this as a strict-mode only warning.
Comment 4 Jesse Ruderman 2007-04-20 00:31:42 PDT
I think "are considered dangerous" sounds better than "should be considered dangerous".  "Considered dangerous" might be too strong, though.

It would be great if the error message hinted at the correct syntax, or pointed to a web page describing how to use anonymous functions, closures, and extra arguments to setTimeout (SpiderMonkey only).  Perhaps "It is safer to call setTimeout with a function instead of a string; see"
Comment 5 Johnny Stenback (:jst, 2007-04-20 13:34:54 PDT
Generating a normal error in the error console for every time setTimeout or setInterval is called with a string error seems too aggressive to me. There's a *lot* of code out there that does that, be it good or bad. I suspect that would simply flood the error console something horrible (way more so on some pages than others of course, i.e. imagine some sort of animation that's using string arguments to setInterval etc). I'd be ok with making this be a strict warning that's not shown to normal users for now, or if we'd limit these errors to one (or a couple) per page. Or something along those lines.
Comment 6 Brian Crowder 2007-04-20 20:32:02 PDT
How do I determine within this code whether or not the calling code is running in strict mode or not?
Comment 7 Brendan Eich [:brendan] 2007-04-20 22:09:37 PDT
Try grepping for STRICT in js*.c and go from there?

Comment 8 Brian Crowder 2007-04-21 08:45:41 PDT
Non of the reporting functions in the jsapi allow for the strict flag (or any additional flags) to be set on a report, and JS_HAS_STRICT_OPTION and JS_HAS_OPTION are in jscntxt.h, not in jsapi, and I'm still not sure if that tells me anything interesting about the specific script being run when the setTimeout/setInterval DOM call happens at runtime.  Can you be more specific about what I'm grepping for, Brendan?
Comment 9 Brian Crowder 2007-04-21 08:53:06 PDT
Created attachment 262351 [details] [diff] [review]
check cx for JSOPTION_STRICT

Maybe this is correct, but it still seems a little kludgy, and I don't see any other DOM code executing a check like this.
Comment 10 Brendan Eich [:brendan] 2007-04-21 09:50:09 PDT
JS_GetOptions, yeah. When inside js/src/*, use jscntxt.h. Outside, use jsapi.h.

This is not kludgey, it's optimized to avoid a warning all the time. But I'm with jst: we shouldn't warn more than once per page load. I'm also skeptical of our ability to deprecate anything. We withdrew the nag-strict-warning about with usage after hearing from developers who thought we were tilting at windmills, and at the expense of developers having to wade through console noise.

I think this bug should be WONTFIX. If the strict warning were limited to chrome contexts, perhaps... jst?

Comment 11 Brian Crowder 2007-04-23 13:59:41 PDT
Dropping off my list until more info arrives.
Comment 12 Brian Crowder 2008-03-31 12:52:02 PDT
Given no movement on this in a year, and no more feedback from jst, I'm with Brendan's comment 10.
Comment 13 Johnny Stenback (:jst, 2008-03-31 13:49:36 PDT
Yeah, fine with me as well.

Note You need to log in before you can comment on or make changes to this bug.