Skip to content

Commit

Permalink
Event: Increase robustness of an inner native event in leverageNative
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mgol committed Apr 28, 2024
1 parent 5880e02 commit 0582060
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 16 deletions.
40 changes: 24 additions & 16 deletions src/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -522,11 +522,12 @@ function leverageNative( el, type, isSetup ) {
if ( ( event.isTrigger & 1 ) && this[ type ] ) {

// Interrupt processing of the outer synthetic .trigger()ed event
if ( !saved ) {
// Detect `saved` of shape `{ value }` and `false`.
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 );

Expand All @@ -541,29 +542,36 @@ 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 ) {
// Exclude `saved` of shape `{ value }` and `false`.
} 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
Expand Down
58 changes: 58 additions & 0 deletions test/unit/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 0582060

Please sign in to comment.