Revision: 5583
Author: [email protected]
Date: Wed Aug 28 18:04:38 2013 UTC
Log: fix Closure Compiler issues in html-css-sanitizer
https://codereview.appspot.com/13258043
This fixes the css sanitizer to be Closure friendly and replaces the
minified version with a closured version. This reduces the size from
76k to 52k uncompressed, 24k to 16k compressed.
The fix is all about how Closure treats a.foo differently from
a['foo']. These particular foos are external linkage names, so
I changed them to a['foo'] so Closure won't rename them.
The change in RhinoTestBed is because the Closured js will emit
code like
if (condition) /foo/.test(bar)
and our java JS parser barfs on the regexp in that position.
The barf is a known problem, it's complicated to fix, but it
turns out we don't really need to parse js in RhinoTestBed,
so that's what the change is.
R=ihab.awad
http://code.google.com/p/google-caja/source/detail?r=5583
Modified:
/trunk/build.xml
/trunk/src/com/google/caja/plugin/cssparser.js
/trunk/src/com/google/caja/plugin/html-sanitizer.js
/trunk/src/com/google/caja/plugin/sanitizecss.js
/trunk/tests/com/google/caja/parser/js/ParserTest.java
/trunk/tests/com/google/caja/util/RhinoTestBed.java
=======================================
--- /trunk/build.xml Fri Aug 23 17:58:39 2013 UTC
+++ /trunk/build.xml Wed Aug 28 18:04:38 2013 UTC
@@ -823,11 +823,8 @@
<transform>
<output file="${lib}/html-css-sanitizer.jslint.js"
language="jslint"/>
- <!-- TODO(felix8a): -closured doesn't work -->
- <output file="${lib.caja}/plugin/html-css-sanitizer-closured.js"
- language="closure"/>
<output file="${lib.caja}/plugin/html-css-sanitizer-minified.js"
- language="javascript" renderer="minify"/>
+ language="closure"/>
<output file="${lib.caja}/plugin/html-css-sanitizer-bundle.js"
language="javascript" renderer="concat"/>
<input file="${lib.caja}/plugin/css-defs.js"/>
@@ -1110,21 +1107,7 @@
<!-- For RhinoTestBed tests; arrange files so that they can use the
same
relative URLs in browser. TODO(kpreid): Clean up the whole
mess. -->
<fileset dir="${lib}">
- <include name="**/caja/plugin/csslexer.js"/>
- <include name="**/caja/plugin/cssparser.js"/>
- <include name="**/caja/plugin/css-defs.js"/>
- <include name="**/caja/plugin/html4-defs.js"/>
- <include name="**/caja/plugin/uri.js"/>
- <include name="**/caja/plugin/sanitizecss.js"/>
- <include name="**/caja/plugin/html-sanitizer.js"/>
- <include name="**/caja/plugin/bridal.js"/>
- <include name="**/caja/plugin/html-schema.js"/>
- <include name="**/caja/plugin/html-emitter.js"/>
- <include name="**/caja/plugin/html-sanitizer-minified.js"/>
- <include name="**/caja/plugin/html-sanitizer-legacy.js"/>
- <include name="**/caja/plugin/html-css-sanitizer-bundle.js"/>
- <include name="**/caja/plugin/html-css-sanitizer-minified.js"/>
- <include name="**/caja/plugin/html-sanitizer-bundle.js"/>
+ <include name="**/caja/plugin/*.js"/>
</fileset>
</copy>
<copy todir="${testlib}">
@@ -1352,7 +1335,6 @@
<include name="html-sanitizer-bundle.js" />
<include name="html-sanitizer-minified.js" />
<include name="html-css-sanitizer-bundle.js" />
- <include name="html-css-sanitizer-closured.js" />
<include name="html-css-sanitizer-minified.js" />
<include name="taming-membrane.out.js" />
</fileset>
=======================================
--- /trunk/src/com/google/caja/plugin/cssparser.js Wed Aug 7 00:29:48 2013
UTC
+++ /trunk/src/com/google/caja/plugin/cssparser.js Wed Aug 28 18:04:38 2013
UTC
@@ -84,12 +84,12 @@
// stylesheet : [ CDO | CDC | S | statement ]*;
parseCssStylesheet = function(cssText, handler) {
var toks = lexCss(cssText);
- if (handler.startStylesheet) { handler.startStylesheet(); }
+ if (handler['startStylesheet']) { handler['startStylesheet'](); }
for (var i = 0, n = toks.length; i < n;) {
// CDO and CDC ("<!--" and "-->") are converted to space by the
lexer.
i = toks[i] === ' ' ? i+1 : statement(toks, i, n, handler);
}
- if (handler.endStylesheet) { handler.endStylesheet(); }
+ if (handler['endStylesheet']) { handler['endStylesheet'](); }
};
// statement : ruleset | at-rule;
@@ -116,14 +116,14 @@
var s = start+1, e = i;
if (s < n && toks[s] === ' ') { ++s; }
if (e > s && toks[e-1] === ' ') { --e; }
- if (handler.startAtrule) {
- handler.startAtrule(toks[start].toLowerCase(), toks.slice(s, e));
+ if (handler['startAtrule']) {
+ handler['startAtrule'](toks[start].toLowerCase(), toks.slice(s,
e));
}
i = (toks[i] === '{')
? block(toks, i, n, handler)
: i+1; // Skip over ';'
- if (handler.endAtrule) {
- handler.endAtrule();
+ if (handler['endAtrule']) {
+ handler['endAtrule']();
}
}
// Else we reached end of input or are missing a semicolon.
@@ -135,7 +135,7 @@
// Assumes the leading '{' has been verified by callers.
function block(toks, i, n, handler) {
++i; // skip over '{'
- if (handler.startBlock) { handler.startBlock(); }
+ if (handler['startBlock']) { handler['startBlock'](); }
while (i < n) {
var ch = toks[i].charAt(0);
if (ch == '}') {
@@ -159,7 +159,7 @@
i = ruleset(toks, i, n, handler);
}
}
- if (handler.endBlock) { handler.endBlock(); }
+ if (handler['endBlock']) { handler['endBlock'](); }
return i;
}
@@ -181,8 +181,8 @@
i = e+1; // Skip over '{'
// Don't include any trailing space in the selector slice.
if (e > s && toks[e-1] === ' ') { --e; }
- if (handler.startRuleset) {
- handler.startRuleset(toks.slice(s, e));
+ if (handler['startRuleset']) {
+ handler['startRuleset'](toks.slice(s, e));
}
while (i < n) {
tok = toks[i];
@@ -196,8 +196,8 @@
i = declaration(toks, i, n, handler);
}
}
- if (handler.endRuleset) {
- handler.endRuleset();
+ if (handler['endRuleset']) {
+ handler['endRuleset']();
}
return i;
}
@@ -298,9 +298,9 @@
++e;
}
}
- if (valuelen && handler.declaration) {
+ if (valuelen && handler['declaration']) {
// TODO: coerce non-keyword ident tokens to quoted strings.
- handler.declaration(property.toLowerCase(), value);
+ handler['declaration'](property.toLowerCase(), value);
}
}
return e;
=======================================
--- /trunk/src/com/google/caja/plugin/html-sanitizer.js Fri Jul 26 21:29:58
2013 UTC
+++ /trunk/src/com/google/caja/plugin/html-sanitizer.js Wed Aug 28 18:04:38
2013 UTC
@@ -900,7 +900,7 @@
parseCssDeclarations(
value,
{
- declaration: function (property, tokens) {
+ 'declaration': function (property, tokens) {
var normProp = property.toLowerCase();
sanitizeCssProperty(
normProp, tokens,
=======================================
--- /trunk/src/com/google/caja/plugin/sanitizecss.js Wed Aug 21 04:26:59
2013 UTC
+++ /trunk/src/com/google/caja/plugin/sanitizecss.js Wed Aug 28 18:04:38
2013 UTC
@@ -162,7 +162,7 @@
return;
}
- var propBits = propertySchema.cssPropBits;
+ var propBits = propertySchema['cssPropBits'];
/**
* Recurse to apply the appropriate function schema to the function
call
@@ -192,7 +192,7 @@
var bareFnToken = withoutVendorPrefix(fnToken);
// Cut out the originals, so the caller can step by one token.
var fnTokens = tokens.splice(start, end - start, '');
- var fns = propertySchema.cssFns;
+ var fns = propertySchema['cssFns'];
// Look for a function that matches the name.
for (var i = 0, nFns = fns.length; i < nFns; ++i) {
if (fns[i].substring(0, bareFnToken.length) == bareFnToken) {
@@ -259,11 +259,12 @@
)
: (
- litGroup = propertySchema.cssLitGroup,
+ litGroup = propertySchema['cssLitGroup'],
litMap = (litGroup
- ? (propertySchema.cssLitMap
+ ? (propertySchema['cssLitMap']
// Lazily compute the union from litGroup.
- || (propertySchema.cssLitMap =
unionArrays(litGroup)))
+ || (propertySchema['cssLitMap'] =
+ unionArrays(litGroup)))
: ALLOWED_LITERAL), // A convenient empty object.
(litMap[withoutVendorPrefix(token)] === ALLOWED_LITERAL)
)
@@ -871,12 +872,12 @@
parseCssStylesheet(
cssText,
{
- startStylesheet: function () {
+ 'startStylesheet': function () {
safeCss = [];
},
- endStylesheet: function () {
+ 'endStylesheet': function () {
},
- startAtrule: function (atIdent, headerArray) {
+ 'startAtrule': function (atIdent, headerArray) {
if (elide) {
atIdent = null;
} else if (atIdent === '@media') {
@@ -936,14 +937,14 @@
elide = !atIdent;
blockStack.push(atIdent);
},
- endAtrule: function () {
+ 'endAtrule': function () {
blockStack.pop();
if (!elide) {
safeCss.push(';');
}
checkElide();
},
- startBlock: function () {
+ 'startBlock': function () {
// There are no bare blocks in CSS, so we do not change the
// block stack here, but instead in the events that bracket
// blocks.
@@ -951,13 +952,13 @@
safeCss.push('{');
}
},
- endBlock: function () {
+ 'endBlock': function () {
if (!elide) {
safeCss.push('}');
elide = true; // skip any semicolon from endAtRule.
}
},
- startRuleset: function (selectorArray) {
+ 'startRuleset': function (selectorArray) {
var historySensitiveSelectors = void 0;
var removeHistoryInsensitiveSelectors = false;
if (!elide) {
@@ -1012,7 +1013,7 @@
removeHistoryInsensitiveSelectors
});
},
- endRuleset: function () {
+ 'endRuleset': function () {
var rules = blockStack.pop();
var propertiesEnd = safeCss.length;
if (!elide) {
@@ -1034,7 +1035,7 @@
}
checkElide();
},
- declaration: function (property, valueArray) {
+ 'declaration': function (property, valueArray) {
if (!elide) {
var isImportant = false;
var nValues = valueArray.length;
=======================================
--- /trunk/tests/com/google/caja/parser/js/ParserTest.java Thu Aug 8
19:36:28 2013 UTC
+++ /trunk/tests/com/google/caja/parser/js/ParserTest.java Wed Aug 28
18:04:38 2013 UTC
@@ -519,6 +519,7 @@
// requires a nontrivial rewrite of the js lexer stack.
@FailureIsAnOption
public final void testRegexpContext() throws Exception {
+ assertParseSucceeds("if(true)/bar/;");
assertParseSucceeds("{} /foo/;");
}
=======================================
--- /trunk/tests/com/google/caja/util/RhinoTestBed.java Thu Aug 8 00:39:21
2013 UTC
+++ /trunk/tests/com/google/caja/util/RhinoTestBed.java Wed Aug 28 18:04:38
2013 UTC
@@ -147,14 +147,14 @@
scriptBody = textContentOf(script);
}
String scriptText;
- Block js = parseJavascript(scriptBody, mq);
+ InputSource isrc = scriptBody.getSourceBreaks(0).source();
// Add blank lines at the front so that Rhino stack traces have
correct
// line numbers.
scriptText = prefixWithBlankLines(
scriptBody.toString(0, scriptBody.getLimit()),
Nodes.getFilePositionFor(script).startLineNo() - 1);
- scriptContent.add(Pair.pair(scriptText,
js.getFilePosition().source()));
- mc.addInputSource(js.getFilePosition().source());
+ scriptContent.add(Pair.pair(scriptText, isrc));
+ mc.addInputSource(isrc);
script.getParentNode().removeChild(script);
}
for (Pair<String, InputSource> script : scriptContent) {
--
---
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.
