Side navigation
#5274 closed bug (fixed)
Opened September 20, 2009 11:45PM UTC
Closed November 11, 2009 02:49PM UTC
Last modified November 11, 2009 02:50PM UTC
Toggle cycles incorrectly with multiple toggle handlers
| Reported by: | mrspeaker | Owned by: | brandon |
|---|---|---|---|
| Priority: | major | Milestone: | 1.4 |
| Component: | event | Version: | 1.3.2 |
| Keywords: | toggle | Cc: | |
| Blocked by: | Blocking: |
Description
The toggle event stores the current cycle position on the DOM element with this.lastToggle. If you attach multiple toggle handlers to the same event then they use the same state variable, so fire in an unexpected order.
I have attached a possible fix for the issue, which uses the function's guid in conjunction with the state variable - so each handler tracks it's own state.
Attachments (2)
Change History (6)
Changed September 21, 2009 01:21AM UTC by comment:1
Changed September 21, 2009 01:38AM UTC by comment:2
Ok. re-uploaded the original patch, as that was correct, and the revised patch was not - I had my tests set up incorrectly. The revised patch suffered the same issue as the current toggle when attached to multiple elements.
Changed September 24, 2009 02:48AM UTC by comment:3
| component: | unfilled → event |
|---|---|
| owner: | → brandon |
Changed October 02, 2009 12:07AM UTC by comment:4
This demo might be more useful to see the issue: Toggle issue demo. If you click on the boxes you'll see the first does not toggle correctly. With the patched version it works as expected.
Changed November 11, 2009 02:49PM UTC by comment:5
| resolution: | → fixed |
|---|---|
| status: | new → closed |
The patch worked, although I had to make some tweaks (namely, using jQuery instead of $ and jQuery.data instead of jQuery().data). I also added some tests to verified that the patch worked (in the future - having tests together with the patch makes it a lot easier to land stuff like this).
Thanks a ton for your patch - it looks like it works great!
Landed here:
http://github.com/jquery/jquery/commit/5cb1163469fb2c9cd80712cd5481b29055821022
Changed November 11, 2009 02:50PM UTC by comment:6
Oh, also, as far as I can tell the fn.guid technique you used is safe. Since the function is going through the proxy handler it'll have a guide by the time that it's used (which was my major concern).
Ah, my original patch used the function guid to help store state (instead of the DOM node: $(this).data( 'lastToggle' + fn.guid ) instead of this.lastToggle ) but it doesn't need to be stored in data - it can just go in the function scope as a closure. Much nicer.