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
This commit is contained in:
Michał Gołębiowski-Owczarek 2024-05-20 18:05:19 +02:00 committed by GitHub
parent 5880e02707
commit 527fb3dcf0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 97 additions and 18 deletions

View File

@ -519,14 +519,29 @@ function leverageNative( el, type, isSetup ) {
var result, var result,
saved = dataPriv.get( this, type ); 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 ] ) { if ( ( event.isTrigger & 1 ) && this[ type ] ) {
// Interrupt processing of the outer synthetic .trigger()ed event // Interrupt processing of the outer synthetic .trigger()ed event
if ( !saved ) { if ( !saved.length ) {
// Store arguments for use when handling the inner native event // Store arguments for use when handling the inner native event
// There will always be at least one argument (an event object), so this array // There will always be at least one argument (an event object),
// will not be confused with a leftover capture object. // so this array will not be confused with a leftover capture object.
saved = slice.call( arguments ); saved = slice.call( arguments );
dataPriv.set( this, type, saved ); dataPriv.set( this, type, saved );
@ -541,29 +556,35 @@ function leverageNative( el, type, isSetup ) {
event.stopImmediatePropagation(); event.stopImmediatePropagation();
event.preventDefault(); 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 // If this is an inner synthetic event for an event with a bubbling
// (focus or blur), assume that the surrogate already propagated from triggering // surrogate (focus or blur), assume that the surrogate already
// the native event and prevent that from happening again here. // propagated from triggering the native event and prevent that
// This technically gets the ordering wrong w.r.t. to `.trigger()` (in which the // from happening again here.
// bubbling surrogate propagates *after* the non-bubbling base), but that seems
// less bad than duplication.
} else if ( ( jQuery.event.special[ type ] || {} ).delegateType ) { } else if ( ( jQuery.event.special[ type ] || {} ).delegateType ) {
event.stopPropagation(); event.stopPropagation();
} }
// If this is a native event triggered above, everything is now in order // If this is a native event triggered above, everything is now in order.
// Fire an inner synthetic event with the original arguments // Fire an inner synthetic event with the original arguments.
} else if ( saved ) { } else if ( saved.length ) {
// ...and capture the result // ...and capture the result
dataPriv.set( this, type, jQuery.event.trigger( dataPriv.set( this, type, {
saved[ 0 ], value: jQuery.event.trigger(
saved.slice( 1 ), saved[ 0 ],
this saved.slice( 1 ),
) ); this
)
} );
// Abort handling of the native event by all jQuery handlers while allowing // 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 // native handlers on the same element to run. On target, this is achieved

View File

@ -3502,6 +3502,64 @@ QUnit.test( "trigger(focus) fires native & jQuery handlers (gh-5015)", function(
input.trigger( "focus" ); 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 // TODO replace with an adaptation of
// https://github.com/jquery/jquery/pull/1367/files#diff-a215316abbaabdf71857809e8673ea28R2464 // https://github.com/jquery/jquery/pull/1367/files#diff-a215316abbaabdf71857809e8673ea28R2464
( function() { ( function() {