JQuery.event.fix causes unnecessary reflows in IE when handling key events

[scott_h]

When normalising the event object, jQuery adds pageX/pageY properties in IE based on a combination of event.clientX/Y and document.body.scrollLeft/Top.

​More Information

Accessing scrollLeft/Top on the body triggers a full-page reflow. On a large DOM, this makes jQuery's key* event handlers unnecessarily slow in IE7 and IE8, since pageX/Y are useless for key events (and so could be defined without triggering reflows).

Interestingly, I discovered that IE defines clientX/Y on key* events to be the current mouse position, whereas in Safari/Firefox they (along with pageX/Y) are both 0. This inconsistency makes me sure that no one uses jQuery's pageX/Y on key events.

[scott_h]

dynaTrace PurePaths tab showing poor performance on a keydown event

[scott_h]

Here's the fix I propose for this bug:

​http://github.com/shghs/jquery/commit/b59718e2dab80a8a92d7a02a427e60e1da06bd20

I've verified that the test suite passes in Safari 5.0.1 (Mac), Firefox 3.6.8 (Mac), IE7 (WinXP) and IE8 (WinXP) with this change.

[snover]

I like this. Please make a ​pull request for your patch if it is still unresolved in 1.4.3. If it is resolved, please let us know so we can close this ticket.

[snover]

Retarget all enhancements/features to next major version.

[john]

Let's look at this for 1.7.

[john]

Nominating ticket for 1.7 discussion.

[Rick Waldron]

+1, Seems like a bug, should be fixed

[jaubourg]

+1, no brainer

[ajpiano]

+1, Ew, unnecessary reflows in IE is like kicking somone when they're down... fix!

[timmywil]

+1, ouch

[paul.irish]

+1, it'd also be worthwhile to take a hard look at the rest of the event props we are copying over. see if theres any more we can cull

[john]

+1, Let's fix this, absolutely.

[scottgonzalez]

+1

[addyosmani]

+1

[jzaefferer]

+1

[anonymous]

Sorry for deleting the repo linked above. I've issued a pull request with a slightly cleaner fix compared to the original one:

​https://github.com/jquery/jquery/pull/452

but I am skeptical that this is actually a bug. I think this needs to be stress tested on a page with lots of elements.

[timmywil]

@rwaldron: can we add this to event property hooks?

[dmethvin]

Fixed along with #8789 refactor.