search

ded / bonzo

library agnostic, extensible DOM utility
Other
1.32k stars 137 forks source link

Make html() more robust for older versions of IE #51

Closed runningskull closed 12 years ago

runningskull commented 12 years ago

IE8 will throw a "Unknown Runtime Error" if you attempt to set the innerHTML for a block-level element nested inside a inline-level element (ie. a <div> inside a <p>).

If we catch an error trying to set the innerHTML, we can fall back on empty()'ing the element and then appending the HTML as an element instead.

This is the same technique that jQuery uses (https://github.com/jquery/jquery/blob/master/src/manipulation.js#L231-243), presumably for the same reason.

Thanks as always for the great micro-libraries. They rock my world.

ps. Here's the earliest mention of this quirk I can find: http://blog.rakeshpai.me/2007/02/ies-unknown-runtime-error-when-using.html

rvagg commented 12 years ago

Some tests to go with this would be good.

runningskull commented 12 years ago

Sure thing. I'll add some in soon.

runningskull commented 12 years ago

When run against the current codebase (w/o these changes), this test should catch an exception and fail in IE8. It should pass in all other browsers, and will pass in IE8 with the changes. Is that sufficient, or do we need more tests? Thanks!

ded commented 12 years ago

both p and div elements are block-level. and the tests still fail?

runningskull commented 12 years ago

D'oh! Of course they're both block-level. I'll blame it on the late night (and the heavy drinking) ;)

It appears to be a problem limited to p tags (and possibly others?), not inline elements. I'm not sure what the root cause is at this point. However, I just checked and the test does fail against the current codebase in IE8 (or at least IE9 emulating IE8).

I added in a few additional tests that document other aspects of the behavior. They may not be strictly necessary, though, so feel free to remove them.

runningskull commented 12 years ago

It seems to be a problem inserting block-level tags into a p tag specifically. Apparently IE8 decided to be inconsistently strict about conforming to http://www.w3.org/TR/html4/struct/text.html#h-9.3.1

IE8 will allow you to insert a block-level tag inside an inline tag (like a span) using innerHTML. I didn't include a test, but I'd be happy to if it's helpful/necessary.

rvagg commented 12 years ago

Can I suggest this as a test (inside a try/catch like you have):

var el = $($.create(fixture)).html(fixture)[0];
ok(el.innerHTML.toLowerCase().indexOf(fixture) != -1, 'got a &lt;p&gt; into a &lt;p&gt;');

IE should have a heart-attack on <p><p></p></p> and you have that in fixture to play with; plus you also get to check that the append actually happened and not that you've just captured the error.

runningskull commented 12 years ago

Good call. Done and done. IE8 pukes on it, of course. I also added a couple more tests to make sure the fixtures actually get appended to the <p> and not just that we caught the exception.