Skip to content

Commit

Permalink
Make sure that hide or show don't fail when operating on non-Element …
Browse files Browse the repository at this point in the history
…nodes. Fixes #6135.
  • Loading branch information
jeresig committed Apr 17, 2011
1 parent 728a70c commit 21c0be8
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 15 deletions.
38 changes: 23 additions & 15 deletions src/effects.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -28,30 +28,36 @@ jQuery.fn.extend({
} else { } else {
for ( var i = 0, j = this.length; i < j; i++ ) { for ( var i = 0, j = this.length; i < j; i++ ) {
elem = this[i]; elem = this[i];
display = elem.style.display;


// Reset the inline display of this element to learn if it is if ( elem.style ) {
// being hidden by cascaded rules or not display = elem.style.display;
if ( !jQuery._data(elem, "olddisplay") && display === "none" ) {
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(elem, "olddisplay") && display === "none" ) {
display = elem.style.display = "";
}


// Set elements which have been overridden with display: none // Set elements which have been overridden with display: none
// in a stylesheet to whatever the default browser style is // in a stylesheet to whatever the default browser style is
// for such an element // for such an element
if ( display === "" && jQuery.css( elem, "display" ) === "none" ) { if ( display === "" && jQuery.css( elem, "display" ) === "none" ) {
jQuery._data(elem, "olddisplay", defaultDisplay(elem.nodeName)); jQuery._data(elem, "olddisplay", defaultDisplay(elem.nodeName));
}
} }
} }


// Set the display of most of the elements in a second loop // Set the display of most of the elements in a second loop
// to avoid the constant reflow // to avoid the constant reflow
for ( i = 0; i < j; i++ ) { for ( i = 0; i < j; i++ ) {
elem = this[i]; elem = this[i];
display = elem.style.display;


if ( display === "" || display === "none" ) { if ( elem.style ) {
elem.style.display = jQuery._data(elem, "olddisplay") || ""; display = elem.style.display;

if ( display === "" || display === "none" ) {
elem.style.display = jQuery._data(elem, "olddisplay") || "";
}
} }
} }


Expand All @@ -75,7 +81,9 @@ jQuery.fn.extend({
// Set the display of the elements in a second loop // Set the display of the elements in a second loop
// to avoid the constant reflow // to avoid the constant reflow
for ( i = 0; i < j; i++ ) { for ( i = 0; i < j; i++ ) {
this[i].style.display = "none"; if ( this[i].style ) {
this[i].style.display = "none";
}
} }


return this; return this;
Expand Down
4 changes: 4 additions & 0 deletions test/unit/effects.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ test("show()", function() {
var elem = jQuery(selector, "#show-tests").show(); var elem = jQuery(selector, "#show-tests").show();
equals( elem.css("display"), expected, "Show using correct display type for " + selector ); equals( elem.css("display"), expected, "Show using correct display type for " + selector );
}); });

// Make sure that showing or hiding a text node doesn't cause an error
jQuery("<div>test</div> text <span>test</span>").show().remove();
jQuery("<div>test</div> text <span>test</span>").hide().remove();
}); });


test("show(Number) - other displays", function() { test("show(Number) - other displays", function() {
Expand Down

0 comments on commit 21c0be8

Please sign in to comment.