Bug Tracker

Modify

Ticket #4849 (closed bug: fixed)

Opened 5 years ago

Last modified 3 years ago

stop() and multiple timers/queue collisions?

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

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

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

Change History

comment:1 Changed 5 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 5 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 5 years ago by gregory80

patch for use case of incorrect scope

comment:3 Changed 3 years ago by snover

  • Priority changed from critical to low
  • Status changed from new to open
  • Version changed from 1.3.2 to 1.4.4
  • Milestone 1.4 deleted

comment:4 Changed 3 years ago by snover

  • Blocking 6641 added

comment:5 Changed 3 years ago by timmywil

  • Status changed from open to closed
  • Resolution set to fixed
  • Milestone set to 1.next

comment:6 Changed 3 years ago by anonymous

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

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

comment:7 Changed 3 years ago by kuchumovn@…

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

comment:8 Changed 3 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.

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.