From 54c27e576393c97ff915c83fda781e1cf35f0f7b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= <matma.rex@gmail.com>
Date: Tue, 23 Aug 2022 23:02:28 +0200
Subject: [PATCH] Fix notification dynamic actions outside of the dropdown

* Event handler was not registered for the non-dropdown version
* Clicks would be handled by both the action and by the default link

Also remove returning false to prevent default and stop propagation,
this is not supported by OOUI events and did nothing. Handling a
'click' event always returns false to jQuery (so this was not needed),
handling 'choose' event never does (hence my other fix was needed).

Change-Id: Ia8a21749a8edc20f34b2a3e445278ea6922b9109
---
 modules/ui/mw.echo.ui.MenuItemWidget.js         | 15 +++++++++++++++
 modules/ui/mw.echo.ui.NotificationItemWidget.js | 17 +++++++----------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/modules/ui/mw.echo.ui.MenuItemWidget.js b/modules/ui/mw.echo.ui.MenuItemWidget.js
index d55dd87c7..4a4462442 100644
--- a/modules/ui/mw.echo.ui.MenuItemWidget.js
+++ b/modules/ui/mw.echo.ui.MenuItemWidget.js
@@ -76,6 +76,21 @@
 		return this.isLink ? 'a' : 'div';
 	};
 
+	mw.echo.ui.MenuItemWidget.prototype.onClick = function ( e ) {
+		// Stop propagation, so that the default dynamic action of the notification isn't triggered
+		// (e.g. expanding a bundled notification).
+		e.stopPropagation();
+
+		// If this is a dynamic action, also prevent default to disable the native browser behavior,
+		// the default link of the notification won't be followed.
+		// (If this is a link, default link of the notification is ignored as native browser behavior.)
+		if ( !this.isLink ) {
+			e.preventDefault();
+		}
+
+		return mw.echo.ui.MenuItemWidget.super.prototype.onClick.apply( this, arguments );
+	};
+
 	mw.echo.ui.MenuItemWidget.prototype.isSelectable = function () {
 		// If we have a link force selectability to false, otherwise defer to parent method
 		// Without a link (for dynamic actions or specific internal actions) we need this widget
diff --git a/modules/ui/mw.echo.ui.NotificationItemWidget.js b/modules/ui/mw.echo.ui.NotificationItemWidget.js
index 17a0886aa..b362dbaa6 100644
--- a/modules/ui/mw.echo.ui.NotificationItemWidget.js
+++ b/modules/ui/mw.echo.ui.NotificationItemWidget.js
@@ -189,6 +189,7 @@
 		}
 
 		// Events
+		this.actionsButtonSelectWidget.connect( this, { choose: 'onPopupButtonWidgetChoose' } );
 		this.menuPopupButtonWidget.getMenu().connect( this, { choose: 'onPopupButtonWidgetChoose' } );
 		this.markAsReadButton.connect( this, { click: 'onMarkAsReadButtonClick' } );
 
@@ -228,10 +229,14 @@
 	 * NOTE: The messages are parsed as HTML. If user-input is expected
 	 * please make sure to properly escape it.
 	 *
-	 * @param {mw.echo.ui.MenuItemWidget} item The selected item
-	 * @return {boolean} False to prevent the native event
+	 * @param {OO.ui.ButtonOptionWidget} item The selected item
 	 */
 	mw.echo.ui.NotificationItemWidget.prototype.onPopupButtonWidgetChoose = function ( item ) {
+		if ( !( item instanceof mw.echo.ui.MenuItemWidget ) ) {
+			// Other kinds of items may be added by subclasses
+			return;
+		}
+
 		var actionData = item && item.getActionData(),
 			messages = item && item.getConfirmationMessages(),
 			widget = this;
@@ -268,24 +273,16 @@
 				// Send to mw.notify
 				mw.notify( $confirmation );
 			} );
-
-		// Prevent the click propagation
-		return false;
 	};
 
 	/**
 	 * Respond to mark as read button click
-	 *
-	 * @return {boolean} False to prevent the native event
 	 */
 	mw.echo.ui.NotificationItemWidget.prototype.onMarkAsReadButtonClick = function () {
 		// If we're marking read or unread, the notification was already seen.
 		// Remove the animation class
 		this.$element.removeClass( 'mw-echo-ui-notificationItemWidget-initiallyUnseen' );
 		this.markRead( !this.model.isRead() );
-		// Prevent propogation in case there's a link wrapping the content
-		// and the mark as read/unread button
-		return false;
 	};
 
 	/**