Hitting X on a modal won't close the modal with onclick but will close it with addEventListener and yet I still get an error

I work with Microsoft Edge version 120.0.2210.91 (Official build) (64-bit).

I have created this modal with an X button that if clicked, should close the modal.

document.body.insertAdjacentHTML('beforeend', `
<div class="modal_wrapper">
    <span class="modal_closing_button" onclick="this.parentNode.style.display = 'none';">X</span>
    <div class="modal_message">
    </div>
  </div>
`)

newStyle = document.createElement("style");
newStyle.type = "text/css";
newStyle.innerHTML +=`
  .modal_wrapper {
      display: flex;
      align-items: center;
      justify-content: center;
      text-align: center;
      position: fixed;
      top: 0;
      right: 0;
      bottom: 0;
      left: 0;
      width: 100%;
      height: 100%;
      z-index: 9999;
      font-weight: bold;
      background: green;
  }

  .modal_closing_button {
    display: block;
    position: absolute;
    text-align: right;
    color: #fff;
    font-size: 5em;
    top: 1em;
    right: 1em;
  }

  .modal_message {
      padding: 25px 25px 0 25px;
      margin: 0 0 10px 0;
      font-size: 20px;
      display: block;
      color: #fff;
  }
`;

document.head.appendChild(newStyle);

When I click the X button I get this error:

Refused to execute inline event handler because it violates the following Content Security Policy directive:

“script-src ‘self’ blob: filesystem:”.

Either the ‘unsafe-inline’ keyword, a hash (‘sha256-…’), or a nonce (‘nonce-…’) is required to enable inline execution.

Note that hashes do not apply to event handlers, style attributes and javascript: navigations unless the ‘unsafe-hashes’ keyword is present.

I didn’t quite understand this error, but from this Stack Overflow discussion I understand that onclick event listeners are often “invalid” and addEvenetListener('click', FUNCTION) should normally be used instead.

This JS code practically works.

document.body.insertAdjacentHTML('beforeend', `
<div class="modal_wrapper">
    <span class="modal_closing_button">X</span>
    <div class="modal_message">
    </div>
  </div>
`)

newStyle = document.createElement("style");
newStyle.type = "text/css";
newStyle.innerHTML +=`
  .modal_wrapper {
      display: flex;
      align-items: center;
      justify-content: center;
      text-align: center;
      position: fixed;
      top: 0;
      right: 0;
      bottom: 0;
      left: 0;
      width: 100%;
      height: 100%;
      z-index: 9999;
      font-weight: bold;
      background: green;
  }

  .modal_closing_button {
    display: block;
    position: absolute;
    text-align: right;
    color: #fff;
    font-size: 5em;
    top: 1em;
    right: 1em;
  }

  .modal_message {
      padding: 25px 25px 0 25px;
      margin: 0 0 10px 0;
      font-size: 20px;
      display: block;
      color: #fff;
  }
`;

document.head.appendChild(newStyle);

window.addEventListener('click', ()=>{
    document.querySelector('.modal_closing_button').parentNode.remove();
});

Anyway, although it seems to me to work fine, I get this console error after running it:

Uncaught TypeError: Cannot read properties of null (reading ‘parentNode’)
at :51:52

What have I done wrong and please share if you will do anything here slightly different.

Just a guess but if you only want it clicked once then try adding the once option.

e.g.

window.addEventListener(
  "click",
  () => {
    document.querySelector(".modal_closing_button").parentNode.remove();
  },
  { once: true }
);

Not sure if that’s the issue but I guess you that previously you were removing the element that the click handler was looking for.

1 Like

It indeed made the error go away. :slight_smile:

Isn’t it some kind of a “bug” or a “design problem” in the current release of EcmaScript that the error was there in the first place? I mean, the event handler should be associated with an element so long as that element exists, but not after it is removed, is it not?

The first error is because you tried to inject a script line into an element, triggering the browser’s safety alarms. (Consider what would happen if you were a bad actor, and injected a line into a website’s link tags to do very nasty stuff…)

You bound the event to the Window. The window still exists, so why wouldnt the event? (It’s not a problem in the language… it’s a problem in the user’s usage :P)

2 Likes

Thanks for your kind wording.
I understand the problem I have caused by not targeting the modal itself but the window object instead.

I have double checked the following code and didn’t get an error:

document.body.insertAdjacentHTML('beforeend', `
<div class="modal_wrapper">
    <span class="modal_closing_button">X</span>
    <div class="modal_message">
    </div>
  </div>
`)

newStyle = document.createElement("style");
newStyle.type = "text/css";
newStyle.innerHTML +=`
  .modal_wrapper {
      display: flex;
      align-items: center;
      justify-content: center;
      text-align: center;
      position: fixed;
      top: 0;
      right: 0;
      bottom: 0;
      left: 0;
      width: 100%;
      height: 100%;
      z-index: 9999;
      font-weight: bold;
      background: green;
  }

  .modal_closing_button {
    display: block;
    position: absolute;
    text-align: right;
    color: #fff;
    font-size: 5em;
    top: 1em;
    right: 1em;
  }

  .modal_message {
      padding: 25px 25px 0 25px;
      margin: 0 0 10px 0;
      font-size: 20px;
      display: block;
      color: #fff;
  }
`;

document.head.appendChild(newStyle);

document.querySelector('.modal_wrapper').addEventListener('click', ()=>{
    document.querySelector('.modal_closing_button').parentNode.remove();
});

BTW, I think I should give better CSS names there.

1 Like

The names seem fine-ish (I assume this is you injecting code into a site you dont control, in which case you’re fine as long as the original site doesnt use those class names elsewhere… might be confusing if they use a different modal, though.)

The code as written should work just fine. You’ve bound the event to the modal wrapper, which will be destroyed when the modal goes away.

You could simplify it a bit - there’s not much point in finding the button just to go up to the wrapper; just find the wrapper to begin with.

If in the future you want to interact with the modal in any way OTHER than to close it, you may need to add some extra logic.

I am trying to create a small modal for my personal website; this is just a “skeleton” code (more HTML and much more CSS to be done).

You could simplify it a bit - there’s not much point in finding the button just to go up to the wrapper; just find the wrapper to begin with.

I don’t know what you mean exactly; how would you prefer to add a purely JavaScript “skeleton” modal to a website you control?

There’s no point in finding the button, then going up to the parent of that button. You already know what the parent’s going to be, because you created it.

document.querySelector('.modal_wrapper').remove();

I believe that instead:

document.querySelector('.modal_wrapper').addEventListener('click', ()=>{
    document.querySelector('.modal_closing_button').parentNode.remove();
});

You meant:

document.querySelector('.modal_wrapper').addEventListener('click', ()=>{
    document.querySelector('.modal_wrapper').remove();
});

I understand that the advantage is mainly that it’s shorter.

Mainly. There is a slight improvement in operations, because Javascript doesnt have to do the extra step of “who’s the parent”. It’s never going to be a noticable improvement, but…

2 Likes