From 755e7ccf018eb150eddefe78063a9ec58b3229e3 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Mon, 4 Apr 2016 09:58:14 -0400 Subject: [PATCH] CSS: Toggle detached elements as visible unless they have display: none Fixes gh-2863 Closes gh-3037 --- src/css/showHide.js | 7 +------ src/css/var/isHiddenWithinTree.js | 23 +++++++++++++++++++---- test/unit/css.js | 24 ++++++++++++++++++++++++ test/unit/effects.js | 21 +++++++++++++++++---- 4 files changed, 61 insertions(+), 14 deletions(-) diff --git a/src/css/showHide.js b/src/css/showHide.js index 45f8f0696..7e9002ca5 100644 --- 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 1c81f6480..c01d228d8 100644 --- 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 26660ab4f..e04ad5dea 100644 --- 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( "

" ).find( "*" ).addBack(), + hiddenDetached = jQuery( "

" ).find( "*" ).addBack().css( "display", "none" ), + cascadeHiddenDetached = jQuery( "

" ).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 0f5eaa415..5f1913575 100644 --- 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( "
" ), $divTest = jQuery( "
test
" ), $divNone = jQuery( "
" ), @@ -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( "
" ), 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( "
" ), opt.concat( [ function() { + assert.strictEqual( jQuery( this ).css( "display" ), "none", + "." + name + " block with null parentNode" ); + } ] ) ); + + jQuery.fn[ name ].apply( jQuery( "
test
" ), opt.concat( [ function() { + assert.strictEqual( jQuery( this ).css( "display" ), "none", + "." + name + " block under fragment" ); + } ] ) ); + } ); clock.tick( 400 ); } ); -- 2.39.5