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

AllanJard · 2024-03-28T10:02:34Z

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.

The text was updated successfully, but these errors were encountered:

dmethvin · 2024-04-01T16:28:44Z

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

In Firefox, alert displayed just before blurring an element dispatches the native blur event twice which tripped the jQuery logic if a jQuery blur handler was not attached before the trigger call. This was because the `leverageNative` logic part for triggering first checks if setup was done before (which, for example, is done if a jQuery handler was registered before for this element+event pair) and - if it was not - adds a dummy handler that just returns `true`. The `leverageNative` logic makes that `true` then saved into private data, replacing the previous `saved` array. Since `true` passes the truthy check, the second native inner handler treated `true` as an array, crashing on the `slice` call. Since it's impossible to call `alert()` in unit tests, simulate the issue by replacing the `addEventListener` method on a test button with a version that calls attached blur handlers twice. Fixes jquerygh-5459 Ref jquerygh-5236

mgol · 2024-04-02T15:59:21Z

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:

jquery/src/event.js

Lines 557 to 559 in 284b082

    
           // If this is a native event triggered above, everything is now in order 
        
           // Fire an inner synthetic event with the original arguments 
        
           } else if ( saved ) {

It checks for a truthy saved which is usually an array of trigger arguments saved in the outer synthetic handler:

jquery/src/event.js

Line 530 in 284b082

saved = slice.call( arguments );

but if the inner native event fires twice, the second call sees the value set here:

jquery/src/event.js

Lines 561 to 566 in 284b082

    
           // ...and capture the result 
        
           dataPriv.set( this, type, jQuery.event.trigger( 
        
           	saved[ 0 ], 
        
           	saved.slice( 1 ), 
        
           	this 
        
           ) );

That value is the return value of the last handler called. In our case, since we attach a simple handler returning true here:

jquery/src/event.js

Line 509 in 284b082

jQuery.event.add( el, type, returnTrue );

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:

jquery/src/event.js

Lines 508 to 510 in 284b082

    
           if ( dataPriv.get( el, type ) === undefined ) { 
        
           	jQuery.event.add( el, type, returnTrue ); 
        
           }

PR: #5466

In Firefox, alert displayed just before blurring an element dispatches the native blur event twice which tripped the jQuery logic if a jQuery blur handler was not attached before the trigger call. This was because the `leverageNative` logic part for triggering first checked if setup was done before (which, for example, is done if a jQuery handler was registered before for this element+event pair) and - if it was not - added a dummy handler that just returned `true`. The `leverageNative` logic made that `true` then saved into private data, replacing the previous `saved` array. Since `true` passed the truthy check, the second native inner handler treated `true` as an array, crashing on the `slice` call. The same issue could happen if a handler returning `true` is attached before triggering. A bare `length` check would not be enough as the user handler may return an array-like as well. To remove this potential data shape clash, capture the inner result in an object with a `value` property instead of saving it directly. Since it's impossible to call `alert()` in unit tests, simulate the issue by replacing the `addEventListener` method on a test button with a version that calls attached blur handlers twice. Fixes jquerygh-5459 Closes jquerygh-5466 Ref jquerygh-5236

In Firefox, alert displayed just before blurring an element dispatches the native blur event twice which tripped the jQuery logic if a jQuery blur handler was not attached before the trigger call. This was because the `leverageNative` logic part for triggering first checked if setup was done before (which, for example, is done if a jQuery handler was registered before for this element+event pair) and - if it was not - added a dummy handler that just returned `true`. The `leverageNative` logic made that `true` then saved into private data, replacing the previous `saved` array. Since `true` passed the truthy check, the second native inner handler treated `true` as an array, crashing on the `slice` call. The same issue could happen if a handler returning `true` is attached before triggering. A bare `length` check would not be enough as the user handler may return an array-like as well. To remove this potential data shape clash, capture the inner result in an object with a `value` property instead of saving it directly. Since it's impossible to call `alert()` in unit tests, simulate the issue by replacing the `addEventListener` method on a test button with a version that calls attached blur handlers twice. Fixes gh-5459 Closes gh-5466 Ref gh-5236

In Firefox, alert displayed just before blurring an element dispatches the native blur event twice which tripped the jQuery logic if a jQuery blur handler was not attached before the trigger call. This was because the `leverageNative` logic part for triggering first checked if setup was done before (which, for example, is done if a jQuery handler was registered before for this element+event pair) and - if it was not - added a dummy handler that just returned `true`. The `leverageNative` logic made that `true` then saved into private data, replacing the previous `saved` array. Since `true` passed the truthy check, the second native inner handler treated `true` as an array, crashing on the `slice` call. The same issue could happen if a handler returning `true` is attached before triggering. A bare `length` check would not be enough as the user handler may return an array-like as well. To remove this potential data shape clash, capture the inner result in an object with a `value` property instead of saving it directly. Since it's impossible to call `alert()` in unit tests, simulate the issue by replacing the `addEventListener` method on a test button with a version that calls attached blur handlers twice. Fixes gh-5459 Closes gh-5466 Ref gh-5236 (cherry picked from commit 527fb3d)

AllanJard · 2024-05-21T08:42:21Z

Thank you very much :-).

As always, immense respect for every thing you do for jQuery and Javascript in general!

mgol self-assigned this Apr 2, 2024

mgol added the Event label Apr 2, 2024

mgol added this to the 3.7.2 milestone Apr 2, 2024

mgol mentioned this issue Apr 2, 2024

Event: Increase robustness of an inner native event in leverageNative #5466

Merged

1 task

mgol added the Has Pull Request label Apr 2, 2024

mgol closed this as completed in #5466 May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

AllanJard commented Mar 28, 2024

dmethvin commented Apr 1, 2024

mgol commented Apr 2, 2024 •

edited

AllanJard commented May 21, 2024

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

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

Comments

AllanJard commented Mar 28, 2024

Description

Link to test case

dmethvin commented Apr 1, 2024

mgol commented Apr 2, 2024 • edited

AllanJard commented May 21, 2024

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

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

mgol commented Apr 2, 2024 •

edited