Skip to main content

Bug Tracker

Side navigation

#6942 closed enhancement (fixed)

Opened August 25, 2010 01:51AM UTC

Closed September 27, 2011 01:27AM UTC

Last modified March 09, 2012 12:04AM UTC

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

Reported by: scott_h Owned by: dmethvin
Priority: blocker Milestone: 1.7
Component: event Version: 1.4.2
Keywords: performance,reflow,1.7-discuss Cc:
Blocked by: Blocking:
Description

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.

Attachments (1)
Change History (21)

Changed August 25, 2010 02:07AM UTC by scott_h comment:1

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.

Changed October 15, 2010 04:23AM UTC by snover comment:2

milestone: 1.4.31.next
priority: → low
status: newopen
type: bugenhancement

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.

Changed October 19, 2010 01:07AM UTC by snover comment:3

milestone: 1.4.41.5

Retarget all enhancements/features to next major version.

Changed April 17, 2011 12:34AM UTC by john comment:4

milestone: → 1.next

Let's look at this for 1.7.

Changed May 22, 2011 04:38PM UTC by john comment:5

description: 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. \ \ http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 \ \ 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.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. \ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.

Changed May 22, 2011 07:27PM UTC by john comment:6

keywords: performance reflowperformance,reflow,1.7-discuss

Nominating ticket for 1.7 discussion.

Changed May 22, 2011 10:05PM UTC by rwaldron comment:7

description: 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. \ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.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.\ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.

+1, Seems like a bug, should be fixed

Changed May 23, 2011 12:10AM UTC by jaubourg comment:8

+1, no brainer

Changed May 23, 2011 02:02AM UTC by ajpiano comment:9

description: 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.\ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.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. \ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.

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

Changed May 23, 2011 03:42AM UTC by timmywil comment:10

description: 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. \ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.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.\ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.

+1, ouch

Changed May 23, 2011 05:22PM UTC by paul.irish comment:11

description: 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.\ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.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. \ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.

+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

Changed June 03, 2011 01:37PM UTC by john comment:12

description: 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. \ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.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.\ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.

+1, Let's fix this, absolutely.

Changed June 03, 2011 03:11PM UTC by scottgonzalez comment:13

+1

Changed June 04, 2011 10:18PM UTC by addyosmani comment:14

+1

Changed June 06, 2011 03:52PM UTC by jzaefferer comment:15

+1

Changed July 12, 2011 02:59PM UTC by dmethvin comment:16

description: 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.\ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.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. \ \ [http://github.com/jquery/jquery/blob/cb40495b21bcb7802f3ab6ae0f837f2bf5b385ed/src/event.js#L463 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.
milestone: 1.next1.7

Changed July 12, 2011 03:00PM UTC by dmethvin comment:17

priority: lowblocker

Changed July 25, 2011 04:16PM UTC by dmethvin comment:18

owner: → dmethvin
status: openassigned

Changed August 03, 2011 07:55AM UTC by anonymous comment:19

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.

Changed September 20, 2011 02:28PM UTC by timmywil comment:20

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

Changed September 27, 2011 01:27AM UTC by dmethvin comment:21

resolution: → fixed
status: assignedclosed

Fixed along with #8789 refactor.