This affects HTML, XUL, and DOM code. My main interest is in supporting Python
in XUL, only (not in HTML). There's no call on the web for other languages than
JS, unless you trip across some VBScript on an MS-related site, or on an intranet.
Anyway, lots to do here. Plan of action soon, bug dependencies first.
/be

Kind of a feature request.
I want support for client side PerlScript (TM), or a Mozilla Brewed embeded
Perl in HTML in Mozilla (if ActiveState protests).
Basically to use Mozilla as a GUI interface/toolkit/renderer to Perl for
Applications. I think Mozilla is much better and easier to use (HTML) for
creating GUIs vs TK and Native OS widgets or WxWindows or other systems used
for GUIs in Perl. I think Mozilla would exceed for this purpose because it is
portable and a bunch of other things.
Note: MS IE has it as a ActiveX control for PerlScript.
I think Mozilla should use Perl because there are people who may know Perl and
HTML but little to no Javascript. Also Perl can be used to create full feature
programs, and uses has alot of existing capabilities in its Modules on CPAN
which is not possible in Javascript.

This patch is close to building (it did a little earlier today, but this one
doesn't quite). It it a fair way towards language independent nsIScript*
interfaces. The patch is mainly attached so a copy exists other than on my
machine, but any comments appreciated. Should this work be done on a branch?

Mark's been busy, and I forgot to give the URL of the plan of action. It
evolved via email and IRC into
http://wiki.mozilla.org/Breaking_the_grip_JS_has_on_the_DOM -- all praise to
Mark for this analysis, which shows great perseverance and talent for handling
complexity and dividing and conquering.
/be

Comment on attachment 193995[details][diff][review]
Updated patch that builds and calls Python
Couple of quick comments:
language= is deprecated in HTML, and I didn't see support for
type=application/x-python or whatever the best MIME type(s) might be.
You really want nsCOMPtrs in places such as XULContentSinkImpl::OpenScript, for
scriptContext -- right? Otherwise early returns (including those hidden in
evil NS_ENSURE_*) look like they will leak. Plus, the ugly manual Release
calling goes away.
More when I have time.
/be

More progress - Python can now get basic properties etc from the DOM! This
patch also includes a restructure of pyxpcom, with the bulk being in a shared
library available for use by any other lib - including my new "pydom" lib.
Addressing Brendan's questions:
* I only used language= as it was the quickest way to hook Python up, rather
than the correct way. Interestingly, my patches have not yet touched
nsIScriptLoader. The best way to handle this language instantiation isn't yet
clear to me.
* I believe the refcounting is correct in XULContentSinkImpl::OpenScript. What
may have confused the issue was my GetLanguageContext() returning an
un-ref-counted object via an out param. I've now moved back to 2 methods that
return pointers directly, closer to the existing GetContext/GetGlobalJSObject
pattern, and added a few extra comments. Please let me know if you still think
it is incorrect.
I will update the design docs to reflect my progress tomorrow.

Fixing a number of issues as I find them. The "void *aScopeObject" can not
change to the nsIScriptGlobalObject, as sometimes different objects are used.
Seamonkey *almost* starts :) Cleanup should be much better (although shutdown
still reports more leaks than without the patch)

Version strings have all been replaced with a PRUint32. This looks like a nice
win (remove lots of conversions, and remove a few hacks for e4x)
New interface "nsIScriptLanguage" (maybe this should be
nsIScriptLanguageFactory?)
nsIScriptObjectFactory now returns instances of nsIScriptLanguage, which is
used to create the nsIScriptContext. New nsJSLanguage.cpp/.h implementing this
for JS.
nsDOMScriptObjectFactory now queries a new xpcom category to find an
nsIScriptLanguage given a mime-type (note the detection of JS mime-types
remains external for perf reasons)
Python implementation of nsIScriptLanguage now in a module and is loaded
dynamically. No hard-coding for Python remains.
Dropped support for language="python" - now only type="application/x-python" is
used (that is the mine-type registered by that language's module - it could be
anything)
We expect to have a branch for this stuff in the next couple of days

Comment on attachment 221577[details][diff][review]
tracking the trunk...
I only really looked at the changes under content/xbl for the time being. No time for anything else until I get back, at least.
For future reference, patches created with the -p option (in addition to the -u8 you used) are a _lot_ easier to review.
>Index: content/xbl/src/nsXBLContentSink.cpp
>@@ -864,16 +864,18 @@
>+ // XBL doesn't do multi-language yet.
>+ prototype->mScriptTypeID = nsIProgrammingLanguage::JAVASCRIPT;
This isn't an XBL issue; we're just creating XUL nodes in the usual way from a prototype. This should just do whatever the XUL content sink does in similar circumstances, imo.
>Index: content/xbl/src/nsXBLDocumentInfo.cpp> virtual void SetContext(nsIScriptContext *aContext);
Why is that virtual? That should now be a private non-virtual method, right?
>+ virtual nsresult SetScriptContext(PRUint32 lang_id, nsIScriptContext *aContext) {
Please put the impl in the C++ file.
Same comments for GetScriptContext/GetContext and GetScriptGlobal/GetGlobalJSObject.
>+nsXBLDocGlobalObject::SetContext(nsIScriptContext *aScriptContext)>+ NS_ASSERTION(aScriptContext, "Must provide a context");
Not only is this in the else of a null-check of aScriptContext, but you've already dereferenced aScriptContext (on the previous line). Just remove this assert.
>+ // NOTE: We init this context with a NULL global - this is subtly
>+ // different than nsGlobalWindow which passes 'this'
Yes, that's what we're doing. But _why_? Explaining that would be a much more useful comment.
>+ nsresult rv;
>+ rv = aScriptContext->InitContext(nsnull);
>+ NS_ASSERTION(NS_SUCCEEDED(rv), "InitContext failed");
Er... if it can't fail, why does it return nsresult? If it can fail, why can you assert that it didn't?
>+ // and we setup our global manually
"set up"
>+nsXBLDocGlobalObject::EnsureScriptEnvironment(PRUint32 aLangID)
>+ if (aLangID != nsIProgrammingLanguage::JAVASCRIPT) {
This file uses 2-space indent, not 4-space.
>+ NS_ENSURE_SUCCESS(rv, nsnull);
s/nsnull/rv/, right?
>+ NS_ENSURE_SUCCESS(rv, nsnull);
Again.
Why bother with stdid here? Just use aLangID, since it's guaranteed to be JAVASCRIPT?
>+ // nsJSEnvironment set the error reporter to NS_ScriptErrorReporter
What does this comment mean? While true, it doesn't seem relevant here, does it? Unless you mean that we want a different error reporter and want to override the one nsJSEnvironment set? (I'd question this last statement, but that's the only reason I can think of for this comment.)
I'd love to see API documentation for EnsureScriptEnvironment() so I could tell whether this implementation is correct.
>+ // This whole fragile mess is predicated on the fact that
>+ // GetContext() will be called before GetScriptObject() is.
So... I know the comment was already there, but what's GetScriptObject()? I don't see it in XBL-land... Does this mean some other method that relies on mScriptContext being inited?
>+ nsresult rv = EnsureScriptEnvironment(nsIProgrammingLanguage::JAVASCRIPT);
>+ NS_ASSERTION(NS_SUCCEEDED(rv), "Failed to setup JS!?");
If it can fail, you can't assert that it succeeded.
>Index: content/xbl/src/nsXBLProtoImplField.cpp
>+ if (!::JS_DefineUCProperty(cx, NS_REINTERPRET_CAST(JSObject *, aScriptObject),
NS_STATIC_CAST, right?
>Index: content/xbl/src/nsXBLPrototypeHandler.cpp>+ // XUL is language aware, so although XBL isn't, we still need to
>+ // help when we can.
Meaning what?
>+ // XXX - apparently we should not be using the global as the scope - what
>+ // should we use?
This needs to be fixed, right? What do you mean by "the scope"?
>+ nsContentUtils::GetEventArgNames(kNameSpaceID_XBL, onEventAtom, &argCount,
>+ &argNames);
Not completely related, but I have to ask. What's the deal with the error event in GetEventArgNames? And does SVG never fire error events?
>+ rv = boundContext->CompileEventHandler(onEventAtom, argCount, argNames,
> handlerText, bindingURI.get(),
>+ mLineNumber, handler);
Again not completely related, but what happened to the aShared arg? We have callers that currently pass false, no?
>- NS_NewJSEventListener(boundContext, boundGlobal->GetGlobalJSObject(),
>- aReceiver, getter_AddRefs(eventListener));
>+ NS_NewJSEventListener(boundContext, scope,
>+ scriptTarget, getter_AddRefs(eventListener));
Why the change from aReceiver to scriptTarget here?

Hi Boris,
Thanks for the review. Anything I haven't commented on has been corrected
(but note that DOM_AGNOSTIC3_BRANCH is where this is now happening)
> For future reference, patches created with the -p option (in addition to the
> -u8 you used) are a _lot_ easier to review.
Sorry about that - noted!
> >Index: content/xbl/src/nsXBLContentSink.cpp
> >@@ -864,16 +864,18 @@
> >+ // XBL doesn't do multi-language yet.
> >+ prototype->mScriptTypeID = nsIProgrammingLanguage::JAVASCRIPT;> This isn't an XBL issue; we're just creating XUL nodes in the usual way from a
> prototype. This should just do whatever the XUL content sink does in similar
> circumstances, imo.
You mean that the XBL code should look at the "script-type" attribute instead
of hard-coding JS? Having XBL do anything other than assume JS was explicitly
out of the scope of this phase of the project (and IIUC, outside the scope of
the current XBL implementation!)
Or am I misunderstanding your comment?
> >+ // NOTE: We init this context with a NULL global - this is subtly
> >+ // different than nsGlobalWindow which passes 'this'> Yes, that's what we're doing. But _why_? Explaining that would be a much more
> useful comment.
That is a very good question :) I added this comment as when initially
reading the code that subtlety slipped by me. I have changed it to:
// NOTE: We init this context with a NULL global, so we automatically
// hook up to the existing nsIScriptGlobalObject global setup by
// nsGlobalWindow.
which I think is a little better.
> >+ nsresult rv;
> >+ rv = aScriptContext->InitContext(nsnull);
> >+ NS_ASSERTION(NS_SUCCEEDED(rv), "InitContext failed");> Er... if it can't fail, why does it return nsresult? If it can fail, why can
> you assert that it didn't?
My thinking was: failure is not really expected, but is possible. Failure
generally reflects a programming error (eg, buggy or incomplete language
implementation), but may also include things like "out of memory".
The way I see it, people trying to implement a new language will be thankful
for these assertions, and the only people that will curse them are developers
using a debug build to track down out-of-memory related issues.
In an effort to keep both of us happy, I've changed that to:
NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Script Language's InitContext failed");
Does that sound reasonable?
> >+ // nsJSEnvironment set the error reporter to NS_ScriptErrorReporter> What does this comment mean? While true, it doesn't seem relevant here, does
> it? Unless you mean that we want a different error reporter and want to
> override the one nsJSEnvironment set? (I'd question this last statement, but
> that's the only reason I can think of for this comment.)
The latter. Again, I'm not 100% why this is implemented this way - I was just
documenting the previous behaviour that I carried through.
> I'd love to see API documentation for EnsureScriptEnvironment() so I could tell
> whether this implementation is correct.
I added the following to nsIScriptGlobalObject - is that what you had in mind?
* Ensure that the script global object is initialized for working with the
* specified script language ID. This will setup the nsIScriptContext
* and 'script global' for that language, allowing these to be fetched
* and manipulated.
* @return NS_OK if successful; error conditions include that the language
* has not been registered, as well as 'normal' errors, such as
* out-of-memory
> >+ nsresult rv = EnsureScriptEnvironment(nsIProgrammingLanguage::JAVASCRIPT);
> >+ NS_ASSERTION(NS_SUCCEEDED(rv), "Failed to setup JS!?");> If it can fail, you can't assert that it succeeded.
As above - but this is a better example. JS is built in, so should always be
available. If JS fails to initialize, I would think the developer would
want to know about it.
Either way, that one also got changed to NS_WARN_IF_FALSE - let me know if
that too is unsuitable and I'll just remove the check
> >Index: content/xbl/src/nsXBLPrototypeHandler.cpp> >+ // XUL is language aware, so although XBL isn't, we still need to
> >+ // help when we can.> Meaning what?
Ooops - that was a dead comment - removed!
> >+ // XXX - apparently we should not be using the global as the scope - what
> >+ // should we use?> This needs to be fixed, right?
Again, I'm afraid I just blindly copied what was there.
nsXBLPrototypeHandler.cpp currently says:
// XXX: Don't use the global object!
rv = nsContentUtils::XPConnect()->WrapNative(cx, global, aReceiver,
NS_GET_IID(nsISupports),
getter_AddRefs(wrapper));
In the above code, the 'global' is equivilent to the new code:
void *scope = boundGlobal->GetScriptGlobal(stID);
I'm afraid I've no idea what the original author of that comment means.
I'm happy to remove the comment if you agree.
> What do you mean by "the scope"?
I tried to re-use the existing terminology - the second arg to WrapNative
is declared 'jsobject * aScope' and the term is used in the existing
nsIScriptContext methods EvaluateString() and ClearScope(). It is something
like the "language namespace for the script global" - but 'namespace' is too
overloaded for that to be precise.
> >+ nsContentUtils::GetEventArgNames(kNameSpaceID_XBL, onEventAtom, &argCount,
> >+ &argNames);> Not completely related, but I have to ask. What's the deal with the error
> event in GetEventArgNames?
When an error event fires, it has always been passed 3 args. Currently
though, the event handler (and all others) is compiled with a single
argument - JS obviously handles a function being passed more args than it
was compiled with.
However, this doesn't work for Python - you can't pass a function more
args than it was compiled with (unless the function was explicitly declared
as varargs)
Now when we compile the error event handler, we correctly pass the 3
arg names.
> And does SVG never fire error events?
SVG appears to use 'onsvgerror'. I made no attempt to confirm that SVG
was consistent in this regard for any events - indeed, I haven't gone
anywhere near svg.
> >+ rv = boundContext->CompileEventHandler(onEventAtom, argCount, argNames,
> > handlerText, bindingURI.get(),
> >+ mLineNumber, handler);> Again not completely related, but what happened to the aShared arg? We have
> callers that currently pass false, no?
Correct. Brendan and I agreed to a subtle change of semantics here.
Previously when aShared was False, the function was compiled directly
into the event target - ie:
::JS_CompileUCFunctionForPrincipals(mContext,
aShared ? nsnull : target, jsprin,
...
CompileEventHandler is now responsbile *only* for compilation of the
function, while BindEventHandler copies the function to the target.
This means that now, BindEventHandler must always be called. If aShared
was previously FASLE, it will BindEventHandler will now be called exactly
once. If it is TRUE, then as in the past, it may be called any number of
times with the same compiled handler.
> >- NS_NewJSEventListener(boundContext, boundGlobal->GetGlobalJSObject(),
> >- aReceiver, getter_AddRefs(eventListener));
> >+ NS_NewJSEventListener(boundContext, scope,
> >+ scriptTarget, getter_AddRefs(eventListener));> Why the change from aReceiver to scriptTarget here?
The existing code always passed aReceiver, but the script object
passed to CompileEventHandler depended on if the target was the
'root window' or not (after doing the JS specific WrapNative() magic.)
This has now been abstracted so the WrappedNative() magic happens inside
nsIScriptContext - as a result, when the event target is the 'root window',
we pass our 'script global' for the window rather than the window itself.

> Or am I misunderstanding your comment?
I think so. The code in question handles the case of a <script> element in the <content> section of an XBL binding. It should be treated like any other <script> element. In particularly, execution of that script is NOT handled via XBL itself, but rather via the normal codepaths for executing <script> elements when they are inserted in the DOM, so the JS-only restrictions on XBL do not apply.
> // NOTE: We init this context with a NULL global, so we automatically
> // hook up to the existing nsIScriptGlobalObject global setup by
> // nsGlobalWindow.
But that's not what InitContext does when passed null... and in fact, there's no nsGlobalWindow around here, is there? This script context is not associated with one...
> Does that sound reasonable?
Sure.
> The latter. Again, I'm not 100% why this is implemented this way - I was just
> documenting the previous behaviour that I carried through.
OK. Want to file a followup bug to look into this and nuke the XBL reporter if we can?
> I added the following to nsIScriptGlobalObject - is that what you had in mind?
Yes, with s/setup/set up/
> I'm happy to remove the comment if you agree.
No, keep the comment. File a bug to investigate, cite the bug# in the comment.
> If aShared was previously FASLE, it will BindEventHandler will now be called
> exactly once.
Is this an observation, or a requirement on the caller?
I'm failing to follow the NS_NewJSEventListener stuff, but then I'm running on a week and a half of 3-hour nights... Please make sure jst checks that part out, ok?

> > Or am I misunderstanding your comment?> I think so. The code in question handles the case of a <script> element
> in the <content> section of an XBL binding. It should be treated like
> any other <script> element. In particularly, execution of that script
> is NOT handled via XBL itself, but rather via the normal codepaths for
> executing <script> elements when they are inserted in the DOM, so the
> JS-only restrictions on XBL do not apply.
I'm afraid I still miss what else needs to be done. XUL looks at the
'script-type' attribute of the node, and if not there, it walks up the tree
until it finds a node that specifies it. Further, XUL looks at the 'version'
attribute. My patch, as it stands, simply hard-codes javascript, which still
seems completely appropriate to me and far simpler than adding all that code. The patch in question is:
+ // XBL doesn't do multi-language yet.
+ prototype->mScriptTypeID = nsIProgrammingLanguage::JAVASCRIPT;
Can you give me another clue as to what you think it should look like?
> > // NOTE: We init this context with a NULL global, so we automatically
> > // hook up to the existing nsIScriptGlobalObject global setup by
> > // nsGlobalWindow.
>
> But that's not what InitContext does when passed null... and in
> fact, there's no nsGlobalWindow around here, is there? This script
> context is not associated with one...
OK - I give up - what *does* it do? The Python implementation only sees
InitContext called with NULL for an inner window. The key distinction
nsJSEnvironment makes is that with a NULL global it doesn't call
xpc->InitClassesWithNewWrappedGlobal. My knowledge of the JS implementation
is still too limited to know exactly what that means.
As I mentioned, I only added the comment as it was a subtlety that escaped
me in the early days of this work. I'm happy to remove the comment if you
think it only serves to confuse, but I'm afraid I don't know what a 'better'
comment would say.
> > The latter. Again, I'm not 100% why this is implemented this
> > way - I was just documenting the previous behaviour that I carried
> > through.
>
> OK. Want to file a followup bug to look into this and nuke the
> XBL reporter if
> we can?
No problem - bug 339647, and bug cited in the comment.
> > I'm happy to remove the comment if you agree.
>
> No, keep the comment. File a bug to investigate, cite the bug#
> in the comment.
Done - bug 339649> > If aShared was previously FASLE, it will BindEventHandler will
> > now be called exactly once.
>
> Is this an observation, or a requirement on the caller?
An observation.
> I'm failing to follow the NS_NewJSEventListener stuff, but then
> I'm running on a week and a half of 3-hour nights... Please make
> sure jst checks that part out, ok?
jst has already reviewed that portion of the patch, although I will be sure to
bring your concerns to his attention.
Cheers,

> XUL looks at the 'script-type' attribute of the node....
You mean the XUL content sink? That's what this code (in the XBL content sink) should do too. The point of the XBL content sink is to be like the XUL sink except for treating elements in the XBL namespace in a special way.
> OK - I give up - what *does* it do?
It initializes the context. If there's a global passed in, it also does some setup on said global; if not, it naturally doesn't do it. Perhaps this part should be refactored somehow, after this patch lands....
I think there should definitely be a comment somewhere -- either here or on nsIScriptContext documenting what InitContext does. The latter is preferable, if people are going to try implementing this API correctly. Right now, it's not obvious from the API docs that the script global is allowed to be null, for example.

I found you "plan of action" because I was searching mozilla.org for "timeout".
It looks like you've taken all the code dealing with timeouts and put it in one
place. Does this mean you're going to give us poor "end users" the ability to
set timout intervals? Pleasepleasepleasepleaseplease?
(heh)
Jim H. (aka CuriousJ)

The answer is nothing, because I just WONTFIXed it :-)
We're currently de-agnostifying the DOM for performance reasons. JS is ubiquitous on the web, and that's not going to change anytime soon, so there's no point in carrying the baggage necessary for multiple scripting engines that never materialized.