Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

For .show() with no arguments, only set display of elements in the se…

…cond loop if they don't have style.display already set or if style.display isn't none. Fixes #7315.
  • Loading branch information...
commit 6ab402dced3339d24ad007ecf3a6c3f5af3e7610 1 parent 7066bb3
@kswedberg kswedberg authored jeresig committed
Showing with 30 additions and 6 deletions.
  1. +16 −5 src/effects.js
  2. +14 −1 test/unit/effects.js
View
21 src/effects.js
@@ -15,28 +15,39 @@ var elemdisplay = {},
jQuery.fn.extend({
show: function( speed, easing, callback ) {
+ var elem, display;
+
if ( speed || speed === 0 ) {
return this.animate( genFx("show", 3), speed, easing, callback);
+
} else {
for ( var i = 0, j = this.length; i < j; i++ ) {
+ elem = this[i];
+ display = elem.style.display;
+
// Reset the inline display of this element to learn if it is
// being hidden by cascaded rules or not
- if ( !jQuery.data(this[i], "olddisplay") && this[i].style.display === "none" ) {
- this[i].style.display = "";
+ if ( !jQuery.data(elem, "olddisplay") && display === "none" ) {
+ elem.style.display = "";
}
// Set elements which have been overridden with display: none
// in a stylesheet to whatever the default browser style is
// for such an element
- if ( this[i].style.display === "" && jQuery.css( this[i], "display" ) === "none" ) {
- jQuery.data(this[i], "olddisplay", defaultDisplay(this[i].nodeName));
+ if ( display === "" && jQuery.css( elem, "display" ) === "none" ) {
+ jQuery.data(elem, "olddisplay", defaultDisplay(elem.nodeName));
}
}
// Set the display of most of the elements in a second loop
// to avoid the constant reflow
for ( i = 0; i < j; i++ ) {
- this[i].style.display = jQuery.data(this[i], "olddisplay") || "";
+ elem = this[i];
+ display = elem.style.display;
+
+ if ( display === "" || display === "none" ) {
+ elem.style.display = jQuery.data(elem, "olddisplay") || "";
+ }
}
return this;
View
15 test/unit/effects.js
@@ -6,7 +6,20 @@ test("sanity check", function() {
});
test("show()", function() {
- expect(23);
+ expect(26);
+
+ var hiddendiv = jQuery("div.hidden");
+
+ equal(jQuery.css( hiddendiv[0], "display"), "none", "hiddendiv is display: none");
+
+ hiddendiv.css("display", "block");
+ equal(jQuery.css( hiddendiv[0], "display"), "block", "hiddendiv is display: block");
+
+ hiddendiv.show();
+ equal(jQuery.css( hiddendiv[0], "display"), "block", "hiddendiv is display: block");
+
+ hiddendiv.css("display","");
+
var pass = true, div = jQuery("#main div");
div.show().each(function(){
if ( this.style.display == "none" ) pass = false;

4 comments on commit 6ab402d

@Enideo

I've spotted a bug, I guess it's from this commit.
Tried to make the simplest scenario here: http://jsfiddle.net/JYFuV/2/
Using Firefox 3.6.11

@kswedberg

Yeah, that's really strange. I see the problem in your linked jsfiddle page, but I can't reproduce it locally in the test suite or in a standalone page with a simple test case: http://test.learningjquery.com/hide-show/

Is my test case doing something obviously different from yours that I'm not seeing?

@jeresig

Karl: I was able to reproduce it in the test suite and fixed it as well, just now:
http://bugs.jquery.com/ticket/7331

@kswedberg

Ah. Cool deal. Thanks, John!

Please sign in to comment.
Something went wrong with that request. Please try again.