https://codereview.appspot.com/10205043/diff/3001/src/com/google/caja/ses/startSES.js
File src/com/google/caja/ses/startSES.js (right):
https://codereview.appspot.com/10205043/diff/3001/src/com/google/caja/ses/startSES.js#newcode181
src/com/google/caja/ses/startSES.js:181: * limited character set
that SES should process; otherwise, do nothing.
How about making this a method hanging off of atLeastFreeVarNames,
instead, since the charset is more about atLeastFreeVarNames's
self-confidence than anything necessary for SES in general?
https://codereview.appspot.com/10205043/diff/3001/src/com/google/caja/ses/startSES.js#newcode934
src/com/google/caja/ses/startSES.js:934: } catch (e) {
Please don't use exceptions for control flow; it will be tedious in
debugging. Add the information to the return value.
https://codereview.appspot.com/10205043/diff/3001/tests/com/google/caja/plugin/es53-test-cajajs-invocation.js
File tests/com/google/caja/plugin/es53-test-cajajs-invocation.js
(right):
https://codereview.appspot.com/10205043/diff/3001/tests/com/google/caja/plugin/es53-test-cajajs-invocation.js#newcode242
tests/com/google/caja/plugin/es53-test-cajajs-invocation.js:242: if
(inES5Mode)
Don't do this any more; use jsunitRegisterIf instead.
https://codereview.appspot.com/10205043/diff/3001/tests/com/google/caja/plugin/es53-test-cajajs-invocation.js#newcode243
tests/com/google/caja/plugin/es53-test-cajajs-invocation.js:243:
jsunitRegister('testUnicodeInCodeFails', function
testUnicodeInCodeFails() {
I think this test case is more appropriately grouped in
es53-test-language-guest.html.
It doesn't need to be a separate load since script blocks parse and fail
independently, and also since it is ES5-mode only you can just invoke
eval instead (it's a reasonable assumption, I would say, that <script>
evals go through the visible SES eval operations).
https://codereview.appspot.com/10205043/diff/3001/tests/com/google/caja/plugin/es53-test-cajajs-invocation.js#newcode246
tests/com/google/caja/plugin/es53-test-cajajs-invocation.js:246:
'<script> var te\ufeffst = 1; x++; </script>' + // should not run
I would put the x++ first, just on the off chance the other thing is
misinterpreted as something valid but which throws at runtime.
https://codereview.appspot.com/10205043/diff/3001/tests/com/google/caja/plugin/es53-test-cajajs-invocation.js#newcode250
tests/com/google/caja/plugin/es53-test-cajajs-invocation.js:250:
caja.load(div, xhrUriPolicy, function (frame) {
no space before (
https://codereview.appspot.com/10205043/diff/3001/tests/com/google/caja/plugin/es53-test-cajajs-invocation.js#newcode257
tests/com/google/caja/plugin/es53-test-cajajs-invocation.js:257:
.run(function(result) {
This should be a jsunitCallback.
https://codereview.appspot.com/10205043/diff/3001/tests/com/google/caja/plugin/es53-test-cajajs-invocation.js#newcode264
tests/com/google/caja/plugin/es53-test-cajajs-invocation.js:264: if
(inES5Mode)
ditto on all above comments
https://codereview.appspot.com/10205043/
--
---
You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
