From ba51712e74f612ad5d6a10993251b4726c273edf Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Thu, 27 Jun 2019 16:37:12 +0100 Subject: [PATCH] makeCollapsible: Avoid Sizzle selectors Change-Id: I1a36e66fd18249b064ebb66fa558743c162e0ec8 --- .../src/jquery/jquery.makeCollapsible.js | 4 +- .../jquery/jquery.makeCollapsible.test.js | 90 +++++++++---------- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/resources/src/jquery/jquery.makeCollapsible.js b/resources/src/jquery/jquery.makeCollapsible.js index 3e4081ad1e..b9c13f03fd 100644 --- a/resources/src/jquery/jquery.makeCollapsible.js +++ b/resources/src/jquery/jquery.makeCollapsible.js @@ -270,7 +270,7 @@ } } else { // The toggle-link will be in one of the cells (td or th) of the first row - $firstItem = $collapsible.find( 'tr:first th, tr:first td' ); + $firstItem = $collapsible.find( 'tr' ).first().find( 'th, td' ); $toggle = $firstItem.find( '> .mw-collapsible-toggle' ); // If theres no toggle link, add it to the last cell @@ -288,7 +288,7 @@ $collapsible.before( $toggle ); } else if ( $collapsible.is( 'ul' ) || $collapsible.is( 'ol' ) ) { // The toggle-link will be in the first list-item - $firstItem = $collapsible.find( 'li:first' ); + $firstItem = $collapsible.find( 'li' ).first(); $toggle = $firstItem.find( '> .mw-collapsible-toggle' ); // If theres no toggle link, add it diff --git a/tests/qunit/suites/resources/jquery/jquery.makeCollapsible.test.js b/tests/qunit/suites/resources/jquery/jquery.makeCollapsible.test.js index 0b3e809eaa..e67af10ab5 100644 --- a/tests/qunit/suites/resources/jquery/jquery.makeCollapsible.test.js +++ b/tests/qunit/suites/resources/jquery/jquery.makeCollapsible.test.js @@ -22,17 +22,17 @@ // On collapse... $collapsible.on( 'beforeCollapse.mw-collapsible', function () { - assert.assertTrue( $content.is( ':visible' ), 'first beforeCollapseExpand: content is visible' ); + assert.assertTrue( $content.css( 'display' ) !== 'none', 'first beforeCollapseExpand: content is visible' ); } ); $collapsible.on( 'afterCollapse.mw-collapsible', function () { - assert.assertTrue( $content.is( ':hidden' ), 'first afterCollapseExpand: content is hidden' ); + assert.assertTrue( $content.css( 'display' ) === 'none', 'first afterCollapseExpand: content is hidden' ); // On expand... $collapsible.on( 'beforeExpand.mw-collapsible', function () { - assert.assertTrue( $content.is( ':hidden' ), 'second beforeCollapseExpand: content is hidden' ); + assert.assertTrue( $content.css( 'display' ) === 'none', 'second beforeCollapseExpand: content is hidden' ); } ); $collapsible.on( 'afterExpand.mw-collapsible', function () { - assert.assertTrue( $content.is( ':visible' ), 'second afterCollapseExpand: content is visible' ); + assert.assertTrue( $content.css( 'display' ) !== 'none', 'second afterCollapseExpand: content is visible' ); } ); // ...expanding happens here @@ -53,13 +53,13 @@ assert.strictEqual( $content.length, 1, 'content is present' ); assert.strictEqual( $content.find( $toggle ).length, 0, 'toggle is not a descendant of content' ); - assert.assertTrue( $content.is( ':visible' ), 'content is visible' ); + assert.assertTrue( $content.css( 'display' ) !== 'none', 'content is visible' ); $collapsible.on( 'afterCollapse.mw-collapsible', function () { - assert.assertTrue( $content.is( ':hidden' ), 'after collapsing: content is hidden' ); + assert.assertTrue( $content.css( 'display' ) === 'none', 'after collapsing: content is hidden' ); $collapsible.on( 'afterExpand.mw-collapsible', function () { - assert.assertTrue( $content.is( ':visible' ), 'after expanding: content is visible' ); + assert.assertTrue( $content.css( 'display' ) !== 'none', 'after expanding: content is visible' ); } ); $toggle.trigger( 'click' ); @@ -76,22 +76,22 @@ '' + loremIpsum + '' + loremIpsum + '' + '' ), - $headerRow = $collapsible.find( 'tr:first' ), - $contentRow = $collapsible.find( 'tr:last' ), - $toggle = $headerRow.find( 'td:last .mw-collapsible-toggle' ); + $headerRow = $collapsible.find( 'tr' ).first(), + $contentRow = $collapsible.find( 'tr' ).last(), + $toggle = $headerRow.find( 'td' ).last().find( '.mw-collapsible-toggle' ); assert.strictEqual( $toggle.length, 1, 'toggle is added to last cell of first row' ); - assert.assertTrue( $headerRow.is( ':visible' ), 'headerRow is visible' ); - assert.assertTrue( $contentRow.is( ':visible' ), 'contentRow is visible' ); + assert.assertTrue( $headerRow.css( 'display' ) !== 'none', 'headerRow is visible' ); + assert.assertTrue( $contentRow.css( 'display' ) !== 'none', 'contentRow is visible' ); $collapsible.on( 'afterCollapse.mw-collapsible', function () { - assert.assertTrue( $headerRow.is( ':visible' ), 'after collapsing: headerRow is still visible' ); - assert.assertTrue( $contentRow.is( ':hidden' ), 'after collapsing: contentRow is hidden' ); + assert.assertTrue( $headerRow.css( 'display' ) !== 'none', 'after collapsing: headerRow is still visible' ); + assert.assertTrue( $contentRow.css( 'display' ) === 'none', 'after collapsing: contentRow is hidden' ); $collapsible.on( 'afterExpand.mw-collapsible', function () { - assert.assertTrue( $headerRow.is( ':visible' ), 'after expanding: headerRow is still visible' ); - assert.assertTrue( $contentRow.is( ':visible' ), 'after expanding: contentRow is visible' ); + assert.assertTrue( $headerRow.css( 'display' ) !== 'none', 'after expanding: headerRow is still visible' ); + assert.assertTrue( $contentRow.css( 'display' ) !== 'none', 'after expanding: contentRow is visible' ); } ); $toggle.trigger( 'click' ); @@ -102,25 +102,25 @@ function tableWithCaptionTest( $collapsible, test, assert ) { var $caption = $collapsible.find( 'caption' ), - $headerRow = $collapsible.find( 'tr:first' ), - $contentRow = $collapsible.find( 'tr:last' ), + $headerRow = $collapsible.find( 'tr' ).first(), + $contentRow = $collapsible.find( 'tr' ).last(), $toggle = $caption.find( '.mw-collapsible-toggle' ); assert.strictEqual( $toggle.length, 1, 'toggle is added to the end of the caption' ); - assert.assertTrue( $caption.is( ':visible' ), 'caption is visible' ); - assert.assertTrue( $headerRow.is( ':visible' ), 'headerRow is visible' ); - assert.assertTrue( $contentRow.is( ':visible' ), 'contentRow is visible' ); + assert.assertTrue( $caption.css( 'display' ) !== 'none', 'caption is visible' ); + assert.assertTrue( $headerRow.css( 'display' ) !== 'none', 'headerRow is visible' ); + assert.assertTrue( $contentRow.css( 'display' ) !== 'none', 'contentRow is visible' ); $collapsible.on( 'afterCollapse.mw-collapsible', function () { - assert.assertTrue( $caption.is( ':visible' ), 'after collapsing: caption is still visible' ); - assert.assertTrue( $headerRow.is( ':hidden' ), 'after collapsing: headerRow is hidden' ); - assert.assertTrue( $contentRow.is( ':hidden' ), 'after collapsing: contentRow is hidden' ); + assert.assertTrue( $caption.css( 'display' ) !== 'none', 'after collapsing: caption is still visible' ); + assert.assertTrue( $headerRow.css( 'display' ) === 'none', 'after collapsing: headerRow is hidden' ); + assert.assertTrue( $contentRow.css( 'display' ) === 'none', 'after collapsing: contentRow is hidden' ); $collapsible.on( 'afterExpand.mw-collapsible', function () { - assert.assertTrue( $caption.is( ':visible' ), 'after expanding: caption is still visible' ); - assert.assertTrue( $headerRow.is( ':visible' ), 'after expanding: headerRow is visible' ); - assert.assertTrue( $contentRow.is( ':visible' ), 'after expanding: contentRow is visible' ); + assert.assertTrue( $caption.css( 'display' ) !== 'none', 'after expanding: caption is still visible' ); + assert.assertTrue( $headerRow.css( 'display' ) !== 'none', 'after expanding: headerRow is visible' ); + assert.assertTrue( $contentRow.css( 'display' ) !== 'none', 'after expanding: contentRow is visible' ); } ); $toggle.trigger( 'click' ); @@ -159,21 +159,21 @@ '' ), $toggleItem = $collapsible.find( 'li.mw-collapsible-toggle-li:first-child' ), - $contentItem = $collapsible.find( 'li:last' ), + $contentItem = $collapsible.find( 'li' ).last(), $toggle = $toggleItem.find( '.mw-collapsible-toggle' ); assert.strictEqual( $toggle.length, 1, 'toggle is present, added inside new zeroth list item' ); - assert.assertTrue( $toggleItem.is( ':visible' ), 'toggleItem is visible' ); - assert.assertTrue( $contentItem.is( ':visible' ), 'contentItem is visible' ); + assert.assertTrue( $toggleItem.css( 'display' ) !== 'none', 'toggleItem is visible' ); + assert.assertTrue( $contentItem.css( 'display' ) !== 'none', 'contentItem is visible' ); $collapsible.on( 'afterCollapse.mw-collapsible', function () { - assert.assertTrue( $toggleItem.is( ':visible' ), 'after collapsing: toggleItem is still visible' ); - assert.assertTrue( $contentItem.is( ':hidden' ), 'after collapsing: contentItem is hidden' ); + assert.assertTrue( $toggleItem.css( 'display' ) !== 'none', 'after collapsing: toggleItem is still visible' ); + assert.assertTrue( $contentItem.css( 'display' ) === 'none', 'after collapsing: contentItem is hidden' ); $collapsible.on( 'afterExpand.mw-collapsible', function () { - assert.assertTrue( $toggleItem.is( ':visible' ), 'after expanding: toggleItem is still visible' ); - assert.assertTrue( $contentItem.is( ':visible' ), 'after expanding: contentItem is visible' ); + assert.assertTrue( $toggleItem.css( 'display' ) !== 'none', 'after expanding: toggleItem is still visible' ); + assert.assertTrue( $contentItem.css( 'display' ) !== 'none', 'after expanding: contentItem is visible' ); } ); $toggle.trigger( 'click' ); @@ -197,11 +197,11 @@ ), $content = $collapsible.find( '.mw-collapsible-content' ); - assert.assertTrue( $content.is( ':visible' ), 'content is visible' ); + assert.assertTrue( $content.css( 'display' ) !== 'none', 'content is visible' ); $collapsible.find( '.mw-collapsible-toggle' ).trigger( 'click' ); - assert.assertTrue( $content.is( ':hidden' ), 'after collapsing: content is hidden' ); + assert.assertTrue( $content.css( 'display' ) === 'none', 'after collapsing: content is hidden' ); } ); QUnit.test( 'mw-made-collapsible data added', function ( assert ) { @@ -236,10 +236,10 @@ $content = $collapsible.find( '.mw-collapsible-content' ); // Synchronous - mw-collapsed should cause instantHide: true to be used on initial collapsing - assert.assertTrue( $content.is( ':hidden' ), 'content is hidden' ); + assert.assertTrue( $content.css( 'display' ) === 'none', 'content is hidden' ); $collapsible.on( 'afterExpand.mw-collapsible', function () { - assert.assertTrue( $content.is( ':visible' ), 'after expanding: content is visible' ); + assert.assertTrue( $content.css( 'display' ) !== 'none', 'after expanding: content is visible' ); } ); $collapsible.find( '.mw-collapsible-toggle' ).trigger( 'click' ); @@ -253,10 +253,10 @@ $content = $collapsible.find( '.mw-collapsible-content' ); // Synchronous - collapsed: true should cause instantHide: true to be used on initial collapsing - assert.assertTrue( $content.is( ':hidden' ), 'content is hidden' ); + assert.assertTrue( $content.css( 'display' ) === 'none', 'content is hidden' ); $collapsible.on( 'afterExpand.mw-collapsible', function () { - assert.assertTrue( $content.is( ':visible' ), 'after expanding: content is visible' ); + assert.assertTrue( $content.css( 'display' ) !== 'none', 'after expanding: content is visible' ); } ); $collapsible.find( '.mw-collapsible-toggle' ).trigger( 'click' ); @@ -276,10 +276,10 @@ $content = $collapsible.find( '.mw-collapsible-content' ); $collapsible.find( '.mw-collapsible-toggle a' ).trigger( 'click' ); - assert.assertTrue( $content.is( ':visible' ), 'click event on link inside toggle passes through (content not toggled)' ); + assert.assertTrue( $content.css( 'display' ) !== 'none', 'click event on link inside toggle passes through (content not toggled)' ); $collapsible.find( '.mw-collapsible-toggle b' ).trigger( 'click' ); - assert.assertTrue( $content.is( ':hidden' ), 'click event on non-link inside toggle toggles content' ); + assert.assertTrue( $content.css( 'display' ) === 'none', 'click event on non-link inside toggle toggles content' ); } ); QUnit.test( 'click on non-link inside toggler counts as trigger', function ( assert ) { @@ -295,7 +295,7 @@ $content = $collapsible.find( '.mw-collapsible-content' ); $collapsible.find( '.mw-collapsible-toggle a' ).trigger( 'click' ); - assert.assertTrue( $content.is( ':hidden' ), 'click event on link (with no href) inside toggle toggles content' ); + assert.assertTrue( $content.css( 'display' ) === 'none', 'click event on link (with no href) inside toggle toggles content' ); } ); QUnit.test( 'collapse/expand text (data-collapsetext, data-expandtext)', function ( assert ) { @@ -366,10 +366,10 @@ .appendTo( '#qunit-fixture' ).makeCollapsible(), $content = $clone.find( '.mw-collapsible-content' ); - assert.assertTrue( $content.is( ':visible' ), 'content is visible' ); + assert.assertTrue( $content.css( 'display' ) !== 'none', 'content is visible' ); $clone.on( 'afterCollapse.mw-collapsible', function () { - assert.assertTrue( $content.is( ':hidden' ), 'after collapsing: content is hidden' ); + assert.assertTrue( $content.css( 'display' ) === 'none', 'after collapsing: content is hidden' ); } ); $clone.find( '.mw-collapsible-toggle a' ).trigger( 'click' ); -- 2.20.1