Permalink
Browse files

Last fix for #2528102 was not robust enough (didn't fix the test cases

added. e.g. .foo, #AABBCC {...}).

This commit attempts to fix the ID selector breakage by only compressing
hex colors inside {...}.

Added test cases which were still breaking in the original fix, and some
more variants for hex color compression and IDs together in the same file.

Also added a tools/cssmin-debugger.html test page, which can be used to
step through the cssmin JS port in a (HTML 5 File API compatible) browser.
  • Loading branch information...
1 parent 71b9d2c commit 87b4ee83155c1c6201b70aaf2671807a82d32f33 @sdesai sdesai committed Sep 23, 2011
View
3 doc/CHANGELOG
@@ -1,8 +1,9 @@
YUI Compressor 2.4.7
--------------------
+ Handle data urls without blowing up Java memory (regex)
-+ Ported data urls handling to JS port for consistency
+ Updated docs to reflect Java >= 1.5 required for CssCompressor
++ Fixed issue where we were breaking #AABBCC id selectors, with the
+ #AABBCC -> #ABC color compression.
YUI Compressor 2.4.6, 2011-04-15
-------------------------------
View
85 ports/js/cssmin.js
@@ -19,6 +19,17 @@
var YAHOO = YAHOO || {};
YAHOO.compressor = YAHOO.compressor || {};
+/**
+ * Utility method to replace all data urls with tokens before we start
+ * compressing, to avoid performance issues running some of the subsequent
+ * regexes against large strings chunks.
+ *
+ * @private
+ * @method _extractDataUrls
+ * @param {String} css The input css
+ * @param {Array} The global array of tokens to preserve
+ * @returns String The processed css
+ */
YAHOO.compressor._extractDataUrls = function (css, preservedTokens) {
// Leave data urls alone to increase parse performance.
@@ -66,15 +77,15 @@ YAHOO.compressor._extractDataUrls = function (css, preservedTokens) {
// Enough searching, start moving stuff over to the buffer
sb.push(css.substring(appendIndex, m.index));
-
+
if (foundTerminator) {
token = css.substring(startIndex, endIndex);
token = token.replace(/\s+/g, "");
preservedTokens.push(token);
preserver = "url(___YUICSSMIN_PRESERVED_TOKEN_" + (preservedTokens.length - 1) + "___)";
sb.push(preserver);
-
+
appendIndex = endIndex + 1;
} else {
// No end terminator found, re-add the whole match. Should we throw/warn here?
@@ -88,6 +99,55 @@ YAHOO.compressor._extractDataUrls = function (css, preservedTokens) {
return sb.join("");
};
+/**
+ * Utility method to compress hex color values of the form #AABBCC to #ABC.
+ *
+ * DOES NOT compress CSS ID selectors which match the above pattern (which would break things).
+ * e.g. #AddressForm { ... }
+ *
+ * DOES NOT compress IE filters, which have hex color values (which would break things).
+ * e.g. filter: chroma(color="#FFFFFF");
+ *
+ * DOES NOT compress invalid hex values.
+ * e.g. background-color: #aabbccdd
+ *
+ * @private
+ * @method _compressHexColors
+ * @param {String} css The input css
+ * @returns String The processed css
+ */
+YAHOO.compressor._compressHexColors = function(css) {
+
+ // Look for hex colors inside { ... } (to avoid IDs) and which don't have a =, or a " in front of them (to avoid filters)
+ var pattern = /([^"'=\s])(\s*)#([0-9a-f])([0-9a-f])([0-9a-f])([0-9a-f])([0-9a-f])([0-9a-f])(\}|[^0-9a-fA-F{][^{]*?\})/gi,
+ m,
+ index = 0,
+ sb = [];
+
+ while ((m = pattern.exec(css)) !== null) {
+
+ if (m[3].toLowerCase() == m[4].toLowerCase() &&
+ m[5].toLowerCase() == m[6].toLowerCase() &&
+ m[7].toLowerCase() == m[8].toLowerCase()) {
+
+ // Enough searching, start moving stuff over to the buffer
+ sb.push(css.substring(index, m.index));
+ sb.push((m[1] + m[2] + "#" + m[3] + m[5] + m[7]).toLowerCase());
+
+ index = pattern.lastIndex = pattern.lastIndex - m[9].length;
+ } else {
+ sb.push(css.substring(index, m.index));
+ sb.push((m[1] + m[2] + "#" + m[3] + m[4] + m[5] + m[6] + m[7] + m[8]).toLowerCase());
+
+ index = pattern.lastIndex = pattern.lastIndex - m[9].length;
+ }
+ }
+
+ sb.push(css.substring(index));
+
+ return sb.join("");
+};
+
YAHOO.compressor.cssmin = function (css, linebreakpos) {
var startIndex = 0,
@@ -240,25 +300,8 @@ YAHOO.compressor.cssmin = function (css, linebreakpos) {
return '#' + rgbcolors.join('');
});
-
- // Shorten colors from #AABBCC to #ABC. Note that we want to make sure
- // the color is not preceded by either ", " or =. Indeed, the property
- // filter: chroma(color="#FFFFFF");
- // would become
- // filter: chroma(color="#FFF");
- // which makes the filter break in IE.
- css = css.replace(/([^"'=\s])(\s*)#([0-9a-f])([0-9a-f])([0-9a-f])([0-9a-f])([0-9a-f])([0-9a-f])/gi, function () {
- var group = arguments;
- if (
- group[3].toLowerCase() === group[4].toLowerCase() &&
- group[5].toLowerCase() === group[6].toLowerCase() &&
- group[7].toLowerCase() === group[8].toLowerCase()
- ) {
- return (group[1] + group[2] + '#' + group[3] + group[5] + group[7]).toLowerCase();
- } else {
- return group[0].toLowerCase();
- }
- });
+ // Shorten colors from #AABBCC to #ABC.
+ css = this._compressHexColors(css);
// border: none -> border:0
css = css.replace(/(border|border-top|border-right|border-bottom|border-right|outline|background):none(;|\})/gi, function(all, prop, tail) {
View
23 src/com/yahoo/platform/yui/compressor/CssCompressor.java
@@ -290,25 +290,28 @@ public void compress(Writer out, int linebreakpos)
// would become
// filter: chroma(color="#FFF");
// which makes the filter break in IE.
- p = Pattern.compile("([^\"'=\\s])(\\s*)#([0-9a-fA-F])([0-9a-fA-F])([0-9a-fA-F])([0-9a-fA-F])([0-9a-fA-F])([0-9a-fA-F])");
+ // We also want to make sure we're only compressing #AABBCC patterns inside { }, not id selectors ( #FAABAC {} )
+ // We also want to avoid compressing invalid values (e.g. #AABBCCD to #ABCD)
+ p = Pattern.compile("([^\"'=\\s])" + "(\\s*)" + "#([0-9a-fA-F])([0-9a-fA-F])([0-9a-fA-F])([0-9a-fA-F])([0-9a-fA-F])([0-9a-fA-F])" + "(:?\\}|[^0-9a-fA-F{][^{]*?\\})");
m = p.matcher(css);
sb = new StringBuffer();
- while (m.find()) {
- if (m.group(1).equals("}")) {
- // Likely an ID selector. Don't touch.
- // #AABBCC is a valid ID. IDs are case-sensitive.
- m.appendReplacement(sb, m.group());
- } else if (m.group(3).equalsIgnoreCase(m.group(4)) &&
+ int index = 0;
+ while (m.find(index)) {
+ if (m.group(3).equalsIgnoreCase(m.group(4)) &&
m.group(5).equalsIgnoreCase(m.group(6)) &&
m.group(7).equalsIgnoreCase(m.group(8))) {
// #AABBCC pattern
- m.appendReplacement(sb, (m.group(1) + m.group(2) + "#" + m.group(3) + m.group(5) + m.group(7)).toLowerCase());
+ sb.append(css.substring(index, m.start(1)));
+ sb.append((m.group(1) + m.group(2) + "#" + m.group(3) + m.group(5) + m.group(7)).toLowerCase());
+ index = m.end(8);
} else {
// Any other color.
- m.appendReplacement(sb, m.group().toLowerCase());
+ sb.append(css.substring(index, m.start(1)));
+ sb.append((m.group(1) + m.group(2) + "#" + m.group(3) + m.group(4) + m.group(5) + m.group(6) + m.group(7) + m.group(8)).toLowerCase());
+ index = m.end(8);
}
}
- m.appendTail(sb);
+ sb.append(css.substring(index));
css = sb.toString();
// border: none -> border:0
View
37 tests/color.css
@@ -1,7 +1,42 @@
.color {
me: rgb(123, 123, 123);
impressed: #ffeedd;
+ again: #456789;
+ andagain:#aa66cc;
+ background-color:#aa66ccc;
filter: chroma(color="#FFFFFF");
background: none repeat scroll 0 0 rgb(255, 0,0);
alpha: rgba(1, 2, 3, 4);
-}
+ color:#1122aa
+}
+
+#AABBCC {
+ background-color:#ffee11;
+ filter: chroma(color="#FFFFFF");
+ color:#441122;
+}
+
+.foo #AABBCC {
+ background-color:#ffee11;
+ color:#441122;
+ filter: chroma(color="#FFFFFF")
+}
+
+.bar, #AABBCC {
+ background-color:#ffee11;
+ color:#441122;
+}
+
+.foo, #AABBCC.foobar {
+ background-color:#ffee11;
+ color:#441122;
+}
+
+@media screen {
+ .bar, #AABBCC {
+ background-color:#ffee11;
+ color:#441122
+ }
+}
+
+
View
4 tests/color.css.min
@@ -1 +1,3 @@
-.color{me:#7b7b7b;impressed:#fed;filter:chroma(color="#FFFFFF");background:none repeat scroll 0 0 #f00;alpha:rgba(1,2,3,4)}
+.color{me:#7b7b7b;impressed:#fed;again:#456789;andagain:#a6c;background-color:#aa66ccc;filter:chroma(color="#FFFFFF");background:none repeat scroll 0 0 #f00;alpha:rgba(1,2,3,4);color:#12a}#AABBCC{background-color:#fe1;filter:chroma(color="#FFFFFF");color:#412}.foo #AABBCC{background-color:#fe1;color:#412;filter:chroma(color="#FFFFFF")}.bar,#AABBCC{background-color:#fe1;color:#412}.foo,#AABBCC.foobar{background-color:#fe1;color:#412}@media screen{.bar,#AABBCC{background-color:#fe1;color:#412}}
+
+
View
3 tests/preserve-case.css
@@ -10,3 +10,6 @@
#FeedbackMailForm .classe{
margin: 0;
}
+.classes, #FeedBackMailForm {
+ margin: 0;
+}
View
2 tests/preserve-case.css.min
@@ -1 +1 @@
-#AddAddressForm{padding:0}#AddAddressForm .messageBoxNeutral{padding:0}#FeedbackMailForm{padding:0}#FeedbackMailForm .classe{margin:0}
+#AddAddressForm{padding:0}#AddAddressForm .messageBoxNeutral{padding:0}#FeedbackMailForm{padding:0}#FeedbackMailForm .classe{margin:0}.classes,#FeedBackMailForm{margin:0}
View
72 tools/cssmin-debugger.html
@@ -0,0 +1,72 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+ <head>
+ <link href="http://yui.yahooapis.com/3.4.0/build/cssreset/cssreset.css" type="text/css" rel="stylesheet">
+ <link href="http://yui.yahooapis.com/3.4.0/build/cssbase/cssbase.css" type="text/css" rel="stylesheet">
+
+ <style>
+ body {
+ padding:10px;
+ }
+
+ pre {
+ width:90%;
+ padding:10px;
+ overflow:auto;
+ background-color:#eee;
+ }
+
+ #testFile {
+ margin:2em;
+ }
+
+ #notsupportedmsg.hidden {
+ display:none;
+ }
+
+ #notsupportedmsg {
+ color:red;
+ }
+ </style>
+
+ <script src="../ports/js/cssmin.js"></script>
+ </head>
+ <body>
+ <h1>Use This Page to Debug cssmin.js</h1>
+
+ <h1 id="notsupportedmsg" class="hidden">Your browser does not support the local file access apis used by this page.</h1>
+
+ <p>Select a css file to compress. You can then step through the cssmin.js implementation using your browser's script debugger.</p>
+
+ <p><input type="file" id="testFile"></p>
+
+ <h2>Compressed</h2>
+ <pre id="out"></pre>
+
+ <h2>Original</h2>
+ <pre id="in"></pre>
+
+ <script>
+ if (window.File && window.FileReader) {
+ document.getElementById('testFile').addEventListener('change', function(e) {
+ var file = this.files[0],
+ fr = new FileReader(),
+ input = document.getElementById("in"),
+ output = document.getElementById("out"),
+ contents;
+
+ fr.onload = function(e) {
+ input.innerHTML = e.target.result;
+ var min = YAHOO.compressor.cssmin(e.target.result);
+ output.innerHTML = min;
+ };
+
+ fr.readAsText(file, "utf-8");
+
+ }, false);
+ } else {
+ document.getElementById("notsupportedmsg").removeClass("hidden");
+ }
+ </script>
+ </body>
+</html>

0 comments on commit 87b4ee8

Please sign in to comment.