Bug Tracker

Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#4849 closed bug (fixed)

stop() and multiple timers/queue collisions?

Reported by: Joust Owned by:
Priority: low Milestone: 1.next
Component: effects Version: 1.4.4
Keywords: Cc:
Blocked by: Blocking:

Description

Hello!

Below is a test case extrapolatet from a more complex case.

After clicking the START button, all squares should dissapear, then the first one should reappear. It will not do it, because of .stop() on other 3 squares (!?).

FadeIn animation of first square will hang.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
	"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> 
<head>
	<title>Timers bug</title>
	<script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1.3.2/jquery.js"></script>
	<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
	<script type="text/javascript">
		$(function(){
			$('#btn').click(function(){
				$('#a').fadeTo('fast',0,function(){
					
					// The below will not work
					$('#a').fadeTo('fast',1);
				});
				$('#b').fadeTo('fast',0,function(){
					$('#b').stop(true,false);
					$('#c').stop(true,false);
					$('#d').stop(true,false);
				});
				$('#c').fadeTo('fast',0);
				$('#d').fadeTo('fast',0);
			});
			
			$('#btn2').click(function(){
				$('#a').queue([]).css({opacity:1});
				$('#b').queue([]).css({opacity:1});
				$('#c').queue([]).css({opacity:1});
				$('#d').queue([]).css({opacity:1});
			})
			
		});
	</script>
</head>
<body>
	<div id="a" style="background:red;width:100px;height:100px"></div>
	<div id="b" style="background:green;width:100px;height:100px"></div>
	<div id="c" style="background:blue;width:100px;height:100px"></div>
	<div id="d" style="background:yellow;width:100px;height:100px"></div>
	<button type="button" id="btn">START</button>
	<button type="button" id="btn2">RESET</button>
</body>


Attachments (1)

bug_4849_example_patch.zip (40.5 KB) - added by gregory80 8 years ago.
patch for use case of incorrect scope

Download all attachments as: .zip

Change History (9)

comment:1 Changed 8 years ago by Joust

It will also work with only two squares, but the behaviour varies between FF, IE and Safari. I couldn't capture it in this example, but I have an app that works with multiple effects and it seems to work quite well under IE+Safari, but is broken under FF. It might be related to the timing - order in which timer-attached functions (i.e. fx.step()) are fired in different browsers when the timer difference is close to 10ms.

I'm sure that when you debug the above, you'll have a fix in no time. I've studied the calls fx makes to/from step(), queue(), dequeue() etc. but I don't see the flaw.

It might be related to the tight timeframe when a timer is popped/pushed from jQuery.timers array. Maybe when fx functions fire simultaneously in 2-3 instances (at the same time) the timers are not popped/pushed in the correct order?

Please take a look and fix it.

comment:2 Changed 8 years ago by gregory80

After reviewing this bug, I believe this error is happening due to an element scope change issue.

In the above example, when the stop() method is executed, dequeue is being called from the stopped element. thus $('#b').get(0) === this. so this.dequeue() happens from the scope of id#b, but it needs to happen for $(#a)

However, in this case (as I imagine in all cases) dequeue needs to be called by the next element in the jQuery.timers queue, which is this scenario is $('#a').

There are several cases which can cause this issue to not surface. For instance, changing the speed of the original fadeTo() will keep this from surfacing: $('#a').fadeTo('normal',0).fadeTo('fast',1);

Also, if $('#b').stop(true,false) is commented out, the scope issue doesn't arise.

I found that by altering the jQuery source code to call the next item in the timers array, this will always work.

on line 4699 of the nightly, changing

<code>if (!gotoEnd)

this.dequeue();</code>

to

<code>if (!gotoEnd)

jQuery(timers.shift()).dequeue();</code>

however, this is far from elegant in my eyes.

Please see my attached example and a patched version of jQuery that contains the above noted alteration of the jQuery core.

Changed 8 years ago by gregory80

Attachment: bug_4849_example_patch.zip added

patch for use case of incorrect scope

comment:3 Changed 6 years ago by snover

Milestone: 1.4
Priority: criticallow
Status: newopen
Version: 1.3.21.4.4

comment:4 Changed 6 years ago by snover

Blocking: 6641 added

comment:5 Changed 6 years ago by timmywil

Milestone: 1.next
Resolution: fixed
Status: openclosed

comment:6 Changed 6 years ago by anonymous

http://jsfiddle.net/timmywil/Qqbs7/1/

If you click "START" multiple times very fast - it'll hang

comment:7 Changed 6 years ago by kuchumovn@…

However, if you remove all the .stop() functions, it won't hang: http://jsfiddle.net/Qqbs7/2/

comment:8 Changed 6 years ago by timmywil

Blocking: 6641 removed

(In #6641) I've looked into this a bit and it does not seem related to toggling. I haven't been able to reproduce this with random clicks (meaning it has not gotten stuck in the middle for me), only when the calls are in direct succession. That leads me to believe it has something to do with the timer on the animation itself, or perhaps the asynchronous call in the mark deferred. Reopening so we can dig in further.

Note: See TracTickets for help on using tickets.