User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.8.1.5) Gecko/20070713 Firefox/2.0.0.5 Build Identifier: Right now it appears that the JavaScript in XBL is executed using JavaScript 1.6. It would be nice to be able to use the stuff from 1.7 and 1.8 in XBL. We could add an attribute to the implementation element to specify what version of javascript you want, or perhaps just make XBL always execute under the latest javascript version? I did a search for "let" and "yield" over *.xml and didn't see any usage of variables with these names. Reproducible: Always

Please attach a testcase

Open up your js console and go to this url: http://skrul.com/bug389322/test.xul It uses the following XBL: http://skrul.com/bug389322/test.xml and the error console should say: Error: missing ; after for-loop initializer Source File: http://skrul.com/bug389322/test.xml Line: 18, Column: 9 Source Code: for (let i = 0; i < 3; i++) {

Please actually attach the testcase to the bug. First the XBL, then the file linking to it, pointing to the actual XBL attachment so that it can be run without downloading. The testcase needs to test JS 1.7 features in all parts of XBL: * methods * properties * fields * event handlers * constructors/destructors because they will in fact need pretty different fixes, from what I can see. Once we have a testcase like that, I'd be happy to work on a fix.

Is this testcase acceptable?

That testcase is great. I have this converted to a mochitest, and a partial patch. I'm not sure when I'll have a chance to finish this, though, so if someone wants to pick it up, let me know.

This doesn't compile yet, and I need this tree.

bz: This is a pretty minor update on what you had, is there more to do?

Nevermind, still have more build errors to chase. More in a bit.

Yeah. There are three other things to do: 1) Fix all the other consumers of this API to deal with passing in a version 2) Just calling JS_SetVersion is sadly not the right thing to do. See the existing nsJSContext code that deals with versions (in two places). It does all sorts of E4X stuff as well, etc. That mess should be factored out into a helper method or macro (or maybe helper object whose ctor sets the version and whose dtor restores the old version). Restoring is important, by the way. 3) Set the version to JSVERSION_LATEST right before we start compiling (and maybe installing?) the binding implementation, so that the version sets when we compile all those properties and methods are just fast no-ops. Restore when done with that.

Would just doing something like this work? --- a/content/xbl/src/nsXBLDocumentInfo.cpp +++ b/content/xbl/src/nsXBLDocumentInfo.cpp @@ -265,12 +265,15 @@ nsXBLDocGlobalObject::SetContext(nsIScri nsresult rv; rv = aScriptContext->InitContext(nsnull); NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Script Language's InitContext failed"); aScriptContext->DidInitializeContext(); // and we set up our global manually mScriptContext = aScriptContext; + + // Set to latest version of JavaScript + mScriptContext->SetDefaultLanguageVersion(JSVERSION_LATEST); }

(That works for me. At least I can use let in xbl. I can't quite get any of the xbl mochitests to run.)

I was hoping for something that trivial early on, but comment #10 here seems to suggest it is not adequate.

Something's wrong if too much code is required... /be

Edward's suggestion would work if you don't care about using JS 1.7 in fields and event handlers. That is, it'll work for properties, methods, constructors, and destructors. That's assuming that the version only affects compile-time. If it affects runtime, then other things would also be affected. This should have been clear from running the test in my patch. Edward, please feel free to contact me about your issues with running mochitest; it should Just Work. > Something's wrong if too much code is required... The main wrongness is twofold: not all of nsIScriptContext takes versions (so we need an API change to do this, hence need to update all callers) and the existing version stuff in nsJSContext was copy/pasted all over, so needs to either be copy/pasted to the new code or refactored properly so it can be sanely reused. I suppose an additional "wrongness" is that this is going through nsIScriptContext to start with, eh? ;)

(In reply to comment #15) > Edward's suggestion would work if you don't care about using JS 1.7 in fields > and event handlers. That is, it'll work for properties, methods, constructors, > and destructors. That's assuming that the version only affects compile-time. > If it affects runtime, then other things would also be affected. Version is saved at compile-time in executable objects, and selected from their private data when they are run. So you don't need version params for any but the Compile* methods. > This should have been clear from running the test in my patch. Edward, please > feel free to contact me about your issues with running mochitest; it should > Just Work. What are the test results with the latest patch? > > Something's wrong if too much code is required... > > The main wrongness is twofold: not all of nsIScriptContext takes versions Only the Compile and Evaluate (not the Execute or Call, etc.) APIs need version parameters. > I suppose an additional "wrongness" is that this is going through > nsIScriptContext to start with, eh? ;) Could use bare JS API from XBL for sure. Why not? /be

> Only the Compile and Evaluate (not the Execute or Call, etc.) APIs need version > parameters. Well, CompileEventHandler doesn't have a version parameter... Nor does CompileFunction. Those are the API changes I made in my first-cut patch. > Could use bare JS API from XBL for sure. Why not? It's an even bigger change than fixing nsIScriptContext to deal with versions for everything as needed, to be honest. Especially if we want to get all the security stuff that nsIScriptContext does for us right...

Unless you meant to do the version-setting via JSAPI while continuing to use nsIScriptContext for the actual compilation, etc... That might be doable.

Yes, bare JS API where needed, followup bugs for later. For HTML, we Ecma folk think it's likely that event handlers, like type-less or unversioned scripts, will need to be JS1-ish forever (ES3). Even with the default scripting language content header / meta http-equiv thingie that HTML 4 spec'ed, which no one uses. Not sure this matters for XBL. /be

Hrm, it would be cool if we'd get XBL to use the same JS version as chrome. We currently can't port Firefox code (that sometimes uses JS 1.7) to SeaMonkey prefs because we load script via loadSubScript() from an XBL handler :(

Requesting blocking -- it's a painful developer confusion point, and should have an easy fix.

This bug needs a better owner than me, for now. I'll pick it up soon, if I can.

Don't think this is blocking, but wanted to avoid the confusion and letting extension developers use the new cewl JS features.

Comment on attachment 303473 [details] [diff] [review] Let's just do this Looks good to me.

Comment on attachment 303473 [details] [diff] [review] Let's just do this I think I steered mhammond in a bad direction, or (memory fails) perhaps the code already was going that way when he came along: instead of |= JSVERSION_HAS_XML in callers, requiring this ugly recovery of the XML option from the version, it might be better to allow JSVERSION_HAS_XML to be included in a version passed to JS_SetVersion. Followup bug fodder? r=me on the patch, anyway -- thanks! /be

> Followup bug fodder? Bug 417871 filed. Checked the patch in.

Typo in this patch breaks the building of the python dom extension. "extensions/python/dom/src/nsPyContext.cpp", line 478. The ";" in the first patch hunk for this file should have been a comma: "+ PRUint32 aVersion;" "+ PRUint32 aVersion,"

Doh. Checked in a fix for that.