Thanks for the comments so far. The CL should be far more clean and
complete now.
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
On 2013/07/30 01:05:59, MarkM wrote:
"xxxx"?
Done.
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;
On 2013/07/30 01:05:59, MarkM wrote:
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?
Done.
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];
On 2013/07/30 01:05:59, MarkM wrote:
Better. But factoring this into an abstraction would be better yet.
Done.
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) {
On 2013/07/30 01:05:59, MarkM wrote:
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.
Yes, see https://code.google.com/p/v8/issues/detail?id=2779#c12
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) &&
On 2013/07/30 01:05:59, MarkM wrote:
80 columns
Done.
https://codereview.appspot.com/12029043/diff/9001/src/com/google/caja/ses/mitigateGotchas.js#newcode388
src/com/google/caja/ses/mitigateGotchas.js:388: debugger;
On 2013/07/30 01:05:59, MarkM wrote:
Remove "debugger;"
Done.
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 {
On 2013/07/30 01:05:59, MarkM wrote:
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.
Done.
https://codereview.appspot.com/12029043/diff/15002/src/com/google/caja/ses/mitigateGotchas.js
File src/com/google/caja/ses/mitigateGotchas.js (right):
https://codereview.appspot.com/12029043/diff/15002/src/com/google/caja/ses/mitigateGotchas.js#newcode136
src/com/google/caja/ses/mitigateGotchas.js:136: var parent =
parent(scope);
On 2013/07/30 18:19:32, MarkM wrote:
Does this run? The scope collision on "parent" should prevent it from
running.
Done.
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.
