lgtm++

https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/plugin/domado.js
File src/com/google/caja/plugin/domado.js (right):

https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/plugin/domado.js#newcode571
src/com/google/caja/plugin/domado.js:571: */
I guess one thing that concerns me is that we do not seem to be
force-testing (or explicitly failing early on) the Cartesian product of
all the diverse and sundry combinations of browser features (WeakMaps,
Proxies, ...). Clearly, this is a long-term concern but this CL
highlights its importance.

https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/plugin/domado.js#newcode591
src/com/google/caja/plugin/domado.js:591: w.get(proxy);
The trick that is happening here -- using a secretive channel to get at
the otherwise secret WeakMap property name -- is very subtle and might
benefit from some explanation.

https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/ses/WeakMap.js
File src/com/google/caja/ses/WeakMap.js (right):

https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/ses/WeakMap.js#newcode462
src/com/google/caja/ses/WeakMap.js:462: if (typeof HostWeakMap ===
'function') (function() {
Please use curlies after condition, as a stylistic thing to avoid
dangling statement problems....

https://codereview.appspot.com/8612048/diff/1/src/com/google/caja/ses/WeakMap.js#newcode500
src/com/google/caja/ses/WeakMap.js:500: WeakMap = DoubleWeakMap;
Is this clever wonderfulness actually exercised by any tests in our
codebase at the moment?

https://codereview.appspot.com/8612048/

--

--- 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.


Reply via email to