Fix context menu abnormaly.

Don't calculate popup position for context menu if the desired position is current cursor position. Instead, pass x = undefined and y = undefined to popup(), and Electron will figure out the accurate value.
microsoft · Jul 6, 2023 · 1483543 · 1483543
1 parent 56f97a4
commit 1483543
Show file tree

Hide file tree

Showing 21 changed files with 44 additions and 31 deletions.
diff --git a/src/vs/base/browser/contextmenu.ts b/src/vs/base/browser/contextmenu.ts
@@ -17,7 +17,7 @@ export interface IContextMenuEvent {
 }
 
 export interface IContextMenuDelegate {
-	getAnchor(): HTMLElement | { x: number; y: number; width?: number; height?: number };
+	getAnchor(): HTMLElement | { x: number; y: number; width?: number; height?: number; isCurrentCursor?: boolean };
 	getActions(): readonly IAction[];
 	getCheckedActionsRepresentation?(action: IAction): 'radio' | 'checkbox';
 	getActionViewItem?(action: IAction, options: IActionViewItemOptions): IActionViewItem | undefined;

diff --git a/src/vs/base/browser/ui/contextview/contextview.ts b/src/vs/base/browser/ui/contextview/contextview.ts
@@ -21,6 +21,7 @@ export interface IAnchor {
 	y: number;
 	width?: number;
 	height?: number;
+	isCurrentCursor?: boolean;
 }
 
 export const enum AnchorAlignment {

diff --git a/src/vs/base/browser/ui/list/list.ts b/src/vs/base/browser/ui/list/list.ts
@@ -61,7 +61,7 @@ export interface IListContextMenuEvent<T> {
 	readonly browserEvent: UIEvent;
 	readonly element: T | undefined;
 	readonly index: number | undefined;
-	readonly anchor: HTMLElement | { readonly x: number; readonly y: number };
+	readonly anchor: HTMLElement | { readonly x: number; readonly y: number; readonly isCurrentCursor?: boolean };
 }
 
 export interface IIdentityProvider<T> {

diff --git a/src/vs/base/browser/ui/list/listWidget.ts b/src/vs/base/browser/ui/list/listWidget.ts
@@ -1353,7 +1353,7 @@ export class List<T> implements ISpliceable<T>, IDisposable {
 
 		const fromMouse = this.disposables.add(Event.chain(this.view.onContextMenu))
 			.filter(_ => !didJustPressContextMenuKey)
-			.map(({ element, index, browserEvent }) => ({ element, index, anchor: { x: browserEvent.pageX + 1, y: browserEvent.pageY }, browserEvent }))
+			.map(({ element, index, browserEvent }) => ({ element, index, anchor: { x: browserEvent.pageX + 1, y: browserEvent.pageY, isCurrentCursor: true }, browserEvent }))
 			.event;
 
 		return Event.any<IListContextMenuEvent<T>>(fromKeyDown, fromKeyUp, fromMouse);

diff --git a/src/vs/base/browser/ui/tree/tree.ts b/src/vs/base/browser/ui/tree/tree.ts
@@ -176,7 +176,7 @@ export interface ITreeMouseEvent<T> {
 export interface ITreeContextMenuEvent<T> {
 	readonly browserEvent: UIEvent;
 	readonly element: T | null;
-	readonly anchor: HTMLElement | { readonly x: number; readonly y: number };
+	readonly anchor: HTMLElement | { readonly x: number; readonly y: number; readonly isCurrentCursor?: boolean };
 }
 
 export interface ITreeNavigator<T> {

diff --git a/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts b/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts
@@ -103,7 +103,7 @@ export class ContextMenuController implements IEditorContribution {
 		e.event.stopPropagation();
 
 		if (e.target.type === MouseTargetType.SCROLLBAR) {
-			return this._showScrollbarContextMenu({ x: e.event.posx - 1, width: 2, y: e.event.posy - 1, height: 2 });
+			return this._showScrollbarContextMenu({ x: e.event.posx - 1, width: 2, y: e.event.posy - 1, height: 2, isCurrentCursor: true });
 		}
 
 		if (e.target.type !== MouseTargetType.CONTENT_TEXT && e.target.type !== MouseTargetType.CONTENT_EMPTY && e.target.type !== MouseTargetType.TEXTAREA) {
@@ -131,7 +131,7 @@ export class ContextMenuController implements IEditorContribution {
 		// Unless the user triggerd the context menu through Shift+F10, use the mouse position as menu position
 		let anchor: IAnchor | null = null;
 		if (e.target.type !== MouseTargetType.TEXTAREA) {
-			anchor = { x: e.event.posx - 1, width: 2, y: e.event.posy - 1, height: 2 };
+			anchor = { x: e.event.posx - 1, width: 2, y: e.event.posy - 1, height: 2, isCurrentCursor: true };
 		}
 
 		// Show the context menu

diff --git a/src/vs/workbench/browser/parts/activitybar/activitybarActions.ts b/src/vs/workbench/browser/parts/activitybar/activitybarActions.ts
@@ -152,7 +152,8 @@ abstract class AbstractGlobalActivityActionViewItem extends ActivityActionViewIt
 			const event = new StandardMouseEvent(e);
 			const anchor = {
 				x: event.posx,
-				y: event.posy
+				y: event.posy,
+				isCurrentCursor: true,
 			};
 
 			this.contextMenuService.showContextMenu({

diff --git a/src/vs/workbench/browser/parts/compositeBar.ts b/src/vs/workbench/browser/parts/compositeBar.ts
@@ -656,7 +656,7 @@ export class CompositeBar extends Widget implements ICompositeBar {
 
 		const event = new StandardMouseEvent(e);
 		this.contextMenuService.showContextMenu({
-			getAnchor: () => { return { x: event.posx, y: event.posy }; },
+			getAnchor: () => { return { x: event.posx, y: event.posy, isCurrentCursor: true }; },
 			getActions: () => this.getContextMenuActions(e)
 		});
 	}

diff --git a/src/vs/workbench/browser/parts/editor/editorGroupView.ts b/src/vs/workbench/browser/parts/editor/editorGroupView.ts
@@ -374,10 +374,10 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		}
 
 		// Find target anchor
-		let anchor: HTMLElement | { x: number; y: number } = this.element;
+		let anchor: HTMLElement | { x: number; y: number; isCurrentCursor?: boolean } = this.element;
 		if (e instanceof MouseEvent) {
 			const event = new StandardMouseEvent(e);
-			anchor = { x: event.posx, y: event.posy };
+			anchor = { x: event.posx, y: event.posy, isCurrentCursor: true };
 		}
 
 		// Show it

diff --git a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts
@@ -454,10 +454,10 @@ export class TabsTitleControl extends TitleControl {
 			EventHelper.stop(e);
 
 			// Find target anchor
-			let anchor: HTMLElement | { x: number; y: number } = tabsContainer;
+			let anchor: HTMLElement | { x: number; y: number; isCurrentCursor?: boolean } = tabsContainer;
 			if (e instanceof MouseEvent) {
 				const event = new StandardMouseEvent(e);
-				anchor = { x: event.posx, y: event.posy };
+				anchor = { x: event.posx, y: event.posy, isCurrentCursor: true };
 			}
 
 			// Show it

diff --git a/src/vs/workbench/browser/parts/editor/titleControl.ts b/src/vs/workbench/browser/parts/editor/titleControl.ts
@@ -379,10 +379,10 @@ export abstract class TitleControl extends Themable {
 		applyAvailableEditorIds(this.editorAvailableEditorIds, editor, this.editorResolverService);
 
 		// Find target anchor
-		let anchor: HTMLElement | { x: number; y: number } = node;
+		let anchor: HTMLElement | { x: number; y: number; isCurrentCursor?: boolean } = node;
 		if (e instanceof MouseEvent) {
 			const event = new StandardMouseEvent(e);
-			anchor = { x: event.posx, y: event.posy };
+			anchor = { x: event.posx, y: event.posy, isCurrentCursor: true };
 		}
 
 		// Show it

diff --git a/src/vs/workbench/browser/parts/sidebar/sidebarPart.ts b/src/vs/workbench/browser/parts/sidebar/sidebarPart.ts
@@ -282,7 +282,7 @@ export class SidebarPart extends CompositePart<PaneComposite> implements IPaneCo
 		if (activeViewlet) {
 			const contextMenuActions = activeViewlet ? activeViewlet.getContextMenuActions() : [];
 			if (contextMenuActions.length) {
-				const anchor: { x: number; y: number } = { x: event.posx, y: event.posy };
+				const anchor: { x: number; y: number; isCurrentCursor?: boolean } = { x: event.posx, y: event.posy, isCurrentCursor: true };
 				this.contextMenuService.showContextMenu({
 					getAnchor: () => anchor,
 					getActions: () => contextMenuActions.slice(),

diff --git a/src/vs/workbench/browser/parts/statusbar/statusbarPart.ts b/src/vs/workbench/browser/parts/statusbar/statusbarPart.ts
@@ -453,7 +453,7 @@ export class StatusbarPart extends Part implements IStatusbarService {
 
 		let actions: IAction[] | undefined = undefined;
 		this.contextMenuService.showContextMenu({
-			getAnchor: () => ({ x: event.posx, y: event.posy }),
+			getAnchor: () => ({ x: event.posx, y: event.posy, isCurrentCursor: true }),
 			getActions: () => {
 				actions = this.getContextMenuActions(event);
 

diff --git a/src/vs/workbench/browser/parts/titlebar/titlebarPart.ts b/src/vs/workbench/browser/parts/titlebar/titlebarPart.ts
@@ -399,7 +399,7 @@ export class TitlebarPart extends Part implements ITitleService {
 	protected onContextMenu(e: MouseEvent, menuId: MenuId): void {
 		// Find target anchor
 		const event = new StandardMouseEvent(e);
-		const anchor = { x: event.posx, y: event.posy };
+		const anchor = { x: event.posx, y: event.posy, isCurrentCursor: true };
 
 		// Show it
 		this.contextMenuService.showContextMenu({

diff --git a/src/vs/workbench/browser/parts/views/viewPaneContainer.ts b/src/vs/workbench/browser/parts/views/viewPaneContainer.ts
@@ -577,7 +577,7 @@ export class ViewPaneContainer extends Component implements IViewPaneContainer {
 		event.stopPropagation();
 		event.preventDefault();
 
-		const anchor: { x: number; y: number } = { x: event.posx, y: event.posy };
+		const anchor: { x: number; y: number; isCurrentCursor?: boolean } = { x: event.posx, y: event.posy, isCurrentCursor: true };
 		this.contextMenuService.showContextMenu({
 			getAnchor: () => anchor,
 			getActions: () => this.menuActions?.getContextMenuActions() ?? []
@@ -743,7 +743,7 @@ export class ViewPaneContainer extends Component implements IViewPaneContainer {
 
 		const actions: IAction[] = viewPane.menuActions.getContextMenuActions();
 
-		const anchor: { x: number; y: number } = { x: event.posx, y: event.posy };
+		const anchor: { x: number; y: number; isCurrentCursor?: boolean } = { x: event.posx, y: event.posy, isCurrentCursor: true };
 		this.contextMenuService.showContextMenu({
 			getAnchor: () => anchor,
 			getActions: () => actions

diff --git a/src/vs/workbench/contrib/codeEditor/browser/editorLineNumberMenu.ts b/src/vs/workbench/contrib/codeEditor/browser/editorLineNumberMenu.ts
@@ -77,7 +77,7 @@ export class EditorLineNumberContextMenu extends Disposable implements IEditorCo
 			return;
 		}
 
-		const anchor = { x: e.event.posx, y: e.event.posy };
+		const anchor = { x: e.event.posx, y: e.event.posy, isCurrentCursor: true };
 		const lineNumber = e.target.position.lineNumber;
 
 		const contextKeyService = this.contextKeyService.createOverlay([['editorLineNumber', lineNumber]]);

diff --git a/src/vs/workbench/contrib/comments/browser/commentsController.ts b/src/vs/workbench/contrib/comments/browser/commentsController.ts
@@ -829,7 +829,7 @@ export class CommentController implements IEditorContribution {
 
 		if (newCommentInfos.length > 1) {
 			if (e && range) {
-				const anchor = { x: e.event.posx, y: e.event.posy };
+				const anchor = { x: e.event.posx, y: e.event.posy, isCurrentCursor: true };
 
 				this.contextMenuService.showContextMenu({
 					getAnchor: () => anchor,

diff --git a/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts b/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts
@@ -728,7 +728,7 @@ class InlineBreakpointWidget implements IContentWidget, IDisposable {
 		}));
 		this.toDispose.push(dom.addDisposableListener(this.domNode, dom.EventType.CONTEXT_MENU, e => {
 			const event = new StandardMouseEvent(e);
-			const anchor = { x: event.posx, y: event.posy };
+			const anchor = { x: event.posx, y: event.posy, isCurrentCursor: true };
 			const actions = this.getContextMenuActions();
 			this.contextMenuService.showContextMenu({
 				getAnchor: () => anchor,

diff --git a/src/vs/workbench/contrib/preferences/browser/preferencesRenderers.ts b/src/vs/workbench/contrib/preferences/browser/preferencesRenderers.ts
@@ -354,7 +354,7 @@ class EditSettingRenderer extends Disposable {
 	private onEditSettingClicked(editPreferenceWidget: EditPreferenceWidget<IIndexedSetting>, e: IEditorMouseEvent): void {
 		EventHelper.stop(e.event, true);
 
-		const anchor = { x: e.event.posx, y: e.event.posy };
+		const anchor = { x: e.event.posx, y: e.event.posy, isCurrentCursor: true };
 		const actions = this.getSettings(editPreferenceWidget.getLine()).length === 1 ? this.getActions(editPreferenceWidget.preferences[0], this.getConfigurationsMap()[editPreferenceWidget.preferences[0].key])
 			: editPreferenceWidget.preferences.map(setting => new SubmenuAction(`preferences.submenu.${setting.key}`, setting.key, this.getActions(setting, this.getConfigurationsMap()[setting.key])));
 		this.contextMenuService.showContextMenu({

diff --git a/src/vs/workbench/contrib/terminal/browser/terminalContextMenu.ts b/src/vs/workbench/contrib/terminal/browser/terminalContextMenu.ts
@@ -12,7 +12,7 @@ import { IContextMenuService } from 'vs/platform/contextview/browser/contextView
 export function openContextMenu(event: MouseEvent, parent: HTMLElement, menu: IMenu, contextMenuService: IContextMenuService, extraActions?: IAction[]): void {
 	const standardEvent = new StandardMouseEvent(event);
 
-	const anchor: { x: number; y: number } = { x: standardEvent.posx, y: standardEvent.posy };
+	const anchor: { x: number; y: number; isCurrentCursor?: boolean } = { x: standardEvent.posx, y: standardEvent.posy, isCurrentCursor: true };
 	const actions: IAction[] = [];
 
 	createAndFillInContextMenuActions(menu, undefined, actions);

diff --git a/src/vs/workbench/services/contextmenu/electron-sandbox/contextmenuService.ts b/src/vs/workbench/services/contextmenu/electron-sandbox/contextmenuService.ts
@@ -103,8 +103,8 @@ class NativeContextMenuService extends Disposable implements IContextMenuService
 			const menu = this.createMenu(delegate, actions, onHide);
 			const anchor = delegate.getAnchor();
 
-			let x: number;
-			let y: number;
+			let x: number | undefined;
+			let y: number | undefined;
 
 			let zoom = getZoomFactor();
 			if (dom.isHTMLElement(anchor)) {
@@ -156,17 +156,28 @@ class NativeContextMenuService extends Disposable implements IContextMenuService
 					y += 4 / zoom;
 				}
 			} else {
-				const pos: { x: number; y: number } = anchor;
+				const pos = anchor;
 				x = pos.x + 1; /* prevent first item from being selected automatically under mouse */
 				y = pos.y;
+				if (pos.isCurrentCursor) {
+					// If (x, y) indicates the current cursor position, we set
+					// both to undefined and let Electron figure out the accurate value.
+					// (https://www.electronjs.org/docs/latest/api/menu#menupopupoptions)
+					// This avoids the numeric loss when applying *zoom and Math.floor,
+					// which leads to the closest being accidentally clicked.
+					// https://github.com/microsoft/vscode/issues/113175
+					x = y = undefined;
+				}
 			}
 
-			x *= zoom;
-			y *= zoom;
+			if (x !== undefined && y !== undefined) {
+				x = Math.floor(x * zoom);
+				y = Math.floor(y * zoom);
+			}
 
 			popup(menu, {
-				x: Math.floor(x),
-				y: Math.floor(y),
+				x: x,
+				y: y,
 				positioningItem: delegate.autoSelectFirstItem ? 0 : undefined,
 			}, () => onHide());