initial comments

http://codereview.appspot.com/6060046/diff/5001/src/com/google/caja/plugin/caja.js
File src/com/google/caja/plugin/caja.js (right):

http://codereview.appspot.com/6060046/diff/5001/src/com/google/caja/plugin/caja.js#newcode43
src/com/google/caja/plugin/caja.js:43: callback(result);
wrap this in a try...finally so that the GC always happens, then rethrow
if necessary.

http://codereview.appspot.com/6060046/diff/5001/src/com/google/caja/plugin/caja.js#newcode256
src/com/google/caja/plugin/caja.js:256: full.cache = partial.cache ||
{};
Any worries about __proto__ here?  Does guest code control URLs coming
to this object?

http://codereview.appspot.com/6060046/diff/5001/src/com/google/caja/plugin/caja.js#newcode441
src/com/google/caja/plugin/caja.js:441: function installSyncScript(name,
url) {
Add a comment that urls need to be escaped before passing them in here,
since they're controlled by guest code.  The defaultFetcher is escaping
the url, so we're ok by default.

http://codereview.appspot.com/6060046/diff/5001/src/com/google/caja/plugin/guest-manager.js
File src/com/google/caja/plugin/guest-manager.js (right):

http://codereview.appspot.com/6060046/diff/5001/src/com/google/caja/plugin/guest-manager.js#newcode147
src/com/google/caja/plugin/guest-manager.js:147: setTimeout(opt_runDone,
1000);
Why a full second?

http://codereview.appspot.com/6060046/diff/5001/src/com/google/caja/plugin/html-emitter.js
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/6060046/diff/5001/src/com/google/caja/plugin/html-emitter.js#newcode418
src/com/google/caja/plugin/html-emitter.js:418: return srcIndex ?
attribs[srcIndex] : undefined;
Off by 1?

http://codereview.appspot.com/6060046/

Reply via email to