Skip to main content

Bug Tracker

Side navigation

#14147 closed bug (migrated)

Opened July 18, 2013 10:52AM UTC

Closed October 21, 2014 12:15AM UTC

jQuery.event.fix() may force layout

Reported by: anonymous Owned by:
Priority: low Milestone: 1.next/2.next
Component: event Version: 1.10.2
Keywords: Cc: paul.irish
Blocked by: Blocking:
Description

Accessing some of the event properties while copying them (like jQuery.event.fix() does in event[ prop ] = originalEvent[ prop ] at https://github.com/jquery/jquery/blob/master/src/event.js#L511) may force style recalculation and layout.

Screenshots of Timeline in Chrome Developer Tools:

http://i.imgur.com/Zuh6WDE.png

http://i.imgur.com/JWLYF2N.png

Attachments (0)
Change History (11)

Changed July 22, 2013 03:57PM UTC by dmethvin comment:1

I could certainly believe this happens for events like mousemove where we populate and normalize the x/y position in our Event object. Browsers may defer doing that work until something asks for the position, and in this case that something is jQuery. Not sure it can be avoided though if we want this information to be in our object.

Adding paul.irish for his insight.

Changed July 22, 2013 09:18PM UTC by dmethvin comment:2

blocking: → 14164

Changed July 23, 2013 12:32AM UTC by timmywil comment:3

cc: → paul.irish
component: unfiledevent
priority: undecidedlow
status: newopen

Changed September 16, 2013 08:01PM UTC by dmethvin comment:4

blocking: 14164

(In #14164) It's blocked by an open ticket, but it is probably better to handle the others on a case-by-case basis rather than a meta-ticket.

Changed October 22, 2013 05:01AM UTC by jbedard comment:5

I recently ran across this issue as well, which lead me here.

How about using ES5 getters to delay the copying/calculating of event properties? Would that be an option for 2.0?

Something similar to what was previously discussed on the forum and more recently the original post author blogged about a solution.

Changed November 14, 2013 05:18PM UTC by dmethvin comment:6

The problem is that ES5 getters are (still) slow in just about every browser. There are also some bugs and quirks, even in our list of 2.x-supported browsers. https://bugzilla.mozilla.org/show_bug.cgi?id=626021

It's true that if you never get any event properties, or perhaps only get one property one time, it can be faster. But in the case of a mousemove event for example, I suspect it's going to be extremely common that the caller's own event handler will want that information and would have forced the layout anyway.

Additionally, the layout is forced here only if you enter with a "dirty" layout from some other DOM change caused by a previous event or handler.

Changed December 21, 2013 09:34AM UTC by jbedard comment:7

ES5 getters are definitely slow still, but after trying this out I think this might still be worth it.

Don't you think most event handlers only access a few event properties? Yet the $.event.fix loop accesses them all. The fix loop is often a bottleneck on its own without a forced layout.

The scenario I've seen this bug occur a few times is toggling a class on mouse enter/leave. So this bug forces a layout, then the event listener simply toggles a class based on the event.type which sets the dirty flag again. In this case that extra layout is completely unnecessary, even the $.event.fix loop copying everything was unnecessary.

The only problem would be the ES5 getters. But at least in FF/chrome when only calling those getters 0-10 times it seems worth it.

Would you be interested in a pull request to at least see it in action?

Changed December 21, 2013 03:15PM UTC by dmethvin comment:8

I think this could be examined as part of #12031 which I wanted to tackle for 1.12/2.2. There are some issues to be decided there, but potentially you could just use the native event object for 2.x and skip "fixing" entirely.

Changed December 22, 2013 02:06AM UTC by jbedard comment:9

If #12031 provides a new event interface which skips $.event.fix that would be great, especially if it still provides things such as event namespaces & selectors.

But if anyone cares to look at what I tried: https://github.com/jbedard/jquery/commit/fd617744638ae16299c10cbcdb2935e3f45c5f27

This turns all hooks into es5 getters instead of filters and prop copying. I think the es5 setter is actually slower then just calling the getter over and over (needs testing). This avoids the forced layout and removes the $.event.fix loop, but does make accessing the properties more expensive due to the getter. In the cases I tried this was well worth it but I assume the change is too big/risky for such a stable API...

Changed March 16, 2014 05:08PM UTC by dmethvin comment:10

milestone: None1.next/2.next

Changed October 21, 2014 12:15AM UTC by m_gol comment:11

resolution: → migrated
status: openclosed