CSS: Toggle detached elements as visible unless they have display: none

Fixes gh-2863
Closes gh-3037

diff --git a/src/css/showHide.js b/src/css/showHide.js
index 45f8f06967b26e0bc6a500a66d559260efa9a784..7e9002ca5d453aa0351f585a5ca491783c6a4a12 100644 (file)
--- a/src/css/showHide.js
+++ b/src/css/showHide.js
@@ -54,12 +54,7 @@ function showHide( elements, show ) {
                                        elem.style.display = "";
                                }
                        }
-                       if ( elem.style.display === "" && jQuery.css( elem, "display" ) === "none" &&
-
-                                       // Support: Firefox 43+
-                                       // Don't set inline display on disconnected elements with computed display: none
-                                       jQuery.contains( elem.ownerDocument, elem ) ) {
-
+                       if ( elem.style.display === "" && isHiddenWithinTree( elem ) ) {
                                values[ index ] = getDefaultDisplay( elem );
                        }
                } else {

diff --git a/src/css/var/isHiddenWithinTree.js b/src/css/var/isHiddenWithinTree.js
index 1c81f6480a284f8754e1abe5af75c75acd51d801..c01d228d87f9cbea0396eec907b028e18f1d5ba1 100644 (file)
--- a/src/css/var/isHiddenWithinTree.js
+++ b/src/css/var/isHiddenWithinTree.js
@@ -5,14 +5,29 @@ define( [
        // css is assumed
 ], function( jQuery ) {
 
-       // This function differs from the :hidden selector
-       // in that it intentionally ignores hidden ancestors (gh-2404)
+       // isHiddenWithinTree reports if an element has a non-"none" display style (inline and/or
+       // through the CSS cascade), which is useful in deciding whether or not to make it visible.
+       // It differs from the :hidden selector (jQuery.expr.pseudos.hidden) in two important ways:
+       // * A hidden ancestor does not force an element to be classified as hidden.
+       // * Being disconnected from the document does not force an element to be classified as hidden.
+       // These differences improve the behavior of .toggle() et al. when applied to elements that are
+       // detached or contained within hidden ancestors (gh-2404, gh-2863).
        return function( elem, el ) {
 
                // isHiddenWithinTree might be called from jQuery#filter function;
                // in that case, element will be second argument
                elem = el || elem;
-               return jQuery.css( elem, "display" ) === "none" ||
-                       !jQuery.contains( elem.ownerDocument, elem );
+
+               // Inline style trumps all
+               return elem.style.display === "none" ||
+                       elem.style.display === "" &&
+
+                       // Otherwise, check computed style
+                       // Support: Firefox <=43 - 45
+                       // Disconnected elements can have computed display: none, so first confirm that elem is
+                       // in the document.
+                       jQuery.contains( elem.ownerDocument, elem ) &&
+
+                       jQuery.css( elem, "display" ) === "none";
        };
 } );

diff --git a/test/unit/css.js b/test/unit/css.js
index 26660ab4fbb96dff9f56ab74493a1aaa691a1645..e04ad5dea0b86b5dac6aca345f892758ffd94739 100644 (file)
--- a/test/unit/css.js
+++ b/test/unit/css.js
@@ -926,6 +926,30 @@ QUnit[ jQuery.find.compile && jQuery.fn.toggle ? "test" : "skip" ]( "toggle()",
        jQuery.fn.hide = oldHide;
 } );
 
+QUnit[ jQuery.find.compile && jQuery.fn.toggle ? "test" : "skip" ]( "detached toggle()", function( assert ) {
+       assert.expect( 6 );
+       var detached = jQuery( "<p><a/><p>" ).find( "*" ).addBack(),
+               hiddenDetached = jQuery( "<p><a/></p>" ).find( "*" ).addBack().css( "display", "none" ),
+               cascadeHiddenDetached = jQuery( "<p><a/></p>" ).find( "*" ).addBack().addClass( "hidden" );
+
+       detached.toggle();
+       detached.appendTo( "#qunit-fixture" );
+       assert.equal( detached[ 0 ].style.display, "none", "detached element" );
+       assert.equal( detached[ 1 ].style.display, "none", "element in detached tree" );
+
+       hiddenDetached.toggle();
+       hiddenDetached.appendTo( "#qunit-fixture" );
+       assert.equal( hiddenDetached[ 0 ].style.display, "", "detached, hidden element" );
+       assert.equal( hiddenDetached[ 1 ].style.display, "", "hidden element in detached tree" );
+
+       cascadeHiddenDetached.toggle();
+       cascadeHiddenDetached.appendTo( "#qunit-fixture" );
+       assert.equal( cascadeHiddenDetached[ 0 ].style.display, "none",
+               "detached, cascade-hidden element" );
+       assert.equal( cascadeHiddenDetached[ 1 ].style.display, "none",
+               "cascade-hidden element in detached tree" );
+} );
+
 QUnit.test( "jQuery.css(elem, 'height') doesn't clear radio buttons (bug #1095)", function( assert ) {
        assert.expect( 4 );
 

diff --git a/test/unit/effects.js b/test/unit/effects.js
index 0f5eaa415a34ad3460e543405f6158facd476670..5f1913575b3d01526ad1512788b57beaab53ef64 100644 (file)
--- a/test/unit/effects.js
+++ b/test/unit/effects.js
@@ -1547,15 +1547,17 @@ QUnit.test( "animate should set display for disconnected nodes", function( asser
        assert.expect( 20 );
 
        var env = this,
-               methods = {
-                       toggle: [ 1 ],
-                       slideToggle: [],
+               showMethods = {
                        fadeIn: [],
                        fadeTo: [ "fast", 0.5 ],
                        slideDown: [ "fast" ],
                        show: [ 1 ],
                        animate: [ { width: "show" } ]
                },
+               toggleMethods = {
+                       toggle: [ 1 ],
+                       slideToggle: []
+               },
                $divEmpty = jQuery( "<div/>" ),
                $divTest = jQuery( "<div>test</div>" ),
                $divNone = jQuery( "<div style='display: none;'/>" ),
@@ -1578,7 +1580,7 @@ QUnit.test( "animate should set display for disconnected nodes", function( asser
 
        assert.expectJqData( env, $divNone[ 0 ], "olddisplay" );
 
-       jQuery.each( methods, function( name, opt ) {
+       jQuery.each( showMethods, function( name, opt ) {
                jQuery.fn[ name ].apply( jQuery( "<div/>" ), opt.concat( [ function() {
                        assert.strictEqual( jQuery( this ).css( "display" ), nullParentDisplay,
                                "." + name + " block with null parentNode" );
@@ -1589,6 +1591,17 @@ QUnit.test( "animate should set display for disconnected nodes", function( asser
                                "." + name + " block under fragment" );
                } ] ) );
        } );
+       jQuery.each( toggleMethods, function( name, opt ) {
+               jQuery.fn[ name ].apply( jQuery( "<div/>" ), opt.concat( [ function() {
+                       assert.strictEqual( jQuery( this ).css( "display" ), "none",
+                               "." + name + " block with null parentNode" );
+               } ] ) );
+
+               jQuery.fn[ name ].apply( jQuery( "<div>test</div>" ), opt.concat( [ function() {
+                       assert.strictEqual( jQuery( this ).css( "display" ), "none",
+                               "." + name + " block under fragment" );
+               } ] ) );
+       } );
        clock.tick( 400 );
 } );