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.
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 comment:1
Changed October 15, 2010 04:23AM UTC by comment:2
| milestone: | 1.4.3 → 1.next |
|---|---|
| priority: | → low |
| status: | new → open |
| type: | bug → enhancement |
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 comment:3
| milestone: | 1.4.4 → 1.5 |
|---|
Retarget all enhancements/features to next major version.
Changed April 17, 2011 12:34AM UTC by comment:4
| milestone: | → 1.next |
|---|
Let's look at this for 1.7.
Changed May 22, 2011 04:38PM UTC by 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 comment:6
| keywords: | performance reflow → performance,reflow,1.7-discuss |
|---|
Nominating ticket for 1.7 discussion.
Changed May 22, 2011 10:05PM UTC by 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 comment:8
+1, no brainer
Changed May 23, 2011 02:02AM UTC by 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 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 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 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 comment:13
+1
Changed June 04, 2011 10:18PM UTC by comment:14
+1
Changed June 06, 2011 03:52PM UTC by comment:15
+1
Changed July 12, 2011 02:59PM UTC by 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.next → 1.7 |
Changed July 12, 2011 03:00PM UTC by comment:17
| priority: | low → blocker |
|---|
Changed July 25, 2011 04:16PM UTC by comment:18
| owner: | → dmethvin |
|---|---|
| status: | open → assigned |
Changed August 03, 2011 07:55AM UTC by 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 comment:20
@rwaldron: can we add this to event property hooks?
Changed September 27, 2011 01:27AM UTC by comment:21
| resolution: | → fixed |
|---|---|
| status: | assigned → closed |
Fixed along with #8789 refactor.
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.