Replace '\uFEFF' with '!' for setInnerHTML to avoid UglifyJS bug

[syranide]

#2247 (and mishoo/UglifyJS#556)

This is obviously quite a serious bug and even if uglifyjs fixes it, old versions will still be around to haunt people who minify themselves.

The comment should explain everything. The char is immediately removed so it should never affect rendering.

Test plan: none (should test in IE8)

cc @zpao @spicyj

[zpao]

This code path is only for IE8, correct?
Can you actually test in IE8 :)

[syranide]

@zpao Knowingly, it exclusively targets IE8 only and it's most definitely safe (I used an ASCII-char at the start).

So as I mentioned on IRC; this PR is safe to pull.

... but I've found that createNodesFromMarkup does NOT use setInnerHTML... it uses getMarkupWrap which when combined apparently includes both of my fixes in setInnerHTML but slightly differently (due to the nature of createNodesFromMarkup). It's possible that createNodesFromMarkup shouldn't replace setInnerHTML as-is, I don't really know, but I think we should move to createNodesFromMarkup/getMarkupWrap instead of having two separate paths for largely the same purpose. Will need to look into this.

[syranide]

I've opened #2273 to track the status of setInnerHTML vs createNodesFromMarkup.

[sophiebits]

Remind me why in #684 we didn't decide to manually insert a whitespace text node at the beginning of the markup like jQuery does in IE8? That feels cleaner to me than inserting an extra character only to remove it immediately.

[syranide]

@spicyj Hmm, not sure if I agree, fiddeling with textnodes seems more low-level than just prepending a char and removing it. Unless I misunderstand how jQuery does it.

[syranide]

PS. Also it doesn't really matter, we should be going the way of #2273 instead I think (createNodesFromMarkup uses the same trick).

[syranide]

I've submitted #2340 which should be preferable.

[syranide]

Since we're not rushing this, we should do it properly with #2340 instead.