mediawiki.util: Clean up nextnode logic

Change-Id: I21469a765002897f03caca97628ad36264d49c83

diff --git a/resources/src/mediawiki/mediawiki.util.js b/resources/src/mediawiki/mediawiki.util.js
index 2662913..fe5f2b2 100644 (file)
--- a/resources/src/mediawiki/mediawiki.util.js
+++ b/resources/src/mediawiki/mediawiki.util.js
@@ -290,26 +290,30 @@
                        }
 
                        if ( nextnode ) {
+                               // Case: nextnode is a DOM element (was the only option before MW 1.17, in wikibits.js)
+                               // Case: nextnode is a CSS selector for jQuery
                                if ( nextnode.nodeType || typeof nextnode === 'string' ) {
-                                       // nextnode is a DOM element (was the only option before MW 1.17, in wikibits.js)
-                                       // or nextnode is a CSS selector for jQuery
                                        nextnode = $ul.find( nextnode );
-                               } else if ( !nextnode.jquery || ( nextnode.length && nextnode[0].parentNode !== $ul[0] ) ) {
-                                       // Fallback
-                                       $ul.append( $item );
-                                       return $item[0];
+                               } else if ( !nextnode.jquery ) {
+                                       // Error: Invalid nextnode
+                                       nextnode = undefined;
                                }
-                               if ( nextnode.length === 1 ) {
-                                       // nextnode is a jQuery object that represents exactly one element
-                                       nextnode.before( $item );
-                                       return $item[0];
+                               if ( nextnode && ( nextnode.length !== 1 || nextnode[0].parentNode !== $ul[0] ) ) {
+                                       // Error: nextnode must resolve to a single node
+                                       // Error: nextnode must have the associated <ul> as its parent
+                                       nextnode = undefined;
                                }
                        }
 
-                       // Fallback (this is the default behavior)
-                       $ul.append( $item );
-                       return $item[0];
+                       // Case: nextnode is a jQuery-wrapped DOM element
+                       if ( nextnode ) {
+                               nextnode.before( $item );
+                       } else {
+                               // Fallback (this is the default behavior)
+                               $ul.append( $item );
+                       }
 
+                       return $item[0];
                },
 
                /**

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js
index 4401ead..9b620de 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js
@@ -189,7 +189,7 @@
                );
 
                assert.equal( $tbMW.closest( '.portlet' ).attr( 'id' ), 'p-test-tb', 'Link was inserted within correct portlet' );
-               assert.strictEqual( $tbMW.next()[0], tbRL, 'Link is in the correct position (by passing nextnode)' );
+               assert.strictEqual( $tbMW.next()[0], tbRL, 'Link is in the correct position (nextnode as Node object)' );
 
                cuQuux = mw.util.addPortletLink( 'p-test-custom', '#', 'Quux', null, 'Example [shift-x]', 'q' );
                $cuQuux = $( cuQuux );
@@ -205,7 +205,7 @@
                tbRLDM = mw.util.addPortletLink( 'p-test-tb', '//mediawiki.org/wiki/RL/DM',
                        'Default modules', 't-rldm', 'List of all default modules ', 'd', '#t-rl' );
 
-               assert.equal( $( tbRLDM ).next().attr( 'id' ), 't-rl', 'Link is in the correct position (by passing CSS selector)' );
+               assert.strictEqual( $( tbRLDM ).next()[0], tbRL, 'Link is in the correct position (CSS selector as nextnode)' );
 
                caFoo = mw.util.addPortletLink( 'p-test-views', '#', 'Foo' );
 
@@ -213,19 +213,19 @@
                assert.strictEqual( $( caFoo ).find( 'span' ).length, 1, 'A <span> element should be added for porlets with vectorTabs class.' );
 
                addedAfter = mw.util.addPortletLink( 'p-test-tb', '#', 'After foo', 'post-foo', 'After foo', null, $( tbRL ) );
-               assert.strictEqual( $( addedAfter ).next()[0], tbRL, 'Link is in the correct position (by passing a jQuery object as nextnode)' );
+               assert.strictEqual( $( addedAfter ).next()[0], tbRL, 'Link is in the correct position (jQuery object as nextnode)' );
 
                // test case - nonexistent id as next node
                tbRLDMnonexistentid = mw.util.addPortletLink( 'p-test-tb', '//mediawiki.org/wiki/RL/DM',
                        'Default modules', 't-rldm-nonexistent', 'List of all default modules ', 'd', '#t-rl-nonexistent' );
 
-               assert.equal( tbRLDMnonexistentid, $( '#p-test-tb li:last' )[0], 'Nonexistent id as nextnode adds the portlet at end' );
+               assert.equal( tbRLDMnonexistentid, $( '#p-test-tb li:last' )[0], 'Fallback to adding at the end (nextnode non-matching CSS selector)' );
 
                // test case - empty jquery object as next node
                tbRLDMemptyjquery = mw.util.addPortletLink( 'p-test-tb', '//mediawiki.org/wiki/RL/DM',
                        'Default modules', 't-rldm-empty-jquery', 'List of all default modules ', 'd', $( '#t-rl-nonexistent' ) );
 
-               assert.equal( tbRLDMemptyjquery, $( '#p-test-tb li:last' )[0], 'Empty jquery as nextnode adds the portlet at end' );
+               assert.equal( tbRLDMemptyjquery, $( '#p-test-tb li:last' )[0], 'Fallback to adding at the end (nextnode as empty jQuery object)' );
        } );
 
        QUnit.test( 'jsMessage', 1, function ( assert ) {