http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/caja.js File src/com/google/caja/plugin/caja.js (right):
http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/caja.js#newcode46 src/com/google/caja/plugin/caja.js:46: window[rndName] = function (result) { Not easily - the document.write blows away the loader frame. I've marked this as a TODO. On 2012/05/08 21:43:26, ihab.awad wrote:
Is it not possible to install the named callback function *only* in
the 'window'
of the loader document, rather than installing it in the top-level
window as
well? The way we have it right here, it'll pollute the host frame....
http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/caja.js#newcode243 src/com/google/caja/plugin/caja.js:243: server = full['server'] = String( On 2012/05/08 21:43:26, ihab.awad wrote:
This should be done in 'initialize' or something. resolveConfig didn't
side
effect anything....
Done. http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/caja.js#newcode463 src/com/google/caja/plugin/caja.js:463: .replace(/[$]name/g, name)) Marked as TODO. Will fix as part of the loaderFrame CL. On 2012/05/08 21:43:26, ihab.awad wrote:
This assignment pins the parent's handler function and, iiuc, this
reference is
never cleared out.
http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/domado.js File src/com/google/caja/plugin/domado.js (right): http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/domado.js#newcode86 src/com/google/caja/plugin/domado.js:86: return; On 2012/05/07 23:05:18, metaweta wrote:
Since you're returning the result of this function below in
domicile.fetchUri,
please provide an explicit return value.
Done. http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/domado.js#newcode91 src/com/google/caja/plugin/domado.js:91: naiveUriPolicy.fetch(undefined, mime, callback); Calls the callback with undefined which logs an error. On 2012/05/08 21:43:26, ihab.awad wrote:
What does fetching 'undefined' do exactly? What is this case doing?
http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/html-emitter.js File src/com/google/caja/plugin/html-emitter.js (right): http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/html-emitter.js#newcode391 src/com/google/caja/plugin/html-emitter.js:391: function defineUntrustedExternalStylesheet(url, marker, continuation) { On 2012/05/08 21:43:26, ihab.awad wrote:
This function and the next have a lot of commonality that can be
abstracted
away.
Done. http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/html-emitter.js#newcode431 src/com/google/caja/plugin/html-emitter.js:431: var pendingExternal = undefined; This is a false assumption and causes a failure - I've marked this with a TODO and will fix in a later CL. On 2012/05/08 21:43:26, ihab.awad wrote:
I'm interested in the assumption that seems to be made here that
documentWriter
methods are never going to be called reentrantly.
http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/ses-frame-group.js File src/com/google/caja/plugin/ses-frame-group.js (right): http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/plugin/ses-frame-group.js#newcode378 src/com/google/caja/plugin/ses-frame-group.js:378: }); On 2012/05/08 21:43:26, ihab.awad wrote:
indentation
Done. http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/service/AbstractCajolingHandler.java File src/com/google/caja/service/AbstractCajolingHandler.java (right): http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/service/AbstractCajolingHandler.java#newcode150 src/com/google/caja/service/AbstractCajolingHandler.java:150: On 2012/05/08 21:43:26, ihab.awad wrote:
stray whitespace
Done. http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/service/AbstractCajolingHandler.java#newcode165 src/com/google/caja/service/AbstractCajolingHandler.java:165: protected static void renderAsJSON( On 2012/05/08 21:43:26, ihab.awad wrote:
un-indent 2 sp
Done. http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/service/ProxyHandler.java File src/com/google/caja/service/ProxyHandler.java (right): http://codereview.appspot.com/6060046/diff/15001/src/com/google/caja/service/ProxyHandler.java#newcode1 src/com/google/caja/service/ProxyHandler.java:1: // Copyright 2009 Google Inc. All Rights Reserved. On 2012/05/08 21:43:26, ihab.awad wrote:
Copyright year
Done. http://codereview.appspot.com/6060046/
