jquery / jquery

jQuery JavaScript Library
https://jquery.com
MIT License
58.93k stars 20.62k forks source link

Triggering after an `alert()` in an event handler results in a JS error #5459

Open AllanJard opened 1 month ago

AllanJard commented 1 month ago

Description

It appears that if I trigger a blur event, immediately after an alert() inside a click event handler, it results in an error:

Uncaught TypeError: saved.slice is not a function

This is happening in jQuery 3.7.1 and 4.0.0-beta. But not in 3.5.1. I wonder if this issue might be related.

Link to test case

The way to reproduce this is quite simple:

    $('button').on('click', function (e) {
      alert('Hello');

      $(this).trigger('blur');
    })

https://jsbin.com/vuquzepidi/edit?html%2Cjs%2Coutput=

I've found that if I use console.log instead of alert() it works just fine. Also if I put the trigger into a setTimeout it works just fine. Likewise if I change it to trigger a focus event it works. I did try a focus before the blur, wondering if the alert might cause the button to loose focus, but no change, and even if it did, I would have expected an error.

Apologies, I haven't yet dug into the code to see what might be happening - I can see in the debugger that saved is true, but I've not looked to see why that is yet.

dmethvin commented 1 month ago

I can reproduce this on Firefox, but not Chrome or Edge. If a blur handler is added the error doesn't occur, but you can see that there is an extra focus/blur for Firefox compared to Chrome.

https://jsbin.com/pigorigazo/edit?html,console,output

mgol commented 1 month ago

Thanks for the report. That's an interesting bug.

The issue on the browser side is caused by Firefox apparently dispatching the blur event twice if an alert is followed by a native .blur() call. Our blur (and focus) handling is a bit complex to support passing data in trigger calls and to see the proper focus state of the element in the handler. What it roughly does is call the native .blur() internally, stopping the outer synthetic event, and then calling trigger again with proper parameters. The native blur handling is the branch under this if: https://github.com/jquery/jquery/blob/284b082eb86602705519d6ca754c40f6d2f8fcc0/src/event.js#L557-L559 It checks for a truthy saved which is usually an array of trigger arguments saved in the outer synthetic handler: https://github.com/jquery/jquery/blob/284b082eb86602705519d6ca754c40f6d2f8fcc0/src/event.js#L530 but if the inner native event fires twice, the second call sees the value set here: https://github.com/jquery/jquery/blob/284b082eb86602705519d6ca754c40f6d2f8fcc0/src/event.js#L561-L566 That value is the return value of the last handler called. In our case, since we attach a simple handler returning true here: https://github.com/jquery/jquery/blob/284b082eb86602705519d6ca754c40f6d2f8fcc0/src/event.js#L509 the value is true. That means the second inner handler sees true instead of an array and since the code above calls .slice() on it, we see a crash.

The reason @dmethvin discovered that attaching any blur handler before the problematic .trigger( "blur" ) call works around the issue is because we only attach the returnTrue handler as a hack to force event setup before the first trigger call if there wasn't a handler attached previously: https://github.com/jquery/jquery/blob/284b082eb86602705519d6ca754c40f6d2f8fcc0/src/event.js#L508-L510

PR: #5466