#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 12 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 12 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