From: Michał Gołębiowski-Owczarek Date: Mon, 20 May 2024 16:05:19 +0000 (+0200) Subject: Event: Increase robustness of an inner native event in leverageNative X-Git-Tag: 4.0.0-beta.2~15 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=527fb3dcf0dcde69302a741dfc61cbfa58e99eb0;p=jquery.git 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 --- diff --git a/src/event.js b/src/event.js index 0112264a4..a216f9a8c 100644 --- a/src/event.js +++ b/src/event.js @@ -519,14 +519,29 @@ function leverageNative( el, type, isSetup ) { var result, 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 ] ) { // Interrupt processing of the outer synthetic .trigger()ed event - if ( !saved ) { + 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 ); @@ -541,29 +556,35 @@ 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 ) { + // 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.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 diff --git a/test/unit/event.js b/test/unit/event.js index ac309d22d..111b1a18b 100644 --- a/test/unit/event.js +++ b/test/unit/event.js @@ -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( "" ); + + 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() {