Bug Tracker

Opened 13 years ago

Closed 13 years ago

Last modified 8 years ago

#1751 closed bug (fixed)

slideToggle() hiding visible elements after animation

Reported by: Deasean Owned by:
Priority: major Milestone: 1.2.2
Component: effects Version: 1.2.1
Keywords: slideToggle Cc:
Blocked by: Blocking:

Description

If slideToggle() is called on a list of elements and there is at least one element in the list that is hidden and at least one that is visible, then after the sliding down animation has finished for the hidden elements, any previously hidden element that was listed after the first previously visible element will become hidden again.

If all elements in the list are hidden, then slideToggle() will work as expected (all elements will remain visible after animating).

So, say you have four elements with IDs A, B, C, and D. A, B, and D are currently hidden, and C is visible. If you call $("#A,#B,#C,#D").slideToggle(), then A and B will be visible, and C and D will be hidden after their animations complete.

However, if all elements were hidden to begin with, then they will all be visible after calling the above statement.

This page probably demonstrates it better:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
	<title>slideToggle Bug</title>
	
	<style type="text/css">#two, #three { display: none; }</style>

	<script type="text/javascript" src="jquery.js"></script>
	<script type="text/javascript">
		function t1() { $("#one,#two,#three").slideToggle(); }
		function t2() { $("#two,#one,#three").slideToggle(); }
		function t3() { $("#three,#two,#one").slideToggle(); }
		function t4() { $("#two,#three").slideToggle(); }
		
		function t5()
		{
			$("#one").slideToggle();
			$("#two").slideToggle();
			$("#three").slideToggle();
		}
	</script>
</head>

<body>

<div id="one">I'm number one!</div>
<div id="two">I'm number two!</div>
<div id="three">I'm number three!</div>

<p>
	Click to execute:<br />
	<a onclick="t1();">$("#one,#two,#three").slideToggle();</a><br />
	<a onclick="t2();">$("#two,#one,#three").slideToggle();</a><br />
	<a onclick="t3();">$("#three,#two,#one").slideToggle();</a><br />
	<a onclick="t4();">$("#two,#three").slideToggle();</a><br />
	<a onclick="t5();">$("#one").slideToggle(); $("#two").slideToggle(); $("#three").slideToggle();</a>
</p>

</body>
</html>

I've tried this in FF 2.0.0.7, IE 7, and Opera 9.23, and the bug appears in all three browsers.

Attachments (2)

1751.diff (643 bytes) - added by davidserduke 13 years ago.
1751.test.diff (871 bytes) - added by davidserduke 13 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 13 years ago by davidserduke

From what I can tell this appears to be a closure problem (nice test case btw) in the function animate(). The variable opt is used in the animate function but really it is the options for all the elements. The way the code works now, it keeps extending the options for each successive element instead of starting over with a fresh set of options. Look in to changing the first few lines of animate() from:

  animate: function( prop, speed, easing, callback ) {
    var opt = jQuery.speed(speed, easing, callback);

    return this[ opt.queue === false ? "each" : "queue" ](function(){
      opt = jQuery.extend({}, opt);

to:

  animate: function( prop, speed, easing, callback ) {
    var optall = jQuery.speed(speed, easing, callback);

    return this[ optall.queue === false ? "each" : "queue" ](function(){
      var opt = jQuery.extend({}, optall);

That appeared to work for the above test case at least.

David

Changed 13 years ago by davidserduke

Attachment: 1751.diff added

comment:2 Changed 13 years ago by davidserduke

I added a unit test that shows the bug fixed. Please note that if it is patched in, the function jQuery.fn.saveState was changed slightly to work for the case when the jQuery object passed in is longer than 1 in length. The other tests that use this function worked fine in my test, but I wanted to point it out just in case.

Changed 13 years ago by davidserduke

Attachment: 1751.test.diff added

comment:3 Changed 13 years ago by john

Component: corefx
Resolution: fixed
Status: newclosed

Fixed in SVN rev [3671].

Note: See TracTickets for help on using tickets.