rviscomi / trunk8

jQuery Truncation Plugin -- THIS PROJECT IS NO LONGER MAINTAINED
MIT License
703 stars 95 forks source link

fix(jquery-trunk8): Fix cross-browser compatibility issue with stripping HTML #47

Closed jamesbrewerdev closed 10 years ago

jamesbrewerdev commented 10 years ago

Internet Explorer introduced element.innerText a while back which retrieves the text from a DOM node. All other major browsers have agreed to use Node.textContent instead. While most of them also support element.innerText, Firefox does not.

The trunk8 plugin for jQuery uses the check tmp.textContent || tmp.innerText to retrieve the text from a given node. This won't work in Firefox when tmp has no content -- specifically when html is an empty string -- because tmp.textContent returns "" (an empty string, which is falsey in JavaScript) and tmp.innerText returns undefined. Because of JavaScript's short circuiting feature, the expression tmp.textContent || tmp.innerText then returns undefined. This caused an error in one code which ultimately resulted in the page not rendering entirely.

This fix checks to see whether the browser supports Node.textContent and, if so, returns tmp.textContent. Otherwise, return tmp.innerText.

ztratar commented 10 years ago

Why not the following? Shorter & sweeter.

return tmp.textContent || tmp.innerText || "";

jamesbrewerdev commented 10 years ago

It will return the same result, but I think my solution better explains that tmp.innerText should only be returned if typeof tmp.textContent != 'undefined' is true.

Returning an empty string is more of a hack that hides the problem instead of fixing it.

ztratar commented 10 years ago

If you feel strongly. :)

What about !== vs !=? I can't approve a merge, btw, so I'm just chatting.