https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/html-emitter.js
File src/com/google/caja/plugin/html-emitter.js (right):
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/html-emitter.js#newcode457
src/com/google/caja/plugin/html-emitter.js:457: // TODO: Why is this
setting mime type to image/*?
Because, until this CL, that was the only envisioned use case for CSS
URIs?
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/html-emitter.js#newcode458
src/com/google/caja/plugin/html-emitter.js:458: function
makeCssUriSanitizer(baseUri) { return makeCssUriHandler(baseUri,
'cssUri', 'image/*'); }
Long lines.
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/html-emitter.js#newcode464
src/com/google/caja/plugin/html-emitter.js:464: function
continuation(sanitizeStyle, moreToCome) {
If you changed the API of "continuation" so it takes a single object
with values "result" and "moreToCome", you could make things so the "add
if no more things to do" logic is done only once.
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/html-emitter.js#newcode465
src/com/google/caja/plugin/html-emitter.js:465:
safeCss.push(sanitizeStyle);
See my comment in the next file: Given arrival order nondeterminism of
your XHRs, it is not clear to me how this simple push() preserves proper
ordering.
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/html-emitter.js#newcode473
src/com/google/caja/plugin/html-emitter.js:473: cssText,
domicile.suffixStr.replace(/^-/, ''),
Nit: Outdent 1 space.
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/html-emitter.js#newcode481
src/com/google/caja/plugin/html-emitter.js:481: safeInlineCss =
sanitized.result;
It seems safeInlineCss is just the last snippet of CSS, right? There's
no "inline" versus "non-inline" distinction iiuc. In any case, I found
the variable name non-mnemonic.
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/sanitizecss.js
File src/com/google/caja/plugin/sanitizecss.js (right):
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/sanitizecss.js#newcode121
src/com/google/caja/plugin/sanitizecss.js:121: sanitizeCssProperty =
(function () {
Could probably do away with this 2nd level of "module pattern" without
too much loss of generality -- just promote a couple of utility
functions and stuff?
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/sanitizecss.js#newcode523
src/com/google/caja/plugin/sanitizecss.js:523: var match =
/^url[(]["'](.*)["'][)]/.exec(url);
Surprised the regex doesn't end in "$" for max para-know-ya.
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/sanitizecss.js#newcode542
src/com/google/caja/plugin/sanitizecss.js:542: * @param
{function(string, boolean)} cont
inuation
Pun intended? :)
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/sanitizecss.js#newcode543
src/com/google/caja/plugin/sanitizecss.js:543: * Support external
stylesheets loaded via @import
Can you please be more specific about the function's API?
https://codereview.appspot.com/8778046/diff/3002/src/com/google/caja/plugin/sanitizecss.js#newcode586
src/com/google/caja/plugin/sanitizecss.js:586:
continuation(sanitized.result, sanitized.moreToCome);
Ok ... the claim being that this mechanism alone guarantees proper
ordering of the CSS declarations in both time and space (lexically
earlier => inserted into the page earlier in wall-clock time *and* in a
<style> node previous to what is lexically later). I guess that depends
on the implementation of "continuation".
https://codereview.appspot.com/8778046/diff/3002/tests/com/google/caja/plugin/es53-test-css-imports-guest.html
File tests/com/google/caja/plugin/es53-test-css-imports-guest.html
(right):
https://codereview.appspot.com/8778046/diff/3002/tests/com/google/caja/plugin/es53-test-css-imports-guest.html#newcode58
tests/com/google/caja/plugin/es53-test-css-imports-guest.html:58:
pass('testCssImports');
Nice tests! I had to "macro-expand" the rulesets into a single file in
order to understand them, but now that I do, it looks really good.
https://codereview.appspot.com/8778046/
--
---
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.
