search

remy / inliner

Node utility to inline images, CSS and JavaScript for a web page - useful for mobile sites
MIT License
1.1k stars 165 forks source link

`</script>` → `<\/script>` workaround #11

Closed mathiasbynens closed 9 years ago

mathiasbynens commented 13 years ago

You have a workaround in place for an uglify.js bug, which is great. Might be interesting to keep an eye on https://github.com/mishoo/UglifyJS/issues/164 (I’m still hoping this will be fixed in uglify.js itself).

I recently did some ETAGO research, and you can read the full details here: https://mathiasbynens.be/notes/etago Here’s the TL;DR:

Just keep in mind that the full </style and </script strings followed by a space character, >, or / will close their respective opening tag.

In other words, the current workaround only detects one of those three cases.

remy commented 13 years ago

I could just re-apply that logic after the compression has happened. Simple fix and doens't rely on a fix in uglify - what do you think?

mathiasbynens commented 13 years ago

Sounds good. When (if) this eventually gets fixed in uglifyjs, you can then remove it — or just leave it in anyway.

remy commented 13 years ago

Damn, looks like I was already doing that: https://github.com/remy/inliner/blob/master/inliner.js#L216

Have you got an example of a url you're hitting? ... unless it's another tag hitting.

mathiasbynens commented 13 years ago

Yeah, I edited my bug report right after posting it explaining that you already work around it (partially). You probably read your email notification and missed my clarification — sorry about that. Anyhow, read ^! ;)

[…] unless it's another tag hitting.

It’s not a problem for other tags, only for script elements. The point is that only escaping the full </script> string still isn’t completely safe. (See first post in this thread after my edits.)

remy commented 9 years ago

This should be fixed @ 1.0.0 (but will reopen otherwise!).