search

tvler / lazy-progressive-enhancement

A lazy image loader designed to enforce progressive enhancement and valid HTML.
http://tylerdeitz.co/lazy-progressive-enhancement/
MIT License
190 stars 10 forks source link

Use .innerHTML instead of .textContent #2

Closed agarzola closed 8 years ago

agarzola commented 8 years ago

I ran into this bug while playing around with this for #1.

tvler commented 8 years ago

Hey! Thanks for the pull request. What was going wrong by using textcontent?

agarzola commented 8 years ago

Nothing was returned. Now that I think about it some, it might have had something to do with the fact I was generating noscript tags dynamically. Regular noscript tags might be interpreted differently when loaded as part of the static document, where its content is interpreted as plain text instead of child nodes.

That said, this S.O. answer suggests that some browsers might interpret noscript’s contents as DOM nodes, so maybe do el.textContent || el.innerHTML to be sure? I’ll add that change in a sec.

tvler commented 8 years ago

Yo. I regressed using textContent for now. I'll add it back when I check out dynamically including noscript elements after pageload.

By just using innerHTML, it looks like the content inside of noscript is being inserted as an HTML escaped string (checkout the value of console.log of noscript.innerHTML on line 38).

Example problem: https://jsfiddle.net/hvq46nb6/1/

agarzola commented 8 years ago

I think the issue I was having was caused by the dynamic creation of noscript elements (as opposed to loading them in HTML) because I wanted to generate new URLs for the images on each run.

Looking back at it, I really should have put more thought into the problem before submitting a pull request.

tvler commented 8 years ago

No worries! I'll be checking it out in more detail when I implement the scroll load setting : )