#6942 closed enhancement (fixed)
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 (last modified by )
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 (22)
Changed 13 years ago by
Attachment: | jQuery.event.fix-dynaTrace.png added |
---|
comment:1 Changed 13 years ago by
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.
comment:2 Changed 13 years ago by
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.
comment:3 Changed 13 years ago by
Milestone: | 1.4.4 → 1.5 |
---|
Retarget all enhancements/features to next major version.
comment:5 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:7 Changed 12 years ago by
Description: | modified (diff) |
---|
+1, Seems like a bug, should be fixed
comment:9 Changed 12 years ago by
Description: | modified (diff) |
---|
+1, Ew, unnecessary reflows in IE is like kicking somone when they're down... fix!
comment:11 Changed 12 years ago by
Description: | modified (diff) |
---|
+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
comment:16 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Milestone: | 1.next → 1.7 |
comment:17 Changed 12 years ago by
Priority: | low → blocker |
---|
comment:18 Changed 12 years ago by
Owner: | set to dmethvin |
---|---|
Status: | open → assigned |
comment:19 Changed 12 years ago by
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.
comment:21 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fixed along with #8789 refactor.
dynaTrace PurePaths tab showing poor performance on a keydown event