Skip to content

Commit

Permalink
Event: Remove fixHooks, propHooks; switch to ES5 getter with addProp
Browse files Browse the repository at this point in the history
Fixes gh-3103
Fixes gh-1746
Closes gh-2860

- Removes the copy loop in jQuery.event.fix
- Avoids accessing properties such as client/offset/page/screen X/Y
  which may cause style recalc or layouts
- Simplifies adding property hooks to event object
  • Loading branch information
jbedard authored and dmethvin committed May 4, 2016
1 parent 7f2ebd2 commit e61fccb
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 96 deletions.
196 changes: 115 additions & 81 deletions src/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,90 +398,38 @@ jQuery.event = {
return handlerQueue;
},

// Includes some event props shared by KeyEvent and MouseEvent
props: ( "altKey bubbles cancelable ctrlKey currentTarget detail eventPhase " +
"metaKey relatedTarget shiftKey target timeStamp view which" ).split( " " ),

fixHooks: {},

keyHooks: {
props: "char charCode key keyCode".split( " " ),
filter: function( event, original ) {

// Add which for key events
if ( event.which == null ) {
event.which = original.charCode != null ? original.charCode : original.keyCode;
addProp: function( name, hook ) {
Object.defineProperty( jQuery.Event.prototype, name, {
enumerable: true,
configurable: true,

get: jQuery.isFunction( hook ) ?
function() {
if ( this.originalEvent ) {
return hook( this.originalEvent );
}
} :
function() {
if ( this.originalEvent ) {
return this.originalEvent[ name ];
}
},

set: function( value ) {
Object.defineProperty( this, name, {
enumerable: true,
configurable: true,
writable: true,
value: value
} );
}

return event;
}
} );
},

mouseHooks: {
props: ( "button buttons clientX clientY offsetX offsetY pageX pageY " +
"screenX screenY toElement" ).split( " " ),
filter: function( event, original ) {
var eventDoc, doc, body,
button = original.button;

// Calculate pageX/Y if missing and clientX/Y available
if ( event.pageX == null && original.clientX != null ) {
eventDoc = event.target.ownerDocument || document;
doc = eventDoc.documentElement;
body = eventDoc.body;

event.pageX = original.clientX +
( doc && doc.scrollLeft || body && body.scrollLeft || 0 ) -
( doc && doc.clientLeft || body && body.clientLeft || 0 );
event.pageY = original.clientY +
( doc && doc.scrollTop || body && body.scrollTop || 0 ) -
( doc && doc.clientTop || body && body.clientTop || 0 );
}

// Add which for click: 1 === left; 2 === middle; 3 === right
// Note: button is not normalized, so don't use it
if ( !event.which && button !== undefined ) {
event.which = ( button & 1 ? 1 : ( button & 2 ? 3 : ( button & 4 ? 2 : 0 ) ) );
}

return event;
}
},

fix: function( event ) {
if ( event[ jQuery.expando ] ) {
return event;
}

// Create a writable copy of the event object and normalize some properties
var i, prop, copy,
type = event.type,
originalEvent = event,
fixHook = this.fixHooks[ type ];

if ( !fixHook ) {
this.fixHooks[ type ] = fixHook =
rmouseEvent.test( type ) ? this.mouseHooks :
rkeyEvent.test( type ) ? this.keyHooks :
{};
}
copy = fixHook.props ? this.props.concat( fixHook.props ) : this.props;

event = new jQuery.Event( originalEvent );

i = copy.length;
while ( i-- ) {
prop = copy[ i ];
event[ prop ] = originalEvent[ prop ];
}

// Support: Safari <=6 - 7 only
// Target should not be a text node (#504, #13143)
if ( event.target.nodeType === 3 ) {
event.target = event.target.parentNode;
}

return fixHook.filter ? fixHook.filter( event, originalEvent ) : event;
fix: function( originalEvent ) {
return originalEvent[ jQuery.expando ] ?
originalEvent :
new jQuery.Event( originalEvent );
},

special: {
Expand Down Expand Up @@ -569,6 +517,16 @@ jQuery.Event = function( src, props ) {
returnTrue :
returnFalse;

// Create target properties
// Support: Safari <=6 - 7 only
// Target should not be a text node (#504, #13143)
this.target = ( src.target.nodeType === 3 ) ?
src.target.parentNode :
src.target;

this.currentTarget = src.currentTarget;
this.relatedTarget = src.relatedTarget;

// Event type
} else {
this.type = src;
Expand Down Expand Up @@ -625,6 +583,82 @@ jQuery.Event.prototype = {
}
};

// Includes all common event props including KeyEvent and MouseEvent specific props
jQuery.each( {
altKey: true,
bubbles: true,
cancelable: true,
ctrlKey: true,
detail: true,
eventPhase: true,
metaKey: true,
shiftKey: true,
view: true,
"char": true,
charCode: true,
key: true,
keyCode: true,
button: true,
buttons: true,
clientX: true,
clientY: true,
offsetX: true,
offsetY: true,
screenX: true,
screenY: true,
toElement: true,

which: function( event ) {
var button = event.button;

// Add which for key events
if ( event.which == null && rkeyEvent.test( event.type ) ) {
return event.charCode != null ? event.charCode : event.keyCode;
}

// Add which for click: 1 === left; 2 === middle; 3 === right
if ( !event.which && button !== undefined && rmouseEvent.test( event.type ) ) {
return ( button & 1 ? 1 : ( button & 2 ? 3 : ( button & 4 ? 2 : 0 ) ) );
}

return event.which;
},

pageX: function( event ) {
var eventDoc, doc, body;

// Calculate pageX if missing and clientX available
if ( event.pageX == null && event.clientX != null ) {
eventDoc = event.target.ownerDocument || document;
doc = eventDoc.documentElement;
body = eventDoc.body;

return event.clientX +
( doc && doc.scrollLeft || body && body.scrollLeft || 0 ) -
( doc && doc.clientLeft || body && body.clientLeft || 0 );
}

return event.pageX;
},

pageY: function( event ) {
var eventDoc, doc, body;

// Calculate pageY if missing and clientY available
if ( event.pageY == null && event.clientY != null ) {
eventDoc = event.target.ownerDocument || document;
doc = eventDoc.documentElement;
body = eventDoc.body;

return event.clientY +
( doc && doc.scrollTop || body && body.scrollTop || 0 ) -
( doc && doc.clientTop || body && body.clientTop || 0 );
}

return event.pageY;
}
}, jQuery.event.addProp );

// Create mouseenter/leave events using mouseover/out and event-time checks
// so that event delegation works in jQuery.
// Do the same for pointerenter/pointerleave and pointerover/pointerout
Expand Down
22 changes: 7 additions & 15 deletions test/unit/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -2410,36 +2410,28 @@ QUnit.test( "event object properties on natively-triggered event", function( ass
$link.off( "click" ).remove();
} );

QUnit.test( "fixHooks extensions", function( assert ) {
QUnit.test( "addProp extensions", function( assert ) {
assert.expect( 2 );

// IE requires focusable elements to be visible, so append to body
var $fixture = jQuery( "<input type='text' id='hook-fixture' />" ).appendTo( "body" ),
saved = jQuery.event.fixHooks.click;
var $fixture = jQuery( "<div>" ).appendTo( "#qunit-fixture" );

// Ensure the property doesn't exist
$fixture.on( "click", function( event ) {
assert.ok( !( "blurrinessLevel" in event ), "event.blurrinessLevel does not exist" );
assert.ok( !( "testProperty" in event ), "event.testProperty does not exist" );
} );
fireNative( $fixture[ 0 ], "click" );
$fixture.off( "click" );

jQuery.event.fixHooks.click = {
filter: function( event ) {
event.blurrinessLevel = 42;
return event;
}
};
jQuery.event.addProp( "testProperty", function() { return 42; } );

// Trigger a native click and ensure the property is set
$fixture.on( "click", function( event ) {
assert.equal( event.blurrinessLevel, 42, "event.blurrinessLevel was set" );
assert.equal( event.testProperty, 42, "event.testProperty getter was invoked" );
} );
fireNative( $fixture[ 0 ], "click" );
$fixture.off( "click" );

delete jQuery.event.fixHooks.click;
$fixture.off( "click" ).remove();
jQuery.event.fixHooks.click = saved;
$fixture.remove();
} );

QUnit.test( "drag/drop events copy mouse-related event properties (gh-1925, gh-2009)", function( assert ) {
Expand Down

3 comments on commit e61fccb

@mgol
Copy link
Member

@mgol mgol commented on e61fccb May 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbedard This commit is not counted as yours as the email is not associated. :) The earlier ones are taken into account so you might want to add this email to your account (AFAIK it doesn't have to be displayed anywhere).

@dmethvin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good catch! Once @jbedard accepts the org invite I'll assign the completed ticket to him as well.

@jbedard
Copy link
Contributor Author

@jbedard jbedard commented on e61fccb May 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out!

Please sign in to comment.