Skip to content

Commit 37bded3

Browse files
committed
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 jquerygh-5459 Closes jquerygh-5466 Ref jquerygh-5236 (cherry picked from commit 527fb3d)
1 parent 18adf66 commit 37bded3

File tree

2 files changed

+97
-18
lines changed

2 files changed

+97
-18
lines changed

src/event.js

+39-18
Original file line numberDiff line numberDiff line change
@@ -533,14 +533,29 @@ function leverageNative( el, type, isSetup ) {
533533
var result,
534534
saved = dataPriv.get( this, type );
535535

536+
// This controller function is invoked under multiple circumstances,
537+
// differentiated by the stored value in `saved`:
538+
// 1. For an outer synthetic `.trigger()`ed event (detected by
539+
// `event.isTrigger & 1` and non-array `saved`), it records arguments
540+
// as an array and fires an [inner] native event to prompt state
541+
// changes that should be observed by registered listeners (such as
542+
// checkbox toggling and focus updating), then clears the stored value.
543+
// 2. For an [inner] native event (detected by `saved` being
544+
// an array), it triggers an inner synthetic event, records the
545+
// result, and preempts propagation to further jQuery listeners.
546+
// 3. For an inner synthetic event (detected by `event.isTrigger & 1` and
547+
// array `saved`), it prevents double-propagation of surrogate events
548+
// but otherwise allows everything to proceed (particularly including
549+
// further listeners).
550+
// Possible `saved` data shapes: `[...], `{ value }`, `false`.
536551
if ( ( event.isTrigger & 1 ) && this[ type ] ) {
537552

538553
// Interrupt processing of the outer synthetic .trigger()ed event
539-
if ( !saved ) {
554+
if ( !saved.length ) {
540555

541556
// Store arguments for use when handling the inner native event
542-
// There will always be at least one argument (an event object), so this array
543-
// will not be confused with a leftover capture object.
557+
// There will always be at least one argument (an event object),
558+
// so this array will not be confused with a leftover capture object.
544559
saved = slice.call( arguments );
545560
dataPriv.set( this, type, saved );
546561

@@ -555,29 +570,35 @@ function leverageNative( el, type, isSetup ) {
555570
event.stopImmediatePropagation();
556571
event.preventDefault();
557572

558-
return result;
573+
// Support: Chrome 86+
574+
// In Chrome, if an element having a focusout handler is
575+
// blurred by clicking outside of it, it invokes the handler
576+
// synchronously. If that handler calls `.remove()` on
577+
// the element, the data is cleared, leaving `result`
578+
// undefined. We need to guard against this.
579+
return result && result.value;
559580
}
560581

561-
// If this is an inner synthetic event for an event with a bubbling surrogate
562-
// (focus or blur), assume that the surrogate already propagated from triggering
563-
// the native event and prevent that from happening again here.
564-
// This technically gets the ordering wrong w.r.t. to `.trigger()` (in which the
565-
// bubbling surrogate propagates *after* the non-bubbling base), but that seems
566-
// less bad than duplication.
582+
// If this is an inner synthetic event for an event with a bubbling
583+
// surrogate (focus or blur), assume that the surrogate already
584+
// propagated from triggering the native event and prevent that
585+
// from happening again here.
567586
} else if ( ( jQuery.event.special[ type ] || {} ).delegateType ) {
568587
event.stopPropagation();
569588
}
570589

571-
// If this is a native event triggered above, everything is now in order
572-
// Fire an inner synthetic event with the original arguments
573-
} else if ( saved ) {
590+
// If this is a native event triggered above, everything is now in order.
591+
// Fire an inner synthetic event with the original arguments.
592+
} else if ( saved.length ) {
574593

575594
// ...and capture the result
576-
dataPriv.set( this, type, jQuery.event.trigger(
577-
saved[ 0 ],
578-
saved.slice( 1 ),
579-
this
580-
) );
595+
dataPriv.set( this, type, {
596+
value: jQuery.event.trigger(
597+
saved[ 0 ],
598+
saved.slice( 1 ),
599+
this
600+
)
601+
} );
581602

582603
// Abort handling of the native event by all jQuery handlers while allowing
583604
// native handlers on the same element to run. On target, this is achieved

test/unit/event.js

+58
Original file line numberDiff line numberDiff line change
@@ -3473,6 +3473,64 @@ QUnit.test( "trigger(focus) fires native & jQuery handlers (gh-5015)", function(
34733473
input.trigger( "focus" );
34743474
} );
34753475

3476+
QUnit.test( "duplicate native blur doesn't crash (gh-5459)", function( assert ) {
3477+
assert.expect( 4 );
3478+
3479+
function patchAddEventListener( elem ) {
3480+
var nativeAddEvent = elem[ 0 ].addEventListener;
3481+
3482+
// Support: Firefox 124+
3483+
// In Firefox, alert displayed just before blurring an element
3484+
// dispatches the native blur event twice which tripped the jQuery
3485+
// logic. We cannot call `alert()` in unit tests; simulate the
3486+
// behavior by overwriting the native `addEventListener` with
3487+
// a version that calls blur handlers twice.
3488+
//
3489+
// Such a simulation allows us to test whether `leverageNative`
3490+
// logic correctly differentiates between data saved by outer/inner
3491+
// handlers, so it's useful even without the Firefox bug.
3492+
elem[ 0 ].addEventListener = function( eventName, handler ) {
3493+
var finalHandler;
3494+
if ( eventName === "blur" ) {
3495+
finalHandler = function wrappedHandler() {
3496+
handler.apply( this, arguments );
3497+
return handler.apply( this, arguments );
3498+
};
3499+
} else {
3500+
finalHandler = handler;
3501+
}
3502+
return nativeAddEvent.call( this, eventName, finalHandler );
3503+
};
3504+
}
3505+
3506+
function runTest( handler, message ) {
3507+
var button = jQuery( "<button></button>" );
3508+
3509+
patchAddEventListener( button );
3510+
button.appendTo( "#qunit-fixture" );
3511+
3512+
if ( handler ) {
3513+
button.on( "blur", handler );
3514+
}
3515+
button.on( "focus", function() {
3516+
button.trigger( "blur" );
3517+
assert.ok( true, "Did not crash (" + message + ")" );
3518+
} );
3519+
button.trigger( "focus" );
3520+
}
3521+
3522+
runTest( undefined, "no prior handler" );
3523+
runTest( function() {
3524+
return true;
3525+
}, "prior handler returning true" );
3526+
runTest( function() {
3527+
return { length: 42 };
3528+
}, "prior handler returning an array-like" );
3529+
runTest( function() {
3530+
return { value: 42 };
3531+
}, "prior handler returning `{ value }`" );
3532+
} );
3533+
34763534
// TODO replace with an adaptation of
34773535
// https://github.com/jquery/jquery/pull/1367/files#diff-a215316abbaabdf71857809e8673ea28R2464
34783536
( function() {

0 commit comments

Comments
 (0)