Refactor domita_test to use module system where possible.

Refactor domita_test to use module system where possible.

Moved the JavaScript in domita_test into a separate file to make it
easier to edit (e.g., in editor modes) and also to help clarify what
remains in domita_test.html -- which includes stuff that is actually
part of the embedded test cases.

[ihab.awad, 2009-09-04 20:58:54]

Adding mikesamuel to the reviewer list -- Mike, does this rearrangement of
domita_test look reasonable?

[MikeSamuel, 2009-09-04 23:26:01]

http://codereview.appspot.com/115054/diff/1/3
File tests/com/google/caja/plugin/domita_test.js (right):

http://codereview.appspot.com/115054/diff/1/3#newcode1
Line 1: function makeXhr() {
This file needs a copyright.

Is there any way to get a meaningfull diff?

[ihab.awad, 2009-09-04 23:28:57]

http://codereview.appspot.com/115054/diff/1/3
File tests/com/google/caja/plugin/domita_test.js (right):

http://codereview.appspot.com/115054/diff/1/3#newcode1
Line 1: function makeXhr() {
On 2009/09/04 23:26:01, MikeSamuel wrote:
> This file needs a copyright.

Oops, tx, will do.

> Is there any way to get a meaningfull diff?

I did an "svn cp" from the original domita_test.html then replaced that with the
new JS-only stuff, hoping that this would be understandable. Unfortunately, it
didn't work. I did rearrange a couple bits, but not too many; just enough to get
the synchronization to work out.

Let me try a couple of things...

[MikeSamuel, 2009-09-05 00:04:04]

http://codereview.appspot.com/115054/diff/1/2
File tests/com/google/caja/plugin/domita_test.html (left):

http://codereview.appspot.com/115054/diff/1/2#oldcode86
Line 86: </script>
Where did this stuff above here go?

http://codereview.appspot.com/115054/diff/1/2#oldcode343
Line 343: /**
And all the stuff below here?

[ihab.awad, 2009-09-05 00:14:07]

http://codereview.appspot.com/115054/diff/1/2
File tests/com/google/caja/plugin/domita_test.html (left):

http://codereview.appspot.com/115054/diff/1/2#oldcode86
Line 86: </script>
On 2009/09/05 00:04:05, MikeSamuel wrote:
> Where did this stuff above here go?

To the bottom of domita_test.js.

http://codereview.appspot.com/115054/diff/1/2#oldcode343
Line 343: /**
On 2009/09/05 00:04:05, MikeSamuel wrote:
> And all the stuff below here?

To domita_test.js also, but in function initialize() since it now gets to wait
until the promise for the necessary modules is resolved.

[ihab.awad, 2009-09-10 22:44:24]

Snapshotted with copyright in domita_test.js.

Ping.

[MikeSamuel, 2009-09-11 17:59:03]

Ok.  I see the bits that moved around now.

http://codereview.appspot.com/115054/diff/12/15
File src/com/google/caja/cajita-module.js (right):

http://codereview.appspot.com/115054/diff/12/15#newcode45
Line 45: }
Can this be consolidated with the same code in domita.js?

http://codereview.appspot.com/115054/diff/12/15#newcode60
Line 60: else {
Please fix braces

http://codereview.appspot.com/115054/diff/12/16
File src/com/google/caja/serverjs-sandbox.js (right):

http://codereview.appspot.com/115054/diff/12/16#newcode111
Line 111: && typeof load !== 'undefined') {
Why not
if (env !== undefined
    && valijaModule !== undefined
    && load !== undefined)?

http://codereview.appspot.com/115054/diff/12/16#newcode114
Line 114: }
I would rather functions not be declared inside conditions since that is not
legal ES3.1.

http://codereview.appspot.com/115054/diff/12/17
File src/com/google/caja/valija-cajita.js (right):

http://codereview.appspot.com/115054/diff/12/17#newcode524
Line 524: // passing an 'outers' as a Cajita-level parameter, the result should
be the
result?

http://codereview.appspot.com/115054/diff/12/14
File tests/com/google/caja/plugin/domita_test.js (right):

http://codereview.appspot.com/115054/diff/12/14#newcode95
Line 95: ___.setNewModuleHandler(___.makeNormalNewModuleHandler());
What's this?

http://codereview.appspot.com/115054/diff/12/14#newcode360
Line 360: initialize(serverJsSandboxModule, valijaModule)
missing ;

http://codereview.appspot.com/115054/diff/12/14#newcode361
Line 361: })
missing ;

[MikeSamuel, 2009-09-11 18:03:19]

http://codereview.appspot.com/115054/diff/12/15
File src/com/google/caja/cajita-module.js (right):

http://codereview.appspot.com/115054/diff/12/15#newcode45
Line 45: }
On 2009/09/11 17:59:04, MikeSamuel wrote:
> Can this be consolidated with the same code in domita.js?

Actually, can it be consolidated with the same code in domita.js and in
domita_test.js, and elsewhere.

)> find src tests/ -name \*.js -o -name \*.html | xargs egrep '\bXMLHTTP\b'
src/com/google/caja/plugin/domita.js:            'MSXML2.XMLHTTP.5.0',
'MSXML2.XMLHTTP.4.0',
src/com/google/caja/plugin/domita.js:            'MSXML2.XMLHTTP.3.0',
'MSXML2.XMLHTTP',
src/com/google/caja/plugin/domita.js:            'MICROSOFT.XMLHTTP.1.0',
'MICROSOFT.XMLHTTP.1',
src/com/google/caja/plugin/domita.js:            'MICROSOFT.XMLHTTP'];
src/com/google/caja/demos/lolcat-search/search.html:             
'MSXML2.XMLHTTP.5.0', 'MSXML2.XMLHTTP.4.0', 'MSXML2.XMLHTTP.3.0',
src/com/google/caja/demos/lolcat-search/search.html:             
'MSXML2.XMLHTTP', 'MICROSOFT.XMLHTTP.1.0', 'MICROSOFT.XMLHTTP.1',
src/com/google/caja/demos/lolcat-search/search.html:             
'MICROSOFT.XMLHTTP'];
src/com/google/caja/demos/oddities/interpreter_oddities.html:             
"MSXML2.XMLHTTP.5.0", "MSXML2.XMLHTTP.4.0", "MSXML2.XMLHTTP.3.0",
src/com/google/caja/demos/oddities/interpreter_oddities.html:             
"MSXML2.XMLHTTP", "MICROSOFT.XMLHTTP.1.0", "MICROSOFT.XMLHTTP.1", 
src/com/google/caja/demos/oddities/interpreter_oddities.html:             
"MICROSOFT.XMLHTTP"];
src/com/google/caja/demos/slides/index.html:              'MSXML2.XMLHTTP.5.0',
'MSXML2.XMLHTTP.4.0', 'MSXML2.XMLHTTP.3.0',
src/com/google/caja/demos/slides/index.html:              'MSXML2.XMLHTTP',
'MICROSOFT.XMLHTTP.1.0', 'MICROSOFT.XMLHTTP.1',
src/com/google/caja/demos/slides/index.html:              'MICROSOFT.XMLHTTP'];
tests/com/google/caja/plugin/domita_test.js:        'MSXML2.XMLHTTP.5.0',
'MSXML2.XMLHTTP.4.0', 'MSXML2.XMLHTTP.3.0',
tests/com/google/caja/plugin/domita_test.js:        'MSXML2.XMLHTTP',
'MICROSOFT.XMLHTTP.1.0', 'MICROSOFT.XMLHTTP.1',
tests/com/google/caja/plugin/domita_test.js:        'MICROSOFT.XMLHTTP'];

[metaweta, 2009-09-11 22:56:52]

http://codereview.appspot.com/115054/diff/12/14
File tests/com/google/caja/plugin/domita_test.js (right):

http://codereview.appspot.com/115054/diff/12/14#newcode39
Line 39: xhr.open('GET', href, false);
Synchronous xhr!?

http://codereview.appspot.com/115054/diff/12/14#newcode95
Line 95: ___.setNewModuleHandler(___.makeNormalNewModuleHandler());
On 2009/09/11 17:59:04, MikeSamuel wrote:
> What's this?

It reinstalls the default new-module handler.

[ihab.awad, 2009-09-17 05:25:48]

http://codereview.appspot.com/115054/diff/12/15
File src/com/google/caja/cajita-module.js (right):

http://codereview.appspot.com/115054/diff/12/15#newcode45
Line 45: }
Added a TODO for myself to consolidate.

http://codereview.appspot.com/115054/diff/12/15#newcode45
Line 45: }
On 2009/09/11 17:59:04, MikeSamuel wrote:
> Can this be consolidated with the same code in domita.js?

Added a TODO. (Per your subsequent comment, this is a nontrivial schleppage.)

http://codereview.appspot.com/115054/diff/12/15#newcode60
Line 60: else {
On 2009/09/11 17:59:04, MikeSamuel wrote:
> Please fix braces

Fixed in a different CL by Ziqing.

http://codereview.appspot.com/115054/diff/12/16
File src/com/google/caja/serverjs-sandbox.js (right):

http://codereview.appspot.com/115054/diff/12/16#newcode111
Line 111: && typeof load !== 'undefined') {
On 2009/09/11 17:59:04, MikeSamuel wrote:
> Why not
> if (env !== undefined
>     && valijaModule !== undefined
>     && load !== undefined)?

Done.

http://codereview.appspot.com/115054/diff/12/16#newcode114
Line 114: }
On 2009/09/11 17:59:04, MikeSamuel wrote:
> I would rather functions not be declared inside conditions since that is not
> legal ES3.1.

Done.

http://codereview.appspot.com/115054/diff/12/17
File src/com/google/caja/valija-cajita.js (right):

http://codereview.appspot.com/115054/diff/12/17#newcode524
Line 524: // passing an 'outers' as a Cajita-level parameter, the result should
be the
On 2009/09/11 17:59:04, MikeSamuel wrote:
> result?

Done.

http://codereview.appspot.com/115054/diff/12/14
File tests/com/google/caja/plugin/domita_test.js (right):

http://codereview.appspot.com/115054/diff/12/14#newcode39
Line 39: xhr.open('GET', href, false);
On 2009/09/11 22:56:52, metaweta wrote:
> Synchronous xhr!?

Yep, just for a test ... was there before. :)

http://codereview.appspot.com/115054/diff/12/14#newcode95
Line 95: ___.setNewModuleHandler(___.makeNormalNewModuleHandler());
On 2009/09/11 22:56:52, metaweta wrote:
> On 2009/09/11 17:59:04, MikeSamuel wrote:
> > What's this?
> 
> It reinstalls the default new-module handler.

Yes, so that the eval() in inlineHTML will work properly. I moved these up to be
*inside* inlineHTML. I also added code to reset the module handler afterwards --
this is now the Right Thing [TM] to do since failing to save and reset the
existing handler will b0rk the newfangled script module loader. However, that
code causes an error. So I added a TODO for myself to figure out why. I'm sure
it's an interaction with the script module loader in some way ... but I really
want to get this CL in before domita_test.html changes *again* out from under
me. :)

http://codereview.appspot.com/115054/diff/12/14#newcode360
Line 360: initialize(serverJsSandboxModule, valijaModule)
On 2009/09/11 17:59:04, MikeSamuel wrote:
> missing ;

Done.

http://codereview.appspot.com/115054/diff/12/14#newcode361
Line 361: })
On 2009/09/11 17:59:04, MikeSamuel wrote:
> missing ;

Done.

[metaweta, 2009-09-17 22:05:23]

LGTM

[metaweta, 2011-08-22 16:40:36]

On 2009/09/17 22:05:23, metaweta wrote:
> LGTM

If this wasn't committed, it's surely obsolete now.