Why Javascript event handler is called twice?

Question

When I add an event handler to element of a class as follows:

<html>

<head>
  <title>test</title>
</head>

<body>

  <div class="class1" id="div1">div1</div>

  <script>
    var divs = document.getElementsByClassName("class1");
    var onclick = function() {
      alert("clicked");
    }
    for (var i = 0; i < divs.length; i++) {
      divs[i].addEventListener("click", onclick);
    }
  </script>
</body>

</html>

The alert window is popped up twice if I click on the div element. But if I use the following code, the alert window is only popped up once.

<html>

<head>
  <title>test</title>
</head>

<body>
  <div class="class1" id="div1">div1</div>
  <script>
    var divs = document.getElementsByClassName("class1");
    var onclick = function() {
      alert("clicked");
    }

    divs.addEventListener("click", onclick);
  </script>
</body>

</html>

The following code also pops up the alert window twice:

<html>

<head>
  <title>test</title>
</head>

<body>

  <div class="class1" id="div1">div1</div>

  <script>
    var div = document.getElementById("div1");
    var onclick = function() {
      alert("clicked");
    }
    div.addEventListener("click", onclick);
  </script>
</body>

</html>

I wonder why the event handler is called twice, regardless I use addEventListener("click",onclick, true); or addEventListener("click",onclick,false);

I use Firefox browser.

JLRishe · Accepted Answer · 2021-03-29T08:41:40.093

4

This is happening because your onclick variable is a global variable, and that means you are essentially doing this when you assign the function to onclick:

window.onclick = function () {
    alert('click');
};

So you are adding the event listener to your divs, and window, and when you click the div, it is firing once on the div, and once on the window.

To avoid this, use an iife to shield your variables from the global scope:

<html>

<head>
  <title>test</title>
</head>

<body>

  <div class="class1" id="div1">div1</div>

  <script>
    (function () {
        var divs = document.getElementsByClassName("class1");
        var onclick = function() {
          alert("clicked");
        }
        for (var i = 0; i < divs.length; i++) {
          divs[i].addEventListener("click", onclick);
        }
    })();
  </script>
</body>

</html>

edited Mar 29 '21 at 08:41

answered Mar 29 '21 at 08:35

JLRishe

99,490
19
131
169

Better is to rename and delegate – mplungjan Mar 29 '21 at 08:39
@mplungjan Rename and delegate? I'm not sure what you mean. – JLRishe Mar 29 '21 at 08:39
@mplungjan Delegation is fine, but I don't think it's any better or worse than assigning the event handler to each `div`. There are times when delegation is better, but I don't think this is one of them. As for renaming, we should be avoiding pollution of the global scope regardless, so there isn't really a need to rename. And your answer is ok as a suggestion of an alternate approach, but you didn't answer OP's question. – JLRishe Mar 29 '21 at 08:47
As I answer, if there is only one div, then there is no need to loop either. And I would use an anonymous function anyway – mplungjan Mar 29 '21 at 08:50

mplungjan · Answer 2 · 2023-03-19T06:49:04.513

Why loop at all. Apart from the name (you overwrite the native onclick) issue, why not just add once? You have only one div with a unique ID.

Using an anonymous function makes it even safer not to accidentally use a native function name

const div = document.getElementById("div1");

div.addEventListener("click", () => {
  alert("clicked");
});

<div class="class1" id="div1">div1</div>

NOTE: Be careful using anonymous functions in a loop though. I never do so we are safe here

If you have more than one element to use the same event handler, I always recommend to delegate

document.getElementById("container").addEventListener("click", (e) => {
  const div = e.target.closest("div");
  if (div && div.matches(".class1")) {
    alert(div.id + " clicked");
  }
});

<div id="container">
  <div class="class1" id="div1">div1</div>
  <div class="class1" id="div2">div2</div>
</div>

score 0 · Answer 3 · edited Aug 12 '23 at 13:59

0

The only solution that worked for me was to force that the addEventListener to be called once.

Like:

   if (!window.onClickListenerAdded) {
      const onClickListener = (event) => {
        alert("clicked");
      };
      window.removeEventListener('click', onClickListener);
      window.addEventListener('click', onClickListener);
      window.onClickListenerAdded = true;
    }

edited Aug 12 '23 at 13:59

Adrian Mole

49,934
160
51
83

answered Aug 10 '23 at 06:20

Fabio Mariano

1

Why Javascript event handler is called twice?

3 Answers3

Linked