8000 Event: Increase robustness of an inner native event in leverageNative · mgol/jquery@0582060 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0582060

Browse files
mgolmgol
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
1 parent 5880e02 commit 0582060

File tree

2 files changed

+82
-16
lines changed

2 files changed

+82
-16
lines changed

‎src/event.js

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -522,11 +522,12 @@ function leverageNative( el, type, isSetup ) {
522522
if ( ( event.isTrigger & 1 ) && this[ type ] ) {
523523

524524
// Interrupt processing of the outer synthetic .trigger()ed event
525-
if ( !saved ) {
525+
// Detect `saved` of shape `{ value }` and `false`.
526+
if ( !saved.length ) {
526527

527528
// Store arguments for use when handling the inner native event
528-
// There will always be at least one argument (an event object), so this array
529-
// will not be confused with a leftover capture object.
529+
// There will always be at least one argument (an event object),
530+
// so this array will not be confused with a leftover capture object.
530531
saved = slice.call( arguments );
531532
dataPriv.set( this, type, saved );
532533

@@ -541,29 +542,36 @@ function leverageNative( el, type, isSetup ) {
541542
event.stopImmediatePropagation();
542543
event.preventDefault();
543544

544-
return result;
545+
// Support: Chrome 86+
546+
// In Chrome, if an element having a focusout handler is
547+
// blurred by clicking outside of it, it invokes the handler
548+
// synchronously. If that handler calls `.remove()` on
549+
// the element, the data is cleared, leaving `result`
550+
// undefined. We need to guard against this.
551+
return result && result.value;
545552
}
546553

547-
// If this is an inner synthetic event for an event with a bubbling surrogate
548-
// (focus or blur), assume that the surrogate already propagated from triggering
549-
// the native event and prevent that from happening again here.
550-
// This technically gets the ordering wrong w.r.t. to `.trigger()` (in which the
551-
// bubbling surrogate propagates *after* the non-bubbling base), but that seems
552-
// less bad than duplication.
554+
// If this is an inner synthetic event for an event with a bubbling
555+
// surrogate (focus or blur), assume that the surrogate already
556+
// propagated from triggering the native event and prevent that
557+
// from happening again here.
553558
} else if ( ( jQuery.event.special[ type ] || {} ).delegateType ) {
554559
event.stopPropagation();
555560
}
556561

557562
// If this is a native event triggered above, everything is now in order
558563
// Fire an inner synthetic event with the original arguments
559-
} else if ( saved ) {
564+
// Exclude `saved` of shape `{ value }` and `false`.
565+
} else if ( saved.length ) {
560566

561567
// ...and capture the result
562-
dataPriv.set( this, type, jQuery.event.trigger(
563-
saved[ 0 ],
564-
saved.slice( 1 ),
565-
this
566-
) );
568+
dataPriv.set( this, type, {
569+
value: jQuery.event.trigger(
570+
saved[ 0 ],
571+
saved.slice( 1 ),
572+
this
573+
)
574+
} );
567575

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

‎test/unit/event.js

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3502,6 +3502,64 @@ QUnit.test( "trigger(focus) fires native & jQuery handlers (gh-5015)", function(
35023502
input.trigger( "focus" );
35033503
} );
35043504

3505+
QUnit.test( "duplicate native blur doesn't crash (gh-5459)", function( assert ) {
3506+
assert.expect( 4 );
3507+
3508+
function patchAddEventListener( elem ) {
3509+
var nativeAddEvent = elem[ 0 ].addEventListener;
3510+
3511+
// Support: Firefox 124+
3512+
// In Firefox, alert displayed just before blurring an element
3513+
// dispatches the native blur event twice which tripped the jQuery
3514+
// logic. We cannot call `alert()` in unit tests; simulate the
3515+
// behavior by overwriting the native `addEventListener` with
3516+
// a version that calls blur handlers twice.
3517+
//
3518+
// Such a simulation allows us to test whether `leverageNative`
3519+
// logic correctly differentiates between data saved by outer/inner
3520+
// handlers, so it's useful even without the Firefox bug.
3521+
elem[ 0 ].addEventListener = function( eventName, handler ) {
3522+
var finalHandler;
3523+
if ( eventName === "blur" ) {
3524+
finalHandler = function wrappedHandler() {
3525+
handler.apply( this, arguments );
3526+
return handler.apply( this, arguments );
3527+
};
3528+
} else {
3529+
finalHandler = handler;
3530+
}
3531+
return nativeAddEvent.call( this, eventName, finalHandler );
3532+
};
3533+
}
3534+
3535+
function runTest( handler, message ) {
3536+
var button = jQuery( "<button></button>" );
3537+
3538+
patchAddEventListener( button );
3539+
button.appendTo( "#qunit-fixture" );
3540+
3541+
if ( handler ) {
3542+
button.on( "blur", handler );
3543+
}
3544+
button.on( "focus", function() {
3545+
button.trigger( "blur" );
3546+
assert.ok( true, "Did not crash (" + message + ")" );
3547+
} );
3548+
button.trigger( "focus" );
3549+
}
3550+
3551+
runTest( undefined, "no prior handler" );
3552+
runTest( function() {
3553+
return true;
3554+
}, "prior handler returning true" );
3555+
runTest( function() {
3556+
return { length: 42 };
3557+
}, "prior handler returning an array-like" );
3558+
runTest( function() {
3559+
return { value: 42 };
3560+
}, "prior handler returning `{ value }`" );
3561+
} );
3562+
35053563
// TODO replace with an adaptation of
35063564
// https://github.com/jquery/jquery/pull/1367/files#diff-a215316abbaabdf71857809e8673ea28R2464
35073565
( function() {

0 commit comments

Comments
 (0)
0