From 669f720edc4f557dfef986db747c09ebfaa16ef5 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Wed, 11 Jan 2017 15:19:30 -0700 Subject: [PATCH] Event: Leverage native events for focus/blur/click; propagate additional data Summary of the changes/fixes: 1. Trigger checkbox and radio click events identically (cherry-picked from b442abacbb8464f0165059e8da734e3143d0721f that was reverted before). 2. Manually trigger a native event before checkbox/radio handlers. 3. Add test coverage for triggering namespaced native-backed events. 4. Propagate extra parameters passed when triggering the click event to the handlers. 5. Intercept and preserve namespaced native-backed events. 6. Leverage native events for focus and blur. 7. Accept that focusin handlers may fire more than once for now. Fixes gh-1741 Fixes gh-3423 Fixes gh-3751 Fixes gh-4139 Closes gh-4279 Ref gh-1367 Ref gh-3494 --- src/event.js | 181 ++++++++++++++++--- src/manipulation.js | 6 +- src/serialize.js | 2 +- src/{manipulation => }/var/rcheckableType.js | 0 test/unit/event.js | 163 ++++++++++++++--- 5 files changed, 296 insertions(+), 56 deletions(-) rename src/{manipulation => }/var/rcheckableType.js (100%) diff --git a/src/event.js b/src/event.js index 0bf481b16..cb70e8ebe 100644 --- a/src/event.js +++ b/src/event.js @@ -4,6 +4,7 @@ define( [ "./var/documentElement", "./var/isFunction", "./var/rnothtmlwhite", + "./var/rcheckableType", "./var/slice", "./data/var/dataPriv", "./core/nodeName", @@ -11,7 +12,7 @@ define( [ "./core/init", "./selector" ], function( jQuery, document, documentElement, isFunction, rnothtmlwhite, - slice, dataPriv, nodeName ) { + rcheckableType, slice, dataPriv, nodeName ) { "use strict"; @@ -329,9 +330,10 @@ jQuery.event = { while ( ( handleObj = matched.handlers[ j++ ] ) && !event.isImmediatePropagationStopped() ) { - // Triggered event must either 1) have no namespace, or 2) have namespace(s) - // a subset or equal to those in the bound event (both can have no namespace). - if ( !event.rnamespace || event.rnamespace.test( handleObj.namespace ) ) { + // If the event is namespaced, then each handler is only invoked if it is + // specially universal or its namespaces are a superset of the event's. + if ( !event.rnamespace || handleObj.namespace === false || + event.rnamespace.test( handleObj.namespace ) ) { event.handleObj = handleObj; event.data = handleObj.data; @@ -457,37 +459,101 @@ jQuery.event = { }, focus: { - // Fire native event if possible so blur/focus sequence is correct - trigger: function() { - if ( this !== safeActiveElement() && this.focus ) { - this.focus(); - return false; - } + // Utilize native event if possible so blur/focus sequence is correct + setup: function() { + + // Claim the first handler + // dataPriv.set( this, "focus", ... ) + leverageNative( this, "focus", false, function( el ) { + return el !== safeActiveElement(); + } ); + + // Return false to allow normal processing in the caller + return false; }, + trigger: function() { + + // Force setup before trigger + leverageNative( this, "focus", returnTrue ); + + // Return non-false to allow normal event-path propagation + return true; + }, + delegateType: "focusin" }, blur: { - trigger: function() { - if ( this === safeActiveElement() && this.blur ) { - this.blur(); - return false; - } + + // Utilize native event if possible so blur/focus sequence is correct + setup: function() { + + // Claim the first handler + // dataPriv.set( this, "blur", ... ) + leverageNative( this, "blur", false, function( el ) { + return el === safeActiveElement(); + } ); + + // Return false to allow normal processing in the caller + return false; }, + trigger: function() { + + // Force setup before trigger + leverageNative( this, "blur", returnTrue ); + + // Return non-false to allow normal event-path propagation + return true; + }, + delegateType: "focusout" }, click: { - // For checkbox, fire native event so checked state will be right - trigger: function() { - if ( this.type === "checkbox" && this.click && nodeName( this, "input" ) ) { - this.click(); - return false; + // Utilize native event to ensure correct state for checkable inputs + setup: function( data ) { + + // For mutual compressibility with _default, replace `this` access with a local var. + // `|| data` is dead code meant only to preserve the variable through minification. + var el = this || data; + + // Claim the first handler + if ( rcheckableType.test( el.type ) && + el.click && nodeName( el, "input" ) && + dataPriv.get( el, "click" ) === undefined ) { + + // dataPriv.set( el, "click", ... ) + leverageNative( el, "click", false, returnFalse ); } + + // Return false to allow normal processing in the caller + return false; + }, + trigger: function( data ) { + + // For mutual compressibility with _default, replace `this` access with a local var. + // `|| data` is dead code meant only to preserve the variable through minification. + var el = this || data; + + // Force setup before triggering a click + if ( rcheckableType.test( el.type ) && + el.click && nodeName( el, "input" ) && + dataPriv.get( el, "click" ) === undefined ) { + + leverageNative( el, "click", returnTrue ); + } + + // Return non-false to allow normal event-path propagation + return true; }, - // For cross-browser consistency, don't fire native .click() on links + // For cross-browser consistency, suppress native .click() on links + // Also prevent it if we're currently inside a leveraged native-event stack _default: function( event ) { - return nodeName( event.target, "a" ); + var target = event.target; + return rcheckableType.test( target.type ) && + target.click && nodeName( target, "input" ) && + dataPriv.get( target, "click" ) || + nodeName( target, "a" ); } }, @@ -504,6 +570,77 @@ jQuery.event = { } }; +// Ensure the presence of an event listener that handles manually-triggered +// synthetic events by interrupting progress until reinvoked in response to +// *native* events that it fires directly, ensuring that state changes have +// already occurred before other listeners are invoked. +function leverageNative( el, type, forceAdd, allowAsync ) { + + // Setup must go through jQuery.event.add + if ( forceAdd ) { + jQuery.event.add( el, type, forceAdd ); + return; + } + + // Register the controller as a special universal handler for all event namespaces + dataPriv.set( el, type, forceAdd ); + jQuery.event.add( el, type, { + namespace: false, + handler: function( event ) { + var maybeAsync, result, + saved = dataPriv.get( this, type ); + + // Interrupt processing of the outer synthetic .trigger()ed event + if ( ( event.isTrigger & 1 ) && this[ type ] && !saved ) { + + // Store arguments for use when handling the inner native event + saved = slice.call( arguments ); + dataPriv.set( this, type, saved ); + + // Trigger the native event and capture its result + // Support: IE <=9 - 11+ + // focus() and blur() are asynchronous + maybeAsync = allowAsync( this, type ); + this[ type ](); + result = dataPriv.get( this, type ); + if ( result !== saved ) { + dataPriv.set( this, type, false ); + + // Cancel the outer synthetic event + event.stopImmediatePropagation(); + event.preventDefault(); + return result; + } else if ( maybeAsync ) { + + // Cancel the outer synthetic event in expectation of a followup + event.stopImmediatePropagation(); + event.preventDefault(); + return; + } else { + dataPriv.set( this, type, false ); + } + + // If this is a native event triggered above, everything is now in order + // Fire an inner synthetic event with the original arguments + } else if ( !event.isTrigger && saved ) { + + // ...and capture the result + dataPriv.set( this, type, jQuery.event.trigger( + + // Support: IE <=9 - 11+ + // Extend with the prototype to reset the above stopImmediatePropagation() + jQuery.extend( saved.shift(), jQuery.Event.prototype ), + saved, + this + ) ); + + // Abort handling of the native event + event.stopImmediatePropagation(); + } + } + } ); +} + jQuery.removeEvent = function( elem, type, handle ) { // This "if" is needed for plain objects diff --git a/src/manipulation.js b/src/manipulation.js index a24a5cc0c..7dbc92689 100644 --- a/src/manipulation.js +++ b/src/manipulation.js @@ -4,8 +4,8 @@ define( [ "./var/concat", "./var/isFunction", "./var/push", + "./var/rcheckableType", "./core/access", - "./manipulation/var/rcheckableType", "./manipulation/var/rtagName", "./manipulation/var/rscriptType", "./manipulation/wrapMap", @@ -24,8 +24,8 @@ define( [ "./traversing", "./selector", "./event" -], function( jQuery, isAttached, concat, isFunction, push, access, - rcheckableType, rtagName, rscriptType, +], function( jQuery, isAttached, concat, isFunction, push, rcheckableType, + access, rtagName, rscriptType, wrapMap, getAll, setGlobalEval, buildFragment, support, dataPriv, dataUser, acceptData, DOMEval, nodeName ) { diff --git a/src/serialize.js b/src/serialize.js index 44d3606b3..d8a9a36a4 100644 --- a/src/serialize.js +++ b/src/serialize.js @@ -1,7 +1,7 @@ define( [ "./core", "./core/toType", - "./manipulation/var/rcheckableType", + "./var/rcheckableType", "./var/isFunction", "./core/init", "./traversing", // filter diff --git a/src/manipulation/var/rcheckableType.js b/src/var/rcheckableType.js similarity index 100% rename from src/manipulation/var/rcheckableType.js rename to src/var/rcheckableType.js diff --git a/test/unit/event.js b/test/unit/event.js index e18bf9ed3..27eda6a8c 100644 --- a/test/unit/event.js +++ b/test/unit/event.js @@ -2383,12 +2383,14 @@ QUnit.test( "clone() delegated events (#11076)", function( assert ) { clone.remove(); } ); -QUnit.test( "checkbox state (#3827)", function( assert ) { - assert.expect( 9 ); +QUnit.test( "checkbox state (trac-3827)", function( assert ) { + assert.expect( 16 ); - var markup = jQuery( "
" ).appendTo( "#qunit-fixture" ), + var markup = jQuery( "
" ), cb = markup.find( "input" )[ 0 ]; + markup.appendTo( "#qunit-fixture" ); + jQuery( cb ).on( "click", function() { assert.equal( this.checked, false, "just-clicked checkbox is not checked" ); } ); @@ -2398,18 +2400,31 @@ QUnit.test( "checkbox state (#3827)", function( assert ) { // Native click cb.checked = true; - assert.equal( cb.checked, true, "native - checkbox is initially checked" ); + assert.equal( cb.checked, true, "native event - checkbox is initially checked" ); cb.click(); - assert.equal( cb.checked, false, "native - checkbox is no longer checked" ); + assert.equal( cb.checked, false, "native event - checkbox is no longer checked" ); // jQuery click cb.checked = true; - assert.equal( cb.checked, true, "jQuery - checkbox is initially checked" ); + assert.equal( cb.checked, true, "jQuery event - checkbox is initially checked" ); jQuery( cb ).trigger( "click" ); - assert.equal( cb.checked, false, "jQuery - checkbox is no longer checked" ); + assert.equal( cb.checked, false, "jQuery event - checkbox is no longer checked" ); // Handlers only; checkbox state remains false jQuery( cb ).triggerHandler( "click" ); + assert.equal( cb.checked, false, "handlers only - checkbox is still unchecked" ); + + // Trigger parameters are preserved (trac-13353, gh-4139) + cb.checked = true; + assert.equal( cb.checked, true, "jQuery event with data - checkbox is initially checked" ); + jQuery( cb ).on( "click", function( e, data ) { + assert.equal( data, "clicked", "trigger data passed to handler" ); + } ); + markup.on( "click", function( e, data ) { + assert.equal( data, "clicked", "trigger data passed to bubbled handler" ); + } ); + jQuery( cb ).trigger( "click", [ "clicked" ] ); + assert.equal( cb.checked, false, "jQuery event with data - checkbox is no longer checked" ); } ); QUnit.test( "event object properties on natively-triggered event", function( assert ) { @@ -2876,18 +2891,16 @@ QUnit.test( QUnit.test( "originalEvent type of simulated event", function( assert ) { assert.expect( 2 ); - var done = assert.async(), - outer = jQuery( + var outer = jQuery( "
" + "
" + - "" + + "" + "
" + "
" ).appendTo( "#qunit-fixture" ), input = jQuery( "#donor-input" ), - expectedType = jQuery.support.focusin ? "focusin" : "focus", + done = assert.async(), finish = function() { - finish = null; // Remove jQuery handlers to ensure removal of capturing handlers on the document outer.off( "focusin" ); @@ -2897,23 +2910,15 @@ QUnit.test( "originalEvent type of simulated event", function( assert ) { outer.on( "focusin", function( event ) { assert.equal( event.type, "focusin", "focusin event at ancestor" ); - assert.equal( event.originalEvent.type, expectedType, + assert.equal( event.originalEvent.type, "click", "focus event at ancestor has correct originalEvent type" ); setTimeout( finish ); } ); - input.trigger( "focus" ); - // DOM focus is unreliable in TestSwarm; set a simulated event workaround timeout - setTimeout( function() { - if ( !finish ) { - return; - } - input[ 0 ].addEventListener( "click", function( nativeEvent ) { - expectedType = nativeEvent.type; - jQuery.event.simulate( "focusin", this, jQuery.event.fix( nativeEvent ) ); - } ); - input[ 0 ].click(); - }, QUnit.config.testTimeout / 4 || 1000 ); + input[ 0 ].addEventListener( "click", function( nativeEvent ) { + jQuery.event.simulate( "focusin", this, jQuery.event.fix( nativeEvent ) ); + } ); + input[ 0 ].click(); } ); QUnit.test( "trigger('click') on radio passes extra params", function( assert ) { @@ -2951,7 +2956,11 @@ QUnit.test( "Check order of focusin/focusout events", function( assert ) { .on( "focus", function() { focus = true; } ) - .on( "focusin", function() { + + // PR gh-4279 fixed a lot of `focus`-related issues but made `focusin` fire twice. + // We've decided to accept this drawback for now. If it's fixed, change `one` to `on` + // in the following line: + .one( "focusin", function() { assert.ok( !focus, "Focusin event should fire before focus does" ); focus = true; } ) @@ -2984,11 +2993,11 @@ QUnit.test( "focus-blur order (#12868)", function( assert ) { var order, $text = jQuery( "#text1" ), - $radio = jQuery( "#radio1" ).trigger( "focus" ), + $radio = jQuery( "#radio1" ), - // Support: IE <=10 only - // IE8-10 fire focus/blur events asynchronously; this is the resulting mess. - // IE's browser window must be topmost for this to work properly!! + // Support: IE <=9 - 11+ + // focus and blur events are asynchronous; this is the resulting mess. + // The browser window must be topmost for this to work properly!! done = assert.async(); $radio[ 0 ].focus(); @@ -3035,3 +3044,97 @@ QUnit.test( "focus-blur order (#12868)", function( assert ) { }, 50 ); }, 50 ); } ); + +QUnit.test( "native-backed events preserve trigger data (gh-1741, gh-4139)", function( assert ) { + assert.expect( 17 ); + + var parent = supportjQuery( + "
" + ).appendTo( "#qunit-fixture" ), + targets = jQuery( parent[ 0 ].childNodes ), + checkbox = jQuery( targets[ 0 ] ), + data = [ "arg1", "arg2" ], + slice = data.slice, + + // Support: IE <=9 - 11+ + // focus and blur events are asynchronous; this is the resulting mess. + // The browser window must be topmost for this to work properly!! + done = assert.async(); + + // click (gh-4139) + assert.strictEqual( targets[ 0 ].checked, false, "checkbox unchecked before click" ); + assert.strictEqual( targets[ 1 ].checked, false, "radio unchecked before click" ); + targets.add( parent ).on( "click", function( event ) { + var type = event.target.type, + level = event.currentTarget === parent[ 0 ] ? "parent" : ""; + assert.strictEqual( event.target.checked, true, + type + " toggled before invoking " + level + " handler" ); + assert.deepEqual( slice.call( arguments, 1 ), data, + type + " " + level + " handler received correct data" ); + } ); + targets.trigger( "click", data ); + assert.strictEqual( targets[ 0 ].checked, true, + "checkbox toggled after click (default action)" ); + assert.strictEqual( targets[ 1 ].checked, true, + "radio toggled after event (default action)" ); + + // focus (gh-1741) + assert.notEqual( document.activeElement, checkbox[ 0 ], + "element not focused before focus event" ); + checkbox.on( "focus blur", function( event ) { + var type = event.type; + assert.deepEqual( slice.call( arguments, 1 ), data, + type + " handler received correct data" ); + } ); + checkbox.trigger( "focus", data ); + setTimeout( function() { + assert.strictEqual( document.activeElement, checkbox[ 0 ], + "element focused after focus event (default action)" ); + checkbox.trigger( "blur", data ); + setTimeout( function() { + assert.notEqual( document.activeElement, checkbox[ 0 ], + "element not focused after blur event (default action)" ); + done(); + }, 50 ); + }, 50 ); +} ); + +// TODO replace with an adaptation of +// https://github.com/jquery/jquery/pull/1367/files#diff-a215316abbaabdf71857809e8673ea28R2464 +( function() { + supportjQuery.each( + { + checkbox: "", + radio: "" + }, + makeTestFor3751 + ); + + function makeTestFor3751( type, html ) { + var testName = "native-backed namespaced clicks are handled correctly (gh-3751) - " + type; + QUnit.test( testName, function( assert ) { + assert.expect( 2 ); + + var parent = supportjQuery( "
" + html + "
" ), + target = jQuery( parent[ 0 ].firstChild ); + + parent.appendTo( "#qunit-fixture" ); + + target.add( parent ) + .on( "click.notFired", function( event ) { + assert.ok( false, "namespaced event should not be received" + + " by wrong-namespace listener at " + event.currentTarget.nodeName ); + } ) + .on( "click.fired", function( event ) { + assert.equal( event.target.checked, true, + "toggled before invoking handler at " + event.currentTarget.nodeName ); + } ) + .on( "click", function( event ) { + assert.ok( false, "namespaced event should not be received" + + " by non-namespaced listener at " + event.currentTarget.nodeName ); + } ); + + target.trigger( "click.fired" ); + } ); + } +} )();