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

Commit ee787ff

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 556eaf4 commit ee787ff

File tree

3 files changed

+88
-21
lines changed

3 files changed

+88
-21
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/data/event/onbeforeunload.html

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,18 @@
44
<script>
55
function report( event ) {
66
var payload = {
7+
source: "jQuery onbeforeunload iframe test",
78
event: event.type
89
};
9-
return parent.postMessage( JSON.stringify(payload), "*" );
10+
return parent.postMessage( JSON.stringify( payload ), "*" );
1011
}
1112

1213
jQuery( window ).on( "beforeunload", function( event ) {
1314
report( event );
14-
}).on( "load", function( event ) {
15-
setTimeout(function() {
15+
} ).on( "load", function( event ) {
16+
setTimeout( function() {
1617
window.location.reload();
17-
}, 50);
18-
});
18+
}, 50 );
19+
} );
1920
</script>
2021
</html>

‎test/unit/event.js

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

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

0 commit comments

Comments
 (0)
0