Unable to refactor inline script contained innerHTML in JavaScript

Question

I currently have the following code in JavaScript:

container.innerHTML = `
            <h3 id="box-title"> Build Analysis
             </u><span id="close" class="close" 
            onclick=parentNode.parentNode.style.display='none';>x</span></h3>
             .... /* skipping the rest because it is irrelevant */

This is just code for dynamically inserting HTML content in the webpage (in this case box that can be opened and closed). My question is regarding in-line scripting. Currently it works with the onclick (which closes the window), but it uses inline scripting, which is not good.

I tried the solutions from this link:(the part with refactoring in-line scripting) https://csp.withgoogle.com/docs/adopting-csp.html

I tried the following solution from there, but nothing happens: In most cases the changes will be straightforward. To refactor event handlers, rewrite them to be added from a JavaScript block:

Copied from there:

<script> function doThings() { ... } </script>
<span onclick="doThings();">A thing.</span>

should become:

<span id="things">A thing.</span>

<script nonce="${nonce}">
document.addEventListener('DOMContentLoaded', function () {
  document.getElementById('things')
          .addEventListener('click', function doThings() { ... });
});
</script>

However when applying it my project, it doesnt seem to work:

 ...
 </u><span id="close" class="close">x</span></h3>
...

<script>
                document.addEventListener('DOMContentLoaded', function () {
                document.getElementById('close')
                        .addEventListener('click', function closeBox() {
                            parentNode.parentNode.style.display='none';
                        });
                });
    </script>

I also tried putting a onclick=CloseBox() in the tag. It just doesn't register this functionality and nothing happens. Not even an error occurs. Can anyone please help? Thank you!

@appleapple https://csp.withgoogle.com/docs/adopting-csp.html — Gergan Zhekov, Nov 13 '19 at 09:33
sorry, I cannot find relevant sentence, are you refering XSS? it's not the case here. — apple apple, Nov 13 '19 at 09:36

T.J. Crowder · Accepted Answer · 2019-11-13T09:36:30.663

The reason it doesn't work is that when you have a script tag in HTML that you assign to innerHTML, the script is not run by the browser.

Instead, you would add your code to the code that's assigning this to innerHTML:

container.innerHTML = `
    <h3 id="box-title"> <u>Build Analysis
     </u><span id="close" class="close">x</span></h3>
     ....`;
document.getElementById('close')
    .addEventListener('click', function closeBox() {
        this.parentNode.parentNode.style.display='none';
    });

Also note that you need this. before the first parentNode (added above) and the starting tag for the u element (added above).

Live Example:

var container = document.getElementById("container");
container.innerHTML = `
    <h3 id="box-title"><u> Build Analysis
     </u><span id="close" class="close">x</span></h3>
     ....`;
document.getElementById('close')
    .addEventListener('click', function closeBox() {
        this.parentNode.parentNode.style.display='none';
    });

<div id="container"></div>

If the script tag has to be in the string being assigned (for instance, because you get the string from somewhere else, rather than it being in your actual code as it is in your question), you can get the script content after the fact and run it. After the innerHTML assignment:

 container.querySelectorAll("script").forEach(function(script) {
     var newScript = document.createElement("script");
     newScript.textContent = script.textContent;
     document.body.appendChild(newScript); // runs it...
     document.body.removeChild(newScript); // ...after which the `script` element is unnecessary
 });

(You may need to polyfill NodeList.prototype.forEach for older browsers, see my answer here for how to do that.)

Live Example:

var container = document.getElementById("container");
container.innerHTML = `
    <h3 id="box-title"><u> Build Analysis
     </u><span id="close" class="close">x</span></h3>
     <script>
     document.getElementById('close')
         .addEventListener('click', function closeBox() {
             this.parentNode.parentNode.style.display='none';
         });
     <\/script>
     ....`;
 container.querySelectorAll("script").forEach(function(script) {
     var newScript = document.createElement("script");
     newScript.textContent = script.textContent;
     document.body.appendChild(newScript); // runs it...
     document.body.removeChild(newScript); // ...after which the `script` element is unnecessary
 });

<div id="container"></div>

Side note: The code in the function could be made more robust by using closest:

document.getElementById('close')
    .addEventListener('click', function closeBox() {
        this.closest("h3").style.display='none';
    });

Thank you so much for the information provided, I made it work and learned a lot of helpful things! — Gergan Zhekov, Nov 13 '19 at 09:39

Unable to refactor inline script contained innerHTML in JavaScript

1 Answers1