Event: Increase robustness of an inner native event in leverageNative

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

diff --git a/src/event.js b/src/event.js
index 0112264a47bc9a0f5b5231da1e27a96a48fce857..a216f9a8ca6d6893a932f86896c1d320ff488c28 100644 (file)
--- a/src/event.js
+++ b/src/event.js
@@ -519,14 +519,29 @@ function leverageNative( el, type, isSetup ) {
                        var result,
                                saved = dataPriv.get( this, type );
 
+                       // This controller function is invoked under multiple circumstances,
+                       // differentiated by the stored value in `saved`:
+                       // 1. For an outer synthetic `.trigger()`ed event (detected by
+                       //    `event.isTrigger & 1` and non-array `saved`), it records arguments
+                       //    as an array and fires an [inner] native event to prompt state
+                       //    changes that should be observed by registered listeners (such as
+                       //    checkbox toggling and focus updating), then clears the stored value.
+                       // 2. For an [inner] native event (detected by `saved` being
+                       //    an array), it triggers an inner synthetic event, records the
+                       //    result, and preempts propagation to further jQuery listeners.
+                       // 3. For an inner synthetic event (detected by `event.isTrigger & 1` and
+                       //    array `saved`), it prevents double-propagation of surrogate events
+                       //    but otherwise allows everything to proceed (particularly including
+                       //    further listeners).
+                       // Possible `saved` data shapes: `[...], `{ value }`, `false`.
                        if ( ( event.isTrigger & 1 ) && this[ type ] ) {
 
                                // Interrupt processing of the outer synthetic .trigger()ed event
-                               if ( !saved ) {
+                               if ( !saved.length ) {
 
                                        // Store arguments for use when handling the inner native event
-                                       // There will always be at least one argument (an event object), so this array
-                                       // will not be confused with a leftover capture object.
+                                       // There will always be at least one argument (an event object),
+                                       // so this array will not be confused with a leftover capture object.
                                        saved = slice.call( arguments );
                                        dataPriv.set( this, type, saved );
 
@@ -541,29 +556,35 @@ function leverageNative( el, type, isSetup ) {
                                                event.stopImmediatePropagation();
                                                event.preventDefault();
 
-                                               return result;
+                                               // Support: Chrome 86+
+                                               // In Chrome, if an element having a focusout handler is
+                                               // blurred by clicking outside of it, it invokes the handler
+                                               // synchronously. If that handler calls `.remove()` on
+                                               // the element, the data is cleared, leaving `result`
+                                               // undefined. We need to guard against this.
+                                               return result && result.value;
                                        }
 
-                               // If this is an inner synthetic event for an event with a bubbling surrogate
-                               // (focus or blur), assume that the surrogate already propagated from triggering
-                               // the native event and prevent that from happening again here.
-                               // This technically gets the ordering wrong w.r.t. to `.trigger()` (in which the
-                               // bubbling surrogate propagates *after* the non-bubbling base), but that seems
-                               // less bad than duplication.
+                               // If this is an inner synthetic event for an event with a bubbling
+                               // surrogate (focus or blur), assume that the surrogate already
+                               // propagated from triggering the native event and prevent that
+                               // from happening again here.
                                } else if ( ( jQuery.event.special[ type ] || {} ).delegateType ) {
                                        event.stopPropagation();
                                }
 
-                       // 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 ) {
+                       // 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.length ) {
 
                                // ...and capture the result
-                               dataPriv.set( this, type, jQuery.event.trigger(
-                                       saved[ 0 ],
-                                       saved.slice( 1 ),
-                                       this
-                               ) );
+                               dataPriv.set( this, type, {
+                                       value: jQuery.event.trigger(
+                                               saved[ 0 ],
+                                               saved.slice( 1 ),
+                                               this
+                                       )
+                               } );
 
                                // Abort handling of the native event by all jQuery handlers while allowing
                                // native handlers on the same element to run. On target, this is achieved

diff --git a/test/unit/event.js b/test/unit/event.js
index ac309d22dee6be20335e6a267bbf645481afc32f..111b1a18bcc643afa10981723b1a5915d5984a80 100644 (file)
--- a/test/unit/event.js
+++ b/test/unit/event.js
@@ -3502,6 +3502,64 @@ QUnit.test( "trigger(focus) fires native & jQuery handlers (gh-5015)", function(
        input.trigger( "focus" );
 } );
 
+QUnit.test( "duplicate native blur doesn't crash (gh-5459)", function( assert ) {
+       assert.expect( 4 );
+
+       function patchAddEventListener( elem ) {
+               var nativeAddEvent = elem[ 0 ].addEventListener;
+
+               // Support: Firefox 124+
+               // In Firefox, alert displayed just before blurring an element
+               // dispatches the native blur event twice which tripped the jQuery
+               // logic. We cannot call `alert()` in unit tests; simulate the
+               // behavior by overwriting the native `addEventListener` with
+               // a version that calls blur handlers twice.
+               //
+               // Such a simulation allows us to test whether `leverageNative`
+               // logic correctly differentiates between data saved by outer/inner
+               // handlers, so it's useful even without the Firefox bug.
+               elem[ 0 ].addEventListener = function( eventName, handler ) {
+                       var finalHandler;
+                       if ( eventName === "blur" ) {
+                               finalHandler = function wrappedHandler() {
+                                       handler.apply( this, arguments );
+                                       return handler.apply( this, arguments );
+                               };
+                       } else {
+                               finalHandler = handler;
+                       }
+                       return nativeAddEvent.call( this, eventName, finalHandler );
+               };
+       }
+
+       function runTest( handler, message ) {
+               var button = jQuery( "<button></button>" );
+
+               patchAddEventListener( button );
+               button.appendTo( "#qunit-fixture" );
+
+               if ( handler ) {
+                       button.on( "blur", handler );
+               }
+               button.on( "focus", function() {
+                       button.trigger( "blur" );
+                       assert.ok( true, "Did not crash (" + message + ")" );
+               } );
+               button.trigger( "focus" );
+       }
+
+       runTest( undefined, "no prior handler" );
+       runTest( function() {
+               return true;
+       }, "prior handler returning true" );
+       runTest( function() {
+               return { length: 42 };
+       }, "prior handler returning an array-like" );
+       runTest( function() {
+               return { value: 42 };
+       }, "prior handler returning `{ value }`" );
+} );
+
 // TODO replace with an adaptation of
 // https://github.com/jquery/jquery/pull/1367/files#diff-a215316abbaabdf71857809e8673ea28R2464
 ( function() {