Fix context menu being opened when clicking message action bar buttons

src/accessibility/roving/RovingAccessibleTooltipButton.tsx

@@ -26,10 +26,12 @@ interface IProps extends Omit<ATBProps, "inputRef" | "tabIndex"> {
 }
 
 // Wrapper to allow use of useRovingTabIndex for simple AccessibleTooltipButtons outside of React Functional Components.
-export const RovingAccessibleTooltipButton: React.FC<IProps> = ({ inputRef, onFocus, ...props }) => {
+export const RovingAccessibleTooltipButton: React.FC<IProps> = ({ inputRef, onFocus, onClick, ...props }) => {
     const [onFocusInternal, isActive, ref] = useRovingTabIndex(inputRef);
     return <AccessibleTooltipButton
         {...props}
+        onClick={onClick}
+        onContextMenu={onClick}
         onFocus={event => {
             onFocusInternal();
             onFocus?.(event);

src/components/structures/ContextMenu.tsx

@@ -225,35 +225,57 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
 
     protected renderMenu(hasBackground = this.props.hasBackground) {
         const position: Partial<Writeable<DOMRect>> = {};
-        const props = this.props;
-
-        if (props.top) {
-            position.top = props.top;
+        const {
+            top,
+            bottom,
+            left,
+            right,
+            bottomAligned,
+            rightAligned,
+            menuClassName,
+            menuHeight,
+            menuWidth,
+            menuPaddingLeft,
+            menuPaddingRight,
+            menuPaddingBottom,
+            menuPaddingTop,
+            zIndex,
+            children,
+            focusLock,
+            managed,
+            wrapperClassName,
+            chevronFace: propsChevronFace,
+            chevronOffset: propsChevronOffset,
+            ...props
+        } = this.props;
+
+        if (top) {
+            position.top = top;
         } else {
-            position.bottom = props.bottom;
+            position.bottom = bottom;
         }
 
         let chevronFace: ChevronFace;
-        if (props.left) {
-            position.left = props.left;
+        if (left) {
+            position.left = left;
             chevronFace = ChevronFace.Left;
         } else {
-            position.right = props.right;
+            position.right = right;
             chevronFace = ChevronFace.Right;
         }
 
         const contextMenuRect = this.state.contextMenuElem ? this.state.contextMenuElem.getBoundingClientRect() : null;
 
         const chevronOffset: CSSProperties = {};
-        if (props.chevronFace) {
-            chevronFace = props.chevronFace;
+        if (propsChevronFace) {
+            chevronFace = propsChevronFace;
         }
         const hasChevron = chevronFace && chevronFace !== ChevronFace.None;
 
         if (chevronFace === ChevronFace.Top || chevronFace === ChevronFace.Bottom) {
-            chevronOffset.left = props.chevronOffset;
+            chevronOffset.left = propsChevronOffset;
         } else {
-            chevronOffset.top = props.chevronOffset;
+            chevronOffset.top = propsChevronOffset;
         }
 
         // If we know the dimensions of the context menu, adjust its position to
@@ -262,39 +284,39 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
         if (contextMenuRect) {
             if (position.top !== undefined) {
                 let maxTop = windowHeight - WINDOW_PADDING;
-                if (!this.props.bottomAligned) {
+                if (!bottomAligned) {
                     maxTop -= contextMenuRect.height;
                 }
                 position.top = Math.min(position.top, maxTop);
                 // Adjust the chevron if necessary
                 if (chevronOffset.top !== undefined) {
-                    chevronOffset.top = props.chevronOffset + props.top - position.top;
+                    chevronOffset.top = propsChevronOffset + top - position.top;
                 }
             } else if (position.bottom !== undefined) {
                 position.bottom = Math.min(
                     position.bottom,
                     windowHeight - contextMenuRect.height - WINDOW_PADDING,
                 );
                 if (chevronOffset.top !== undefined) {
-                    chevronOffset.top = props.chevronOffset + position.bottom - props.bottom;
+                    chevronOffset.top = propsChevronOffset + position.bottom - bottom;
                 }
             }
             if (position.left !== undefined) {
                 let maxLeft = windowWidth - WINDOW_PADDING;
-                if (!this.props.rightAligned) {
+                if (!rightAligned) {
                     maxLeft -= contextMenuRect.width;
                 }
                 position.left = Math.min(position.left, maxLeft);
                 if (chevronOffset.left !== undefined) {
-                    chevronOffset.left = props.chevronOffset + props.left - position.left;
+                    chevronOffset.left = propsChevronOffset + left - position.left;
                 }
             } else if (position.right !== undefined) {
                 position.right = Math.min(
                     position.right,
                     windowWidth - contextMenuRect.width - WINDOW_PADDING,
                 );
                 if (chevronOffset.left !== undefined) {
-                    chevronOffset.left = props.chevronOffset + position.right - props.right;
+                    chevronOffset.left = propsChevronOffset + position.right - right;
                 }
             }
         }
@@ -320,36 +342,36 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
             'mx_ContextualMenu_withChevron_right': chevronFace === ChevronFace.Right,
             'mx_ContextualMenu_withChevron_top': chevronFace === ChevronFace.Top,
             'mx_ContextualMenu_withChevron_bottom': chevronFace === ChevronFace.Bottom,
-            'mx_ContextualMenu_rightAligned': this.props.rightAligned === true,
-            'mx_ContextualMenu_bottomAligned': this.props.bottomAligned === true,
-        }, this.props.menuClassName);
+            'mx_ContextualMenu_rightAligned': rightAligned === true,
+            'mx_ContextualMenu_bottomAligned': bottomAligned === true,
+        }, menuClassName);
 
         const menuStyle: CSSProperties = {};
-        if (props.menuWidth) {
-            menuStyle.width = props.menuWidth;
+        if (menuWidth) {
+            menuStyle.width = menuWidth;
         }
 
-        if (props.menuHeight) {
-            menuStyle.height = props.menuHeight;
+        if (menuHeight) {
+            menuStyle.height = menuHeight;
         }
 
-        if (!isNaN(Number(props.menuPaddingTop))) {
-            menuStyle["paddingTop"] = props.menuPaddingTop;
+        if (!isNaN(Number(menuPaddingTop))) {
+            menuStyle["paddingTop"] = menuPaddingTop;
         }
-        if (!isNaN(Number(props.menuPaddingLeft))) {
-            menuStyle["paddingLeft"] = props.menuPaddingLeft;
+        if (!isNaN(Number(menuPaddingLeft))) {
+            menuStyle["paddingLeft"] = menuPaddingLeft;
         }
-        if (!isNaN(Number(props.menuPaddingBottom))) {
-            menuStyle["paddingBottom"] = props.menuPaddingBottom;
+        if (!isNaN(Number(menuPaddingBottom))) {
+            menuStyle["paddingBottom"] = menuPaddingBottom;
         }
-        if (!isNaN(Number(props.menuPaddingRight))) {
-            menuStyle["paddingRight"] = props.menuPaddingRight;
+        if (!isNaN(Number(menuPaddingRight))) {
+            menuStyle["paddingRight"] = menuPaddingRight;
         }
 
         const wrapperStyle = {};
-        if (!isNaN(Number(props.zIndex))) {
-            menuStyle["zIndex"] = props.zIndex + 1;
-            wrapperStyle["zIndex"] = props.zIndex;
+        if (!isNaN(Number(zIndex))) {
+            menuStyle["zIndex"] = zIndex + 1;
+            wrapperStyle["zIndex"] = zIndex;
         }
 
         let background;
@@ -366,10 +388,10 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
 
         let body = <>
             { chevron }
-            { props.children }
+            { children }
         </>;
 
-        if (props.focusLock) {
+        if (focusLock) {
             body = <FocusLock>
                 { body }
             </FocusLock>;
@@ -379,7 +401,7 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
             <RovingTabIndexProvider handleHomeEnd handleUpDown onKeyDown={this.onKeyDown}>
                 { ({ onKeyDownHandler }) => (
                     <div
-                        className={classNames("mx_ContextualMenu_wrapper", this.props.wrapperClassName)}
+                        className={classNames("mx_ContextualMenu_wrapper", wrapperClassName)}
                         style={{ ...position, ...wrapperStyle }}
                         onClick={this.onClick}
                         onKeyDown={onKeyDownHandler}
@@ -390,7 +412,8 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
                             className={menuClasses}
                             style={menuStyle}
                             ref={this.collectContextMenuRect}
-                            role={this.props.managed ? "menu" : undefined}
+                            role={managed ? "menu" : undefined}
+                            {...props}
                         >
                             { body }
                         </div>

src/components/views/messages/MessageActionBar.tsx

@@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React, { ReactElement, useContext, useEffect } from 'react';
+import React, { ReactElement, useCallback, useContext, useEffect } from 'react';
 import { EventStatus, MatrixEvent, MatrixEventEvent } from 'matrix-js-sdk/src/models/event';
 import classNames from 'classnames';
 import { MsgType, RelationType } from 'matrix-js-sdk/src/@types/event';
@@ -88,7 +88,7 @@ const OptionsButton: React.FC<IOptionsButtonProps> = ({
         onFocusChange(menuDisplayed);
     }, [onFocusChange, menuDisplayed]);
 
-    const onOptionsClick = (e: React.MouseEvent): void => {
+    const onOptionsClick = useCallback((e: React.MouseEvent): void => {
         // Don't open the regular browser or our context menu on right-click
         e.preventDefault();
         e.stopPropagation();
@@ -97,7 +97,7 @@ const OptionsButton: React.FC<IOptionsButtonProps> = ({
         // the element that is currently focused is skipped. So we want to call onFocus manually to keep the
         // position in the page even when someone is clicking around.
         onFocus();
-    };
+    }, [openMenu, onFocus]);
 
     let contextMenu: ReactElement | null;
     if (menuDisplayed) {
@@ -153,17 +153,23 @@ const ReactButton: React.FC<IReactButtonProps> = ({ mxEvent, reactions, onFocusC
         </ContextMenu>;
     }
 
+    const onClick = useCallback((e: React.MouseEvent) => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
+        openMenu();
+        // when the context menu is opened directly, e.g. via mouse click, the onFocus handler which tracks
+        // the element that is currently focused is skipped. So we want to call onFocus manually to keep the
+        // position in the page even when someone is clicking around.
+        onFocus();
+    }, [openMenu, onFocus]);
+
     return <React.Fragment>
         <ContextMenuTooltipButton
             className="mx_MessageActionBar_iconButton"
             title={_t("React")}
-            onClick={() => {
-                openMenu();
-                // when the context menu is opened directly, e.g. via mouse click, the onFocus handler which tracks
-                // the element that is currently focused is skipped. So we want to call onFocus manually to keep the
-                // position in the page even when someone is clicking around.
-                onFocus();
-            }}
+            onClick={onClick}
             isExpanded={menuDisplayed}
             inputRef={ref}
             onFocus={onFocus}
@@ -193,7 +199,11 @@ const ReplyInThreadButton = ({ mxEvent }: IReplyInThreadButton) => {
         return null;
     }
 
-    const onClick = (): void => {
+    const onClick = (e: React.MouseEvent): void => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
         if (firstTimeSeeingThreads) {
             localStorage.setItem("mx_seen_feature_thread", "true");
         }
@@ -265,10 +275,18 @@ const FavouriteButton = ({ mxEvent }: IFavouriteButtonProp) => {
         'mx_MessageActionBar_favouriteButton_fillstar': isFavourite(eventId),
     });
 
+    const onClick = useCallback((e: React.MouseEvent) => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
+        toggleFavourite(eventId);
+    }, [toggleFavourite, eventId]);
+
     return <RovingAccessibleTooltipButton
         className={classes}
         title={_t("Favourite")}
-        onClick={() => toggleFavourite(eventId)}
+        onClick={onClick}
         data-testid={eventId}
     >
         <StarIcon />
@@ -335,15 +353,23 @@ export default class MessageActionBar extends React.PureComponent<IMessageAction
         this.props.onFocusChange?.(focused);
     };
 
-    private onReplyClick = (ev: React.MouseEvent): void => {
+    private onReplyClick = (e: React.MouseEvent): void => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
         dis.dispatch({
             action: 'reply_to_event',
             event: this.props.mxEvent,
             context: this.context.timelineRenderingType,
         });
     };
 
-    private onEditClick = (): void => {
+    private onEditClick = (e: React.MouseEvent): void => {
+        // Don't open the regular browser or our context menu on right-click
+        e.preventDefault();
+        e.stopPropagation();
+
         editEvent(this.props.mxEvent, this.context.timelineRenderingType, this.props.getRelationsForEvent);
     };
 

test/components/views/messages/MessageActionBar-test.tsx

@@ -565,4 +565,29 @@ describe('<MessageActionBar />', () => {
             });
         });
     });
+
+    it.each([
+        ["React"],
+        ["Reply"],
+        ["Reply in thread"],
+        ["Favourite"],
+        ["Edit"],
+    ])("does not show context menu when right-clicking", (buttonLabel: string) => {
+        // For favourite button
+        jest.spyOn(SettingsStore, 'getValue').mockReturnValue(true);
+
+        const { queryByTestId, queryByLabelText } = getComponent({ mxEvent: alicesMessageEvent });
+        act(() => {
+            fireEvent.contextMenu(queryByLabelText(buttonLabel));
+        });
+        expect(queryByTestId("mx_MessageContextMenu")).toBeFalsy();
+    });
+
+    it("does shows context menu when right-clicking options", () => {
+        const { queryByTestId, queryByLabelText } = getComponent({ mxEvent: alicesMessageEvent });
+        act(() => {
+            fireEvent.contextMenu(queryByLabelText("Options"));
+        });
+        expect(queryByTestId("mx_MessageContextMenu")).toBeTruthy();
+    });
 });