Event: Remove fixHooks, propHooks; switch to ES5 getter with addProp

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

diff --git a/src/event.js b/src/event.js
index 4015f87688a7e6984c14902baad149d5728ed8ab..ba2a9dca6d8e530389c60364fa15f6de21921f3f 100644 (file)
--- a/src/event.js
+++ b/src/event.js
@@ -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: {
@@ -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;
@@ -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

diff --git a/test/unit/event.js b/test/unit/event.js
index 5b00c7c3bbc4b98b6187ea4be00aaa8699025ac1..2362147305511c8c1d83ddc71aab4b168e4ce13a 100644 (file)
--- a/test/unit/event.js
+++ b/test/unit/event.js
@@ -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 ) {