Fixes to addPortletLink hook handler

* Ensure addPortletLink hook is only run once
* Mark more menu as not supporting icons so icons are not added

Bug: T317491
Bug: T318495
Change-Id: I99450a5b0410e88cc7cdb2753b9b4256e3fe41db

includes/SkinVector.php

@@ -835,8 +835,12 @@ abstract class SkinVector extends SkinMustache {
 		string $key,
 		array $portletData
 	): array {
+		$isIconDropdown = false;
 		switch ( $key ) {
 			case 'data-user-menu':
+				$type = self::MENU_TYPE_DROPDOWN;
+				$isIconDropdown = true;
+				break;
 			case 'data-actions':
 			case 'data-variants':
 			case 'data-sticky-header-toc':
@@ -874,6 +878,11 @@ abstract class SkinVector extends SkinMustache {
 			$portletData['class'] .= ' vector-user-menu-overflow';
 		}
 
+		$isDropdown = $type === self::MENU_TYPE_DROPDOWN;
+		if ( $isDropdown && !$isIconDropdown ) {
+			$portletData['class'] .= ' vector-menu-dropdown-noicon';
+		}
+
 		if ( $key === 'data-personal' && $this->isLegacy() ) {
 			// Set tooltip to empty string for the personal menu for both logged-in and logged-out users
 			// to avoid showing the tooltip for legacy version.
@@ -900,7 +909,7 @@ abstract class SkinVector extends SkinMustache {
 		);
 
 		return $portletData + [
-			'is-dropdown' => $type === self::MENU_TYPE_DROPDOWN,
+			'is-dropdown' => $isDropdown,
 			'is-portal' => $type === self::MENU_TYPE_PORTAL,
 		];
 	}

resources/skins.vector.js/dropdownMenus.js

@@ -61,6 +61,8 @@ function createIconElement( menuElement, parentElement, id ) {
 	return iconElement;
 }
 
+var /** @type {Object} */handledLinks = {};
+
 /**
  * Adds icon placeholder for gadgets to use.
  *
@@ -76,6 +78,10 @@ function addPortletLinkHandler( item, data ) {
 	var $menu = $( item ).parents( '.vector-menu' );
 	var menuElement = $menu.length && $menu.get( 0 ) || null;
 	var iconElement = createIconElement( menuElement, link, data.id );
+	// @ts-ignore see gerrit:836861
+	if ( data.id && handledLinks[ data.id ] ) {
+		return;
+	}
 
 	// The views menu has limited space so we need to decide whether there is space
 	// to accomodate the new item and if not to redirect to the more dropdown.
@@ -99,6 +105,12 @@ function addPortletLinkHandler( item, data ) {
 	if ( link && iconElement ) {
 		link.prepend( iconElement );
 	}
+
+	// Mark the link as handled.
+	if ( data.id ) {
+		// @ts-ignore see gerrit:836861
+		handledLinks[ data.id ] = true;
+	}
 }
 
 // Enhance previously added items.

tests/jest/__snapshots__/dropdownMenu.test.js.snap

@@ -36,17 +36,17 @@ exports[`addPortletLinkHandler JS portlet should be moved to more menu (#p-cacti
 
 			<div id=\\"p-cactions\\">
 				<ul>
+				</ul>
+			</div>
+
+			<ul id=\\"p-views\\" class=\\"vector-menu vector-menu-dropdown\\">
 				<li class=\\"mw-list-item mw-list-item-js\\" id=\\"test-id\\">
-					<a href=\\"#test-href\\"><span class=\\"mw-ui-icon mw-ui-icon-vector-gadget-test-id\\"></span>
+					<a href=\\"#test-href\\">
 						<span>
 							test link content
 						</span>
 					</a>
-				</li></ul>
-			</div>
-
-			<ul id=\\"p-views\\" class=\\"vector-menu vector-menu-dropdown\\">
-				
+				</li>
 			</ul>
 		</div>"
 `;

tests/phpunit/integration/SkinVectorTest.php

@@ -271,11 +271,11 @@ class SkinVectorTest extends MediaWikiIntegrationTestCase {
 			$namespaces['class']
 		);
 		$this->assertSame(
-			'mw-portlet mw-portlet-variants vector-menu-dropdown',
+			'mw-portlet mw-portlet-variants vector-menu-dropdown-noicon vector-menu-dropdown',
 			$variants['class']
 		);
 		$this->assertSame(
-			'mw-portlet mw-portlet-cactions vector-menu-dropdown',
+			'mw-portlet mw-portlet-cactions vector-menu-dropdown-noicon vector-menu-dropdown',
 			$actions['class']
 		);
 		$this->assertSame(