fix: don't corrupt whitespace in scripts

Fixes #120
remy · Apr 5, 2017 · 5528517 · specious · May 19, 2017 · remy
1 parent 83edb2b
commit 5528517
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 6 deletions.
diff --git a/lib/index.js b/lib/index.js
@@ -190,7 +190,7 @@ function main() {
     inliner.url = url; // this is a hack for the `resolve` function later on
     return inliner.get(this.filename || url, { encoding: 'binary' });
   })
-  .catch(function isUrl(error) {
+  .catch(function isUrl() {
     // make the best guess as to whether we're working with a url
     if (inliner.url || url.indexOf('<') === -1) {
       url = inliner.url || inliner.source;
@@ -312,12 +312,19 @@ function main() {
 
       // collapse the white space
       if (inliner.options.collapseWhitespace) {
+        debug('collapsing whitespace');
         $('pre, textarea').each(function () {
           $(this).html($(this).html()
               .replace(/\n/g, '~~nl~~')
               .replace(/\s/g, '~~s~~'));
         });
 
+        $('script').each(function () {
+          $(this).text($(this).text()
+              .replace(/\n/g, '~~nl~~')
+              .replace(/\s/g, '~~s~~'));
+        });
+
         html = $.html()
           .replace(/\s+/g, ' ')
           .replace(/~~nl~~/g, '\n')

diff --git a/lib/tasks/js.js b/lib/tasks/js.js
@@ -16,6 +16,7 @@ function resolve(inliner, todo, $) {
     var isMinified = false;
 
     if (type && type.toLowerCase() !== 'text/javascript') {
+      debug('skipping %s', type);
       return false;
     }
 

diff --git a/test/README.md b/test/README.md
@@ -0,0 +1,20 @@
+# Tests
+
+When filing bugs, if it's an inlining issue, ensure you include full test source to work against.
+
+This can be via a [PR](https://github.com/remy/inliner/pulls) or by linking to a [gist](https://gist.github.com) that include at least **two** (first) files:
+
+- `<issue>.src.html`
+- `<issue>.result.html`
+
+When these are put in the [fixtures](https://github.com/remy/inliner/tree/master/test/fixtures) directory, they are automatically tested against.
+
+If there are any external assets the example needs, please also include these and name them with the same root as your example, i.e. `<issue>.css` or `<issue>.min.js` etc.
+
+In addition `<issue>.opts.json` can be loaded to help specify runtime options during the test.
+
+**To test a single fixture you can use:**:
+
+```bash
+$ FILTER=<issue> npm test
+```
diff --git a/test/fixtures/120-script-template.result.html b/test/fixtures/120-script-template.result.html
@@ -0,0 +1,7 @@
+<!doctype html> <html> <body> <script type="text/template">
+  First paragraph
+
+  Second paragraph
+
+  # Heading
+  </script> <script>var foo=function(){console.log("foo")};foo();var bar=10;</script> </body></html>
diff --git a/test/fixtures/120-script-template.src.html b/test/fixtures/120-script-template.src.html
@@ -0,0 +1,18 @@
+<!doctype html>
+<html>
+<body>
+  <script type="text/template">
+  First paragraph
+
+  Second paragraph
+
+  # Heading
+  </script>
+  <script>
+  var foo = function () {
+    console.log('foo');
+  }
+
+  foo();
+  var bar = 10;
+  </script>
diff --git a/test/fixtures/backtick-string.result.html b/test/fixtures/backtick-string.result.html
@@ -0,0 +1,5 @@
+<script>const string = `This
+
+is a multline
+
+string`;</script>
diff --git a/test/fixtures/backtick-string.src.html b/test/fixtures/backtick-string.src.html
@@ -0,0 +1,7 @@
+<script>
+const string = `This
+
+is a multline
+
+string`;
+</script>
diff --git a/test/fixtures/headers.result.html b/test/fixtures/headers.result.html
@@ -1 +1 @@
-<script>{ "user-agent": "inliner" }</script>
+<script>"function"==typeof callback&&callback({"user-agent":"inliner"});</script>
diff --git a/test/fixtures/headers.src.html b/test/fixtures/headers.src.html
@@ -1 +1 @@
-<script src="https://httpbin.org/user-agent"></script>
+<script src="https://httpbin.now.sh/user-agent?callback=callback"></script>
diff --git a/test/index.test.js b/test/index.test.js
@@ -8,7 +8,7 @@ var st = require('st');
 var server;
 
 test('setup mock server', function (t) {
-  server = http.createServer(function(req, res) {
+  server = http.createServer(function (req, res) {
     if (isASCII(req.url)) {
       st(path.resolve(__dirname, 'fixtures'))(req, res);
     }
@@ -39,7 +39,7 @@ test('inliner core functions', function coreTests(t) {
   t.ok(inliner, 'inline is instantiated');
 
   var roundtripHTML = '<!DOCTYPE html><html></html>';
-  new Inliner(roundtripHTML, function(error, html) {
+  new Inliner(roundtripHTML, function (error, html) {
     t.equal(html, roundtripHTML, 'recognizes HTML as main input');
   });
 });
@@ -50,7 +50,7 @@ test('inliner handles given source as local', function sourcedTests(t) {
   t.plan(1);
 
   var content = fs.readFileSync(__dirname + '/fixtures/css-ext-import.src.html', 'utf8');
-  new Inliner(content, function (error, content) {
+  new Inliner(content, function (error) {
     t.equal(error, null, 'treats local content as file');
   });
 });
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		<script>{ "user-agent": "inliner" }</script>
		<script>"function"==typeof callback&&callback({"user-agent":"inliner"});</script>
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		<script src="https://httpbin.org/user-agent"></script>
		<script src="https://httpbin.now.sh/user-agent?callback=callback"></script>
This comment has been minimized. Sign in to view Copy link specious May 19, 2017 Contributor https://httpbin.now.sh returns 404. This comment has been minimized. Sign in to view Copy link remy May 19, 2017 Author Owner Dangit. I'll get the VM up again