User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3 (from WebWorkers spec draft, section 4.6 - Processing model) > Let language be JavaScript. > > NOTE: As with script elements, the MIME type of the script > is ignored. Unlike with script elements, there is no way > to override the type. It's always assumed to be JavaScript. http://www.whatwg.org/specs/web-workers/current-work/#processing-model but firefox interprets this "assumed to be JavaScript" as "assumed to be JavaScript 1.5", which would be ok, but only if the default assumption could be changed (which it can't, as to the spec). Reproducible: Always Steps to Reproduce: 1. write JS1.7+ code for your worker (ie using yield) 2. start a WebWorker in your code: new Worker('ww.js') 3. ... Actual Results: nothing happens. the web worker script is not executed, it fails to parse because it contains new keywords introduced in JS 1.7, and firefox defaults to interpreting it as JS1.5. Expected Results: web worker should be parsed and interpreted as JavaScript 1.7 by default, because this (WebWorkers) is a new feature, and there is no legacy code that could be broken by switching to JS1.7. OR there should be a mozilla-specific way to determine the JS version used in the script, either with another (optional) param to the constructor: new Worker('script.js', 'version=1.7') OR using something similar to what javascript shell uses: version(170); i first posted this on the whatwg forums (sorry if you already seen this): http://forums.whatwg.org/viewtopic.php?t=4065 but, on second thought, this might be a mozilla-specific feature, as no other vendor (form whatwg) has similar needs for different JS versions.

Yeah, this came up during the design phase and we opted for 1.5. Brendan, sicking, what was the logic here again? Threaded gecko has somehow wiped my memories of this decision...

Other vendors will have a need, post-ES5, but if we choose something after 1.5 then I believe that a few (unlikely) constructs change meaning due to syntax and keywords we've added. I can't conjure one off the top of my head, though.

Oh, one thing I do remember, there was also concern about scripts imported via 'importScripts'. The spec doesn't include a way to specify version there, either, and it seems risky to assume that all scripts imported should be interpreted with the same version as the worker (especially if we're talking about large libraries like jQuery or something).

We figured cross-browser worker scripts wouldn't be able to use let, yield, etc., and so there wouldn't be much demand for these. Probably being too unassertive, in hindsight -- people like these features, we have add-ons and others who will write Firefox-3.5-specific code, etc. Short of an in-language pseudo-pragma, e.g.: "use version 1.7"; a la the "use strict"; pseudo-pragma in ES3.1 (cough, ES5), we should at least allow version to be selected when importing scripts. Why not extend the new and not-yet-standardized importScripts API? Or if adding arguments is reserved to future versions of the standard, make an importVersionedScript API? /be

The concern was also that existing libraries couldn't be run inside workers since there is no way to indicate that they should be parsed *without* support for let/yield. And that once other vendors get workers (sounds like it'll be pretty soon), there's a risk that scripts authored for those workers won't run in firefox because we treat let/yield as keywords. I think our hope was that JS will grow an in-language way to opt in to these new features. Alternatively we can add hacks like a global property that's set before 'new Worker' or 'importScripts' is called. Especially 'importScripts' is hard to fit an extra argument on since it uses varargs.

Existing Ajax libraries that I know of do not use let or yield. ES3.1^H^H^H5 is indeed reserving these identifiers in its "use strict"; mode. The odds of anyone writing a modern, "Web 2.0"-ish JS script that uses var let or function foo(yield) ... are very low, IMHO. In-language version selection is potentially hairy. It came up at the last Ecma TC39 meeting. We will work on it, but no promises. At that meeting, we separated two issues: runtime version, modulated per script at the limit, that controls what keywords are reserved; and global object graph version, which needs to be per-global-object and addresses what names are bound on standard objects (including JSON on the global, toJSON on standard object prototypes, etc. -- this is already a cause of web compatibility pain, but we are hanging tough). We shouldn't extend importScripts, so importVersionedScripts with leading version string seems best. /be

I'm still not convinced. While I agree that the risk of collision is small, so is benefit since none of this works cross browser anyway. If nothing happens in JS land to allow for in-script opt-in to these features then I'm fine with adding an API for specifying version in the next version of FF. I'm not exited about using a different version by default for main-page <script> and workers.

(In reply to comment #7) > I'm still not convinced. While I agree that the risk of collision is small, so > is benefit since none of this works cross browser anyway. How does that follow? Not working cross-browser does not undermine the utility of yield, it enhances it. Doing without yield means rewriting one's code (cf. Weave). > If nothing happens in JS land to allow for in-script opt-in to these features > then I'm fine with adding an API for specifying version in the next version of > FF. Putting off something to "the next version of Firefox" could mean "putting something off for a while". Meanwhile, JS land having no in-language version selection has *nothing* to do with the lack of utility problem you've intentionally created in *this* version. And of course, JS land doesn't need such a mechanism for the web, since our JS implementation respects the ";version=" parameter on <script type=...>. > I'm not exited about using a different version by default for main-page > <script> and workers. You aren't, currently. You're not using JS1.5, either -- rather, "the default version", which has object-graph versioned extensions beyond 1.5 (ES3). The problem remains that one cannot use keyword versioned extensions, as one can in <script> tags via ;version=. This is the bug to fix. /be

after some pondering, i came here to vote for the "use version"; pseudo-pragma solution, as it covers all the other use-cases where there is _no_way_ to externally specify JS version -- like inline event handlers, or "javascript:" URLs from this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=352228 and while inline code might not make much use of "yield", "let" expressions are badly needed to prevent global namespace pollution -- to clean-up all the (function() {code;})(); hacks found in many different places, like bookmarklets, greasemonkey user scripts, mozilla addons/chrome code, etc.. but before posting my vote, i re-read all the comments (as i should, and i am glad i did ;) (In reply to comment #6) > ES3.1^H^H^H5 is indeed reserving these identifiers in its "use strict"; mode. i missed this on the last reading.. this is *great* news! (correct me if i misunderstood something, but) any JS source that has "use strict"; and a variable named yield/let will fail to parse, and this will hold for both mozilla and any other vendor that implements ES5. this allows us to (safely) default to JS1.7 parsing for ES5 strict mode, which eliminates the need to introduce any new pseudo-pragmas (like "use version"). and unless there are many who will want to use JS1.7 features in non-strict mode -- problem solved.. furthermore, as all of the abovementioned use-cases are mozilla-specific (bookmarlets don't need to be, but usually are), i propose that *all* chrome code in mozilla should default to JS1.7 mode (but we might want to wait for the next major version to make this change).

Data point: I wanted to use localStorage in a Worker (as well as "let" and comprehensions) on 3.6, onerror.message == "localStorage is not defined".

(In reply to comment #10) > Data point: I wanted to use localStorage in a Worker The localStorage API has been removed from the Worker spec and is not available in Workers on any version of Firefox.

(In reply to comment #10) > Data point: I wanted to use localStorage in a Worker Oh, in case anyone is interested in that, here's a link to the start of the discussion that had it removed: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2009-March/018907.html As I understand it no consensus has emerged on the best way forward, though IndexedDB (http://dev.w3.org/2006/webapi/WebSimpleDB) and WebSqlDB (http://dev.w3.org/html5/webdatabase) both have worker-facing APIs. Those won't be ready any time soon, though.

(In reply to comment #8) > > You aren't, currently. You're not using JS1.5, either -- rather, "the > default version", which has object-graph versioned extensions beyond 1.5 > (ES3). The problem remains that one cannot use keyword versioned extensions, > as one can in <script> tags via ;version=. > > This is the bug to fix. > > /be I can't tell from this discussion whether it would be acceptable for workers to simply inherit the JS version from their parent script. So if you declare a script with type="application/javascript; version=1.8" then any workers it creates (and any scripts that worker imports) would also use that version of the language. It seems to me that this version inheritance will probably be the desired behavior when scripts are opting-in to es-harmony, so even without an explicit version selection mechanism, it might be useful to do this. I have a patch if there is interest in this.

I doubt that would be the case. ECMA deliberately chose to make strictness not inherit into Function() scripts because that would require a way for scripts to know about their caller, and they didn't want to require that. Having script-version inherit that way (outside of the narrow special case of a direct eval, which is more of a pseudo-operator than an actual function call) would also require such a way. So I don't think workers should, or ever will, inherit script version from the context that created them.

In my opinion, the standard should be amended so that web workers work just like any other web content, relying on the MIME media type and parameters, and that ECMAScript version parameter (which is defined in RFC 4329, but provided with no list of values) values should be standardized (e.g. "3", "5", and "moz-harmony" could all be versions that SpiderMonkey will attempt to run). The version parameter of application/javascript should be supported for scripts on normal webpages without a type specified, in addition to in web workers.

Any news on this? I find the most intuitive way to be David Flanagan's suggestion "inherit the JS version from their parent script". Thats what the first thing I tried. We need this at http://repl.it/ and on other commercial products (codecademy.com, trybloc.com, etc.) using repl.it's engine to load emscripten compiled language interpreters into Web Workers while maintaining standard input. I believe this could be achieved by yielding on input to wait for a message event with the input data. At the moment we load them in iframes to be able to block on input using window.prompt. However on Chrome we were able to load the compiled sources in Workers while maintaining input using a hack which uses the not standard WebSQL Sync API as a way to share resources between the worker and the main thread. (https://github.com/replit/jsrepl/blob/fdaaff29f364c02968e41eb5c855c9d949ed881e/sandbox.js#L7-18). This is making our FF users unhappy because we tend to block the UI thread, at times make the browser unresponsive and most importantly could completely break on future releases of FF from disabling ArrayBuffer responses in sync XHRs (we currently fetch data files as ArrayBuffers).

It's currently looking like the next official version of javascript will do one of two things: 1. Define all new features in such a way that they are backwards compatible. I.e. you'll be able to use yield and let without having to opt-in to them. They will "just work". 2. Have an in-script way to opt in to the new features. Likely something like similar to the "use strict" that ES5 introduced. In either case we won't need any new worker-specific syntax to get access to these new things.

ES6 is still well into the future, is there a reason we can't make worker scripts inherit the parent script's js version today?

This is an old bug, but it doesn't look like there's any movement on it at all. Projects in Research tend to want to use the latest javascript features, and the fact that there's no way to choose js 1.8 in a worker is a common pain point.

Jonas, until we are there and can use yield everywhere, can we enable full JS support in workers?

I'm happy to do that in chrome workers by default. For non-chrome workers things are a lot trickier. If we enable it by default we'll risk random workers not working in Firefox due to using 'yield' as variable names. It'd also be a pain to disable in the future since disabling would then surely break Firefox-specific content. That leaves us with some sort of opt-in mechanism. We can't add any arguments to the Worker constructor since that will likely collide with future expansions to the Worker constructor. We could add some sort of header which lets the script opt in to the latest JS version, but everyone hates headers and they don't work during local developing from filesystem. The best thing I could think of would be a new "class". I.e. something like w = new JS18Worker(url); This is hideous but would give people a way to use the feature.

(In reply to Jonas Sicking (:sicking) from comment #21) > I'm happy to do that in chrome workers by default. We already do: https://mxr.mozilla.org/mozilla-central/source/dom/workers/RuntimeService.cpp#327

(In reply to Jonas Sicking (:sicking) from comment #21) > w = new JS18Worker(url); > > This is hideous but would give people a way to use the feature. This is fine. It should be something that is obviously moz-specific and anybody other than those of us that are writing parallel web applications specifically targeting Firefox won't notice.

Sigh, this just bit me when porting some code to run in a worker. Var is the new let. :( Any plans to either implement either "new JS18Worker" or some other way to make this possible? Any of the above, or something like |myworker.mozJSVersion = 1.8| / |Worker.prototype.mozDefaultJSVersion = 1.8|?

Vendo had the same problem when he was porting async IDB tests to sync IDB.

(In reply to Jan Varga [:janv] from comment #25) > Vendo had the same problem when he was porting async IDB tests to sync IDB. Using ChromeWorker is an obvious workaround per comment #22.

(In reply to Masatoshi Kimura [:emk] from comment #26) > (In reply to Jan Varga [:janv] from comment #25) > > Vendo had the same problem when he was porting async IDB tests to sync IDB. > > Using ChromeWorker is an obvious workaround per comment #22. I don't see how that helps, we need to test functionality of synchronous IDB API in standard workers first.

JS18Worker (or mozJSVersion or something) isn't standard either, no?

(In reply to Masatoshi Kimura [:emk] from comment #26) > Using ChromeWorker is an obvious workaround per comment #22. I happen to be writing unprivileged content, where this isn't an option.

Hm, ChromeWorker was unavailable from the content even with SpecialPowers. Perhaps inheriting JSVersion would be the most natural solution. (In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #14) > I doubt that would be the case. ECMA deliberately chose to make strictness > not inherit into Function() scripts because that would require a way for > scripts to know about their caller, and they didn't want to require that. > Having script-version inherit that way (outside of the narrow special case > of a direct eval, which is more of a pseudo-operator than an actual function > call) would also require such a way. So I don't think workers should, or > ever will, inherit script version from the context that created them. Function() inherits JSVersion from the caller. <script type="application/javascript;version=1.8"> new Function('let a = 1;'); // doesn't throw </script> After all, it is not an ES6 thing but a Mozilla-specific extension.

Forgot to hg add

Comment on attachment 746932 [details] [diff] [review] Inherit JSVersion from the caller in workers Function's behavior is not necessarily something to be emulated. Examining the stack, as Function does for this, makes for a good slowdown when it happens from JITted code. Note that JS-the-standard-language is moving away from having versions, and SpiderMonkey plans to move in that direction over time. So it actually *is* going to be an ES6 thing, at least if current trajectories hold. Adding f? for a few different JS hackers -- Luke as module owner, jorendorff also for considering this from the standards angle. I'm not comfortable with this change without getting some JS language people looking at this, and thinking deeper about it than I have time to think right now.

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #33) > Note that JS-the-standard-language is moving away from having versions, and > SpiderMonkey plans to move in that direction over time. So it actually *is* > going to be an ES6 thing, at least if current trajectories hold. If we can activate 'let' without the versioning (as speced in the ES6 draft), this patch will not be needed. But the current SpiderMonkey impl doesn't support it yet. SpiderMonkey doesn't even support ES6 'let' semantics. For example, 'let' in the global scope shouldn't stick a property on the global object (bug 589199). It was the reason I said "a Mozilla extension". When bug 855665 will be fixed?

I think this bug is WONTFIX. The new ES6 spec is the way to go and it enables both let and yield without version opt-in. TC39, the group designing ES, has declared versioned opt-in as an anti-pattern.

Comment on attachment 746932 [details] [diff] [review] Inherit JSVersion from the caller in workers Going to hold off reviewing this until we come to some agreement on the way forward here.

(In reply to Jonas Sicking (:sicking) from comment #35) > I think this bug is WONTFIX. The new ES6 spec is the way to go and it > enables both let and yield without version opt-in. > > TC39, the group designing ES, has declared versioned opt-in as an > anti-pattern. Indeed. The way forward (which admittedly will take some time) is probably to just remove JavaScript versions. Taking the freedom to set the dependency.

Hm, resetting owner didn't cleat the status?

I guess this won't be fixed before the next ESR branches? That's too bad …

Comment on attachment 746932 [details] [diff] [review] Inherit JSVersion from the caller in workers I have no clue about any of this; if it's good enough for you and bent, it's good enough for me.

Comment on attachment 746932 [details] [diff] [review] Inherit JSVersion from the caller in workers Review of attachment 746932 [details] [diff] [review]: ----------------------------------------------------------------- This code looks correct to me, but I'm against landing it, for a few reasons. 1. It's all rather implicit and subtle. 2. Who is this for? Web developers? How many are using Firefox-specific JS1.7+ features? We should focus on standard-track features. 3. Oh and there's great news on that front. The good stuff in JS1.7-1.85 is coming soon to standard JS. ES6 provides generators, array comprehensions, and more, in a standard form so it'll work cross-browser. We want to add those features to JS as we go, over roughly the next year, so you can use them without a version tag. In the long run, we want to remove versions from the JS engine entirely.

ES6 generators should be working in workers now. 'let' still doesn't work though :(

The reason why I'm proposing a patch for this bug is because I'm porting IDB to workers and I want to reuse the existing utests. Unfortunately they use a lot of nice JS features (in particular 'let') that are not available on workers. I don't want to change all of them so here a 'temporary' patch. The JSVersion used in workers is taken from a pref, so that if needed, it's possible to change it. Feedback?

Comment on attachment 828627 [details] [diff] [review] JSVersion from a pref Review of attachment 828627 [details] [diff] [review]: ----------------------------------------------------------------- Also, I thought Preferences didn't like it if the pref wasn't set in libpref/.../all.js. Has this changed? ::: dom/workers/RuntimeService.cpp @@ +195,5 @@ > } > #endif > > +// Worker JS Version > +#define PREF_WORKER_JS_VERSION "dom.worker.jsversion" Nit: The convention is to use 'workers'. @@ +205,5 @@ > + MOZ_ASSERT(NS_IsMainThread()); > + > + nsAdoptingCString version = Preferences::GetCString(PREF_WORKER_JS_VERSION); > + JSVersion jsVersion = version.IsEmpty() > + ? JSVERSION_DEFAULT : JS_StringToVersion(version); JS_StringToVersion() can return UNKNOWN if it couldn't parse the string. Please check that and set to JSVERSION_DEFAULT, otherwise the setVersion() call will MOZ_ASSERT. @@ +211,5 @@ > + Mutex* mutex = static_cast<Mutex*>(aClosure); > + MutexAutoLock lock(*mutex); > + gWorkersJSVersion = jsVersion; > + return 0; > +} Just a note, between this, bug 925849 and bug 925437, there is a definite pattern. I'll see if I can stick it into something generic. ::: dom/workers/test/test_jsversion.html @@ +4,5 @@ > +--> > +<!DOCTYPE HTML> > +<html> > +<head> > + <title>Test for JSVersion in workers</title> Please put the bug number somewhere in the test. @@ +26,5 @@ > + > + function doMagic() { > + var worker = new Worker('jsversion_worker.js'); > + worker.onmessage = function(evt) { > + ok(evt.data, 'All the tests are passed'); Nit: s/are // @@ +63,5 @@ > + > + SpecialPowers.setCharPref('dom.worker.jsversion', '1.7'); > + doMagic(false); > + SpecialPowers.setCharPref('dom.worker.jsversion', version ? version : ''); > + } Maybe add a test where the pref is set to an invalid value and the script should error.

Comment on attachment 828627 [details] [diff] [review] JSVersion from a pref Review of attachment 828627 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/RuntimeService.cpp @@ +195,5 @@ > } > #endif > > +// Worker JS Version > +#define PREF_WORKER_JS_VERSION "dom.worker.jsversion" Yeah, let's use a boolean 'dom.workers.latestJSVersion' that is false by default. @@ +199,5 @@ > +#define PREF_WORKER_JS_VERSION "dom.worker.jsversion" > +JSVersion gWorkersJSVersion; > + > +static int > +JSVersionPrefChanged(const char* aPrefName, void* aClosure) I'd prefer not to use a lock here. Once a worker is created with a particular version its child workers should run the same version. Switching would probably cause problems. So, since we're going to keep all child workers using their parents' version let's just make this a main-thread-only getter. Then we can send the version along with the CompileScriptRunnable.

What about this patch?

Comment on attachment 8337667 [details] [diff] [review] JSVersion from a pref Review of attachment 8337667 [details] [diff] [review]: ----------------------------------------------------------------- This looks good, but bug 936700 added a new pref cache that you should use instead.

better approach.

Ben this is your patch + mine. Can you review your own code? :)

Comment on attachment 8339868 [details] [diff] [review] changes.patch Review of attachment 8339868 [details] [diff] [review]: ----------------------------------------------------------------- This looks fine to me but I'd like your feedback too :)

Comment on attachment 8339868 [details] [diff] [review] changes.patch Review of attachment 8339868 [details] [diff] [review]: ----------------------------------------------------------------- lgtm

Backed out for B2G mochitest-8 orange. https://hg.mozilla.org/integration/mozilla-inbound/rev/7b3b24386a3f https://tbpl.mozilla.org/php/getParsedLog.php?id=31745605&tree=Mozilla-Inbound

Ok the tests now should be fixed for b2g emulator too. https://tbpl.mozilla.org/?tree=Try&rev=fd56728569a0

Comment on attachment 8345441 [details] [diff] [review] patch wrong name for this patch. https://hg.mozilla.org/integration/mozilla-inbound/rev/10800c0df8b5

(In reply to Andrea Marchesini (:baku) from comment #56) > Comment on attachment 8345441 [details] [diff] [review] > patch > > wrong name for this patch. > > https://hg.mozilla.org/integration/mozilla-inbound/rev/10800c0df8b5 had to backout this change for build failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=31795767&tree=Mozilla-Inbound maybe we need to touch the clobber file here ?

Sorry, wrong merge. No clobber file needed. https://hg.mozilla.org/integration/mozilla-inbound/rev/6d5dc2ce8569