Needs more work.
How does this enforce that this mitigation is mandatory on those
platforms suffering from this bug? I see no logic which enforces this.
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/mitigateGotchas.js
File src/com/google/caja/ses/mitigateGotchas.js (right):
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/mitigateGotchas.js#newcode31
src/com/google/caja/ses/mitigateGotchas.js:31: * xxxx requires console,
JSON
"xxxx"?
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/mitigateGotchas.js#newcode114
src/com/google/caja/ses/mitigateGotchas.js:114: var body =
scope.path[scope.path.length - 2].body;
Can we factor out this idiom for obtaining a parent from a scope? As is,
the new code is vastly less clear than the old code. How is a reader
supposed to figure out that this is navigating to the parent?
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/mitigateGotchas.js#newcode141
src/com/google/caja/ses/mitigateGotchas.js:141: var parent =
scope.path[scope.path.length - 2];
Better. But factoring this into an abstraction would be better yet.
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/mitigateGotchas.js#newcode271
src/com/google/caja/ses/mitigateGotchas.js:271: function
rewriteStaticKeyMemberExpression(scope, node) {
Did we ever get a confirmation from the v8 folks that this rewrite is
adequate for the actual problem? I am unwilling to sign off on this
based just on black box testing.
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/mitigateGotchas.js#newcode324
src/com/google/caja/ses/mitigateGotchas.js:324: } else if
((scope.options.rewritePropertyCompoundAssignmentExpr || true) &&
80 columns
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/mitigateGotchas.js#newcode388
src/com/google/caja/ses/mitigateGotchas.js:388: debugger;
Remove "debugger;"
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/patterns.js
File src/com/google/caja/ses/patterns.js (right):
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/patterns.js#newcode68
src/com/google/caja/ses/patterns.js:68: ses.patterns {
Is this code actually used? If not, please remove this file from the CL.
If it is, I don't understand how this line even parses. It seems like a
syntax error to me.
https://codereview.appspot.com/12029043/
--
---
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.
