Merge pull request #13792 from desktop/refactor-spell-check-remote-usage-to-main-process

Refactor spell check menu remote usage to main process

app/src/lib/ipc-shared.ts

@@ -86,7 +86,8 @@ export type RequestResponseChannels = {
   'is-running-under-rosetta-translation': () => Promise<boolean>
   'move-to-trash': (path: string) => Promise<void>
   'show-contextual-menu': (
-    items: ReadonlyArray<ISerializableMenuItem>
+    items: ReadonlyArray<ISerializableMenuItem>,
+    addSpellCheckMenu: boolean
   ) => Promise<ReadonlyArray<number> | null>
   'is-window-focused': () => Promise<boolean>
   'open-external': (path: string) => Promise<boolean>

app/src/lib/menu-item.ts

@@ -1,3 +1,5 @@
+import { invokeContextualMenu } from '../ui/main-process-proxy'
+
 export interface IMenuItem {
   /** The user-facing label. */
   readonly label?: string
@@ -72,3 +74,58 @@ export function getPlatformSpecificNameOrSymbolForModifier(
   // Not a known modifier, likely a normal key
   return modifier
 }
+
+/** Show the given menu items in a contextual menu. */
+export async function showContextualMenu(
+  items: ReadonlyArray<IMenuItem>,
+  addSpellCheckMenu = false
+) {
+  const indices = await invokeContextualMenu(
+    serializeMenuItems(items),
+    addSpellCheckMenu
+  )
+
+  if (indices !== null) {
+    const menuItem = findSubmenuItem(items, indices)
+
+    if (menuItem !== undefined && menuItem.action !== undefined) {
+      menuItem.action()
+    }
+  }
+}
+
+/**
+ * Remove the menu items properties that can't be serializable in
+ * order to pass them via IPC.
+ */
+function serializeMenuItems(
+  items: ReadonlyArray<IMenuItem>
+): ReadonlyArray<ISerializableMenuItem> {
+  return items.map(item => ({
+    ...item,
+    action: undefined,
+    submenu: item.submenu ? serializeMenuItems(item.submenu) : undefined,
+  }))
+}
+
+/**
+ * Traverse the submenus of the context menu until we find the appropriate index.
+ */
+function findSubmenuItem(
+  currentContextualMenuItems: ReadonlyArray<IMenuItem>,
+  indices: ReadonlyArray<number>
+): IMenuItem | undefined {
+  let foundMenuItem: IMenuItem | undefined = {
+    submenu: currentContextualMenuItems,
+  }
+
+  for (const index of indices) {
+    if (foundMenuItem === undefined || foundMenuItem.submenu === undefined) {
+      return undefined
+    }
+
+    foundMenuItem = foundMenuItem.submenu[index]
+  }
+
+  return foundMenuItem
+}

app/src/main-process/main.ts

@@ -37,6 +37,7 @@ import { installSameOriginFilter } from './same-origin-filter'
 import * as ipcMain from './ipc-main'
 import { getArchitecture } from '../lib/get-architecture'
 import * as remoteMain from '@electron/remote/main'
+import { buildSpellCheckMenu } from './menu/build-spell-check-menu'
 remoteMain.initialize()
 
 app.setAppLogsPath()
@@ -450,14 +451,24 @@ app.on('ready', () => {
    * Handle the action to show a contextual menu.
    *
    * It responds an array of indices that maps to the path to reach
-   * the menu (or submenu) item that was clicked or null if the menu
-   * was closed without clicking on any item.
+   * the menu (or submenu) item that was clicked or null if the menu was closed
+   * without clicking on any item or the item click was handled by the main
+   * process as opposed to the renderer.
    */
-  ipcMain.handle('show-contextual-menu', (event, items) => {
-    return new Promise(resolve => {
-      const menu = buildContextMenu(items, indices => resolve(indices))
+  ipcMain.handle('show-contextual-menu', (event, items, addSpellCheckMenu) => {
+    return new Promise(async resolve => {
       const window = BrowserWindow.fromWebContents(event.sender) || undefined
 
+      const spellCheckMenuItems = addSpellCheckMenu
+        ? await buildSpellCheckMenu(window)
+        : undefined
+
+      const menu = buildContextMenu(
+        items,
+        indices => resolve(indices),
+        spellCheckMenuItems
+      )
+
       menu.popup({ window, callback: () => resolve(null) })
     })
   })

app/src/main-process/menu/build-context-menu.ts

@@ -48,9 +48,20 @@ function getEditMenuItems(): ReadonlyArray<MenuItem> {
  */
 export function buildContextMenu(
   template: ReadonlyArray<ISerializableMenuItem>,
-  onClick: (indices: ReadonlyArray<number>) => void
+  onClick: (indices: ReadonlyArray<number>) => void,
+  spellCheckMenuItems?: ReadonlyArray<MenuItem>
 ): Menu {
-  return buildRecursiveContextMenu(template, onClick)
+  const menu = buildRecursiveContextMenu(template, onClick)
+
+  if (spellCheckMenuItems === undefined) {
+    return menu
+  }
+
+  for (const spellCheckMenuItem of spellCheckMenuItems) {
+    menu.append(spellCheckMenuItem)
+  }
+
+  return menu
 }
 
 function buildRecursiveContextMenu(

app/src/main-process/menu/build-spell-check-menu.ts

@@ -0,0 +1,111 @@
+import { app, BrowserWindow, MenuItem } from 'electron'
+
+export async function buildSpellCheckMenu(
+  window: BrowserWindow | undefined
+): Promise<ReadonlyArray<MenuItem> | undefined> {
+  if (window === undefined) {
+    return
+  }
+
+  /*
+    When a user right clicks on a misspelled word in an input, we get event from
+    electron. That event comes after the context menu event that we get from the
+    dom.
+  */
+  return new Promise(resolve => {
+    window.webContents.once('context-menu', (event, params) =>
+      resolve(getSpellCheckMenuItems(event, params, window.webContents))
+    )
+  })
+}
+
+function getSpellCheckMenuItems(
+  event: Electron.Event,
+  params: Electron.ContextMenuParams,
+  webContents: Electron.WebContents
+): ReadonlyArray<MenuItem> | undefined {
+  const { misspelledWord, dictionarySuggestions } = params
+  if (!misspelledWord && dictionarySuggestions.length === 0) {
+    return
+  }
+
+  const items = new Array<MenuItem>()
+
+  items.push(
+    new MenuItem({
+      type: 'separator',
+    })
+  )
+
+  for (const suggestion of dictionarySuggestions) {
+    items.push(
+      new MenuItem({
+        label: suggestion,
+        click: () => webContents.replaceMisspelling(suggestion),
+      })
+    )
+  }
+
+  if (misspelledWord) {
+    items.push(
+      new MenuItem({
+        label: __DARWIN__ ? 'Add to Dictionary' : 'Add to dictionary',
+        click: () =>
+          webContents.session.addWordToSpellCheckerDictionary(misspelledWord),
+      })
+    )
+  }
+
+  if (!__DARWIN__) {
+    // NOTE: "On macOS as we use the native APIs there is no way to set the
+    // language that the spellchecker uses" -- electron docs Therefore, we are
+    // only allowing setting to English for non-mac machines.
+    const spellCheckLanguageItem = getSpellCheckLanguageMenuItem(
+      webContents.session
+    )
+    if (spellCheckLanguageItem !== null) {
+      items.push(spellCheckLanguageItem)
+    }
+  }
+
+  return items
+}
+
+/**
+ * Method to get a menu item to give user the option to use English or their
+ * system language.
+ *
+ * If system language is english, it returns null. If spellchecker is not set to
+ * english, it returns item that can set it to English. If spellchecker is set
+ * to english, it returns the item that can set it to their system language.
+ */
+function getSpellCheckLanguageMenuItem(
+  session: Electron.session
+): MenuItem | null {
+  const userLanguageCode = app.getLocale()
+  const englishLanguageCode = 'en-US'
+  const spellcheckLanguageCodes = session.getSpellCheckerLanguages()
+
+  if (
+    userLanguageCode === englishLanguageCode &&
+    spellcheckLanguageCodes.includes(englishLanguageCode)
+  ) {
+    return null
+  }
+
+  const languageCode =
+    spellcheckLanguageCodes.includes(englishLanguageCode) &&
+    !spellcheckLanguageCodes.includes(userLanguageCode)
+      ? userLanguageCode
+      : englishLanguageCode
+
+  const label =
+    languageCode === englishLanguageCode
+      ? 'Set spellcheck to English'
+      : 'Set spellcheck to system language'
+
+  return new MenuItem({
+    label,
+    click: () => session.setSpellCheckerLanguages([languageCode]),
+  })
+}

app/src/ui/autocompletion/autocompleting-text-input.tsx

@@ -15,7 +15,7 @@ interface IRange {
 }
 
 import getCaretCoordinates from 'textarea-caret'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 
 interface IAutocompletingTextInputProps<ElementType> {
   /**

app/src/ui/branches/branch-list-item.tsx

@@ -6,7 +6,7 @@ import { IMatches } from '../../lib/fuzzy-find'
 import { Octicon } from '../octicons'
 import * as OcticonSymbol from '../octicons/octicons.generated'
 import { HighlightText } from '../lib/highlight-text'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 import { IMenuItem } from '../../lib/menu-item'
 import { dragAndDropManager } from '../../lib/drag-and-drop-manager'
 import { DragType, DropTargetType } from '../../models/drag-drop'

app/src/ui/changes/changes-list.tsx

@@ -27,7 +27,7 @@ import {
 import { CommitMessage } from './commit-message'
 import { ChangedFile } from './changed-file'
 import { IAutocompletionProvider } from '../autocompletion'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 import { arrayEquals } from '../../lib/equality'
 import { clipboard } from 'electron'
 import { basename } from 'path'

app/src/ui/changes/commit-message.tsx

@@ -27,7 +27,7 @@ import { CommitWarning, CommitWarningIcon } from './commit-warning'
 import { LinkButton } from '../lib/link-button'
 import { FoldoutType } from '../../lib/app-state'
 import { IAvatarUser, getAvatarUserFromAuthor } from '../../models/avatar'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 import { Account } from '../../models/account'
 import { CommitMessageAvatar } from './commit-message-avatar'
 import { getDotComAPIEndpoint } from '../../lib/api'

app/src/ui/diff/side-by-side-diff.tsx

@@ -46,7 +46,7 @@ import {
   getLineWidthFromDigitCount,
   getNumberOfDigits,
 } from './diff-helpers'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 import { getTokens } from './diff-syntax-mode'
 import { DiffSearchInput } from './diff-search-input'
 import { escapeRegExp } from '../../lib/helpers/regex'

app/src/ui/diff/text-diff.tsx

@@ -40,7 +40,7 @@ import { structuralEquals } from '../../lib/equality'
 import { assertNever } from '../../lib/fatal-error'
 import { clamp } from '../../lib/clamp'
 import { uuid } from '../../lib/uuid'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 import { IMenuItem } from '../../lib/menu-item'
 import {
   canSelect,

app/src/ui/history/commit-list-item.tsx

@@ -6,7 +6,7 @@ import { RichText } from '../lib/rich-text'
 import { RelativeTime } from '../relative-time'
 import { getDotComAPIEndpoint } from '../../lib/api'
 import { clipboard } from 'electron'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 import { CommitAttribution } from '../lib/commit-attribution'
 import { AvatarStack } from '../lib/avatar-stack'
 import { IMenuItem } from '../../lib/menu-item'

app/src/ui/history/selected-commit.tsx

@@ -23,7 +23,7 @@ import { ThrottledScheduler } from '../lib/throttled-scheduler'
 
 import { Dispatcher } from '../dispatcher'
 import { Resizable } from '../resizable'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 
 import { CommitSummary } from './commit-summary'
 import { FileList } from './file-list'

app/src/ui/lib/author-input.tsx

@@ -13,7 +13,7 @@ import { arrayEquals } from '../../lib/equality'
 import { syncClockwise } from '../octicons'
 import * as OcticonSymbol from '../octicons/octicons.generated'
 import { IAuthor } from '../../models/author'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 import { IMenuItem } from '../../lib/menu-item'
 import { getLegacyStealthEmailForUser } from '../../lib/email'
 

app/src/ui/lib/conflicts/unmerged-file.tsx

@@ -10,7 +10,7 @@ import {
 import { join } from 'path'
 import { Repository } from '../../../models/repository'
 import { Dispatcher } from '../../dispatcher'
-import { showContextualMenu } from '../../main-process-proxy'
+import { showContextualMenu } from '../../../lib/menu-item'
 import { Octicon } from '../../octicons'
 import * as OcticonSymbol from '../../octicons/octicons.generated'
 import { PathText } from '../path-text'

app/src/ui/lib/text-area.tsx

@@ -1,6 +1,6 @@
 import * as React from 'react'
 import classNames from 'classnames'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 
 interface ITextAreaProps {
   /** The label for the textarea field. */

app/src/ui/lib/text-box.tsx

@@ -2,7 +2,7 @@ import * as React from 'react'
 import classNames from 'classnames'
 import { createUniqueId, releaseUniqueId } from './id-pool'
 import { LinkButton } from './link-button'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 
 export interface ITextBoxProps {
   /** The label for the input field. */

app/src/ui/main-process-proxy.ts

@@ -1,6 +1,4 @@
-import * as remote from '@electron/remote'
 import { ExecutableMenuItem } from '../models/app-menu'
-import { IMenuItem, ISerializableMenuItem } from '../lib/menu-item'
 import { RequestResponseChannels, RequestChannels } from '../lib/ipc-shared'
 import * as ipcRenderer from '../lib/ipc-renderer'
 
@@ -215,176 +213,7 @@ export const moveToApplicationsFolder = sendProxy(
  */
 export const getAppMenu = sendProxy('get-app-menu', 0)
 
-function findSubmenuItem(
-  currentContextualMenuItems: ReadonlyArray<IMenuItem>,
-  indices: ReadonlyArray<number>
-): IMenuItem | undefined {
-  let foundMenuItem: IMenuItem | undefined = {
-    submenu: currentContextualMenuItems,
-  }
-
-  // Traverse the submenus of the context menu until we find the appropriate index.
-  for (const index of indices) {
-    if (foundMenuItem === undefined || foundMenuItem.submenu === undefined) {
-      return undefined
-    }
-
-    foundMenuItem = foundMenuItem.submenu[index]
-  }
-
-  return foundMenuItem
-}
-
-let deferredContextMenuItems: ReadonlyArray<IMenuItem> | null = null
-
-/** Takes a context menu and spelling suggestions from electron and merges them
- * into one context menu. */
-function mergeDeferredContextMenuItems(
-  event: Electron.Event,
-  params: Electron.ContextMenuParams
-) {
-  if (deferredContextMenuItems === null) {
-    return
-  }
-
-  const items = [...deferredContextMenuItems]
-  const { misspelledWord, dictionarySuggestions } = params
-
-  if (!misspelledWord && dictionarySuggestions.length === 0) {
-    showContextualMenu(items, false)
-    return
-  }
-
-  items.push({ type: 'separator' })
-
-  const { webContents } = remote.getCurrentWindow()
-
-  for (const suggestion of dictionarySuggestions) {
-    items.push({
-      label: suggestion,
-      action: () => webContents.replaceMisspelling(suggestion),
-    })
-  }
-
-  if (misspelledWord) {
-    items.push({
-      label: __DARWIN__ ? 'Add to Dictionary' : 'Add to dictionary',
-      action: () =>
-        webContents.session.addWordToSpellCheckerDictionary(misspelledWord),
-    })
-  }
-
-  if (!__DARWIN__) {
-    // NOTE: "On macOS as we use the native APIs there is no way to set the
-    // language that the spellchecker uses" -- electron docs Therefore, we are
-    // only allowing setting to English for non-mac machines.
-    const spellCheckLanguageItem = getSpellCheckLanguageMenuItem(
-      webContents.session
-    )
-    if (spellCheckLanguageItem !== null) {
-      items.push(spellCheckLanguageItem)
-    }
-  }
-
-  showContextualMenu(items, false)
-}
-
-/**
- * Method to get a menu item to give user the option to use English or their
- * system language.
- *
- * If system language is english, it returns null. If spellchecker is not set to
- * english, it returns item that can set it to English. If spellchecker is set
- * to english, it returns the item that can set it to their system language.
- */
-function getSpellCheckLanguageMenuItem(
-  session: Electron.session
-): IMenuItem | null {
-  const userLanguageCode = remote.app.getLocale()
-  const englishLanguageCode = 'en-US'
-  const spellcheckLanguageCodes = session.getSpellCheckerLanguages()
-
-  if (
-    userLanguageCode === englishLanguageCode &&
-    spellcheckLanguageCodes.includes(englishLanguageCode)
-  ) {
-    return null
-  }
-
-  const languageCode =
-    spellcheckLanguageCodes.includes(englishLanguageCode) &&
-    !spellcheckLanguageCodes.includes(userLanguageCode)
-      ? userLanguageCode
-      : englishLanguageCode
-
-  const label =
-    languageCode === englishLanguageCode
-      ? 'Set spellcheck to English'
-      : 'Set spellcheck to system language'
-
-  return {
-    label,
-    action: () => session.setSpellCheckerLanguages([languageCode]),
-  }
-}
-
-const _showContextualMenu = invokeProxy('show-contextual-menu', 1)
-
-/** Show the given menu items in a contextual menu. */
-export async function showContextualMenu(
-  items: ReadonlyArray<IMenuItem>,
-  mergeWithSpellcheckSuggestions = false
-) {
-  /*
-    When a user right clicks on a misspelled word in an input, we get event from
-    electron. That event comes after the context menu event that we get from the
-    dom. In order merge the spelling suggestions from electron with the context
-    menu that the input wants to show, we stash the context menu items from the
-    input away while we wait for the event from electron.
-  */
-  if (deferredContextMenuItems !== null) {
-    deferredContextMenuItems = null
-    remote
-      .getCurrentWebContents()
-      .off('context-menu', mergeDeferredContextMenuItems)
-  }
-
-  if (mergeWithSpellcheckSuggestions) {
-    deferredContextMenuItems = items
-    remote
-      .getCurrentWebContents()
-      .once('context-menu', mergeDeferredContextMenuItems)
-    return
-  }
-
-  /*
-  This is a regular context menu that does not need to merge with spellcheck
-  items. They can be shown right away.
-  */
-  const indices = await _showContextualMenu(serializeMenuItems(items))
-
-  if (indices !== null) {
-    const menuItem = findSubmenuItem(items, indices)
-
-    if (menuItem !== undefined && menuItem.action !== undefined) {
-      menuItem.action()
-    }
-  }
-}
-
-/**
- * Remove the menu items properties that can't be serializable in
- * order to pass them via IPC.
- */
-function serializeMenuItems(
-  items: ReadonlyArray<IMenuItem>
-): ReadonlyArray<ISerializableMenuItem> {
-  return items.map(item => ({
-    ...item,
-    action: undefined,
-    submenu: item.submenu ? serializeMenuItems(item.submenu) : undefined,
-  }))
-}
+export const invokeContextualMenu = invokeProxy('show-contextual-menu', 2)
 
 /** Update the menu item labels with the user's preferred apps. */
 export const updatePreferredAppMenuItemLabels = sendProxy(

app/src/ui/repositories-list/repositories-list.tsx

@@ -16,7 +16,7 @@ import { Dispatcher } from '../dispatcher'
 import { Button } from '../lib/button'
 import { Octicon } from '../octicons'
 import * as OcticonSymbol from '../octicons/octicons.generated'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 import { IMenuItem } from '../../lib/menu-item'
 import { PopupType } from '../../models/popup'
 import { encodePathAsUrl } from '../../lib/path'

app/src/ui/repositories-list/repository-list-item.tsx

@@ -4,7 +4,7 @@ import { clipboard } from 'electron'
 import { Repository } from '../../models/repository'
 import { Octicon, iconForRepository } from '../octicons'
 import * as OcticonSymbol from '../octicons/octicons.generated'
-import { showContextualMenu } from '../main-process-proxy'
+import { showContextualMenu } from '../../lib/menu-item'
 import { Repositoryish } from './group-repositories'
 import { IMenuItem } from '../../lib/menu-item'
 import { HighlightText } from '../lib/highlight-text'