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.
On 2013/06/11 23:48:49, kpreid_google wrote:
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?
I'd honestly rather not mess with things that way, hanging functions off
functions or making the data structure of atLeastFreeVarNames more
complicated. Yes this exposes some gory details, but so be it.
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) {
On 2013/06/11 23:48:49, kpreid_google wrote:
Please don't use exceptions for control flow; it will be tedious in
debugging.
Add the information to the return value.
Done.
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)
On 2013/06/11 23:48:49, kpreid_google wrote:
Don't do this any more; use jsunitRegisterIf instead.
Done.
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() {
Hm. Good point but this way, it's in a JS file rather than HTML, so it
seems that the encoding issues might be less scary. I'm tending to leave
as-is....
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
On 2013/06/11 23:48:49, kpreid_google wrote:
I would put the x++ first, just on the off chance the other thing is
misinterpreted as something valid but which throws at runtime.
Done.
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) {
On 2013/06/11 23:48:49, kpreid_google wrote:
no space before (
Done.
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) {
On 2013/06/11 23:48:49, kpreid_google wrote:
This should be a jsunitCallback.
Done.
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)
On 2013/06/11 23:48:49, kpreid_google wrote:
ditto on all above comments
Done.
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.
