From 9ff16d7e60e591a1540acbbea51d92dbc083e509 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 27 Mar 2020 16:43:19 +1100 Subject: [PATCH] move to electron spellchecker --- _locales/en/messages.json | 10 ++ app/spell_check.js | 88 ++++++++++++++++++ integration_test/common.js | 1 - js/background.js | 19 +--- js/spell_check.js | 185 ------------------------------------- main.js | 22 ++++- preload.js | 35 ++----- test/index.html | 1 - test/spellcheck_test.js | 15 --- 9 files changed, 131 insertions(+), 245 deletions(-) create mode 100644 app/spell_check.js delete mode 100644 js/spell_check.js delete mode 100644 test/spellcheck_test.js diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 25be3b73b..96aec291b 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -166,6 +166,11 @@ "description": "Only available on development modes, menu option to open up the standalone device setup sequence" }, + "contextMenuNoSuggestions": { + "message": "No Suggestions", + "description": + "Shown in the context menu for a misspelled word to indicate that there are no suggestions to replace the misspelled word" + }, "connectingLoad": { "message": "Connecting To Server", "description": @@ -1414,6 +1419,11 @@ "message": "Enable spell check of text entered in message composition box", "description": "Description of the media permission description" }, + "spellCheckDirty": { + "message": "You must restart Session to apply your new settings", + "description": + "Shown when the user changes their spellcheck setting to indicate that they must restart Signal." + }, "clearDataHeader": { "message": "Clear All Local Data", "description": diff --git a/app/spell_check.js b/app/spell_check.js new file mode 100644 index 000000000..695c171a6 --- /dev/null +++ b/app/spell_check.js @@ -0,0 +1,88 @@ +/* global exports, require */ +/* eslint-disable strict */ + +const { Menu } = require('electron'); +const osLocale = require('os-locale'); + +exports.setup = (browserWindow, messages) => { + const { session } = browserWindow.webContents; + const userLocale = osLocale.sync().replace(/_/g, '-'); + const userLocales = [userLocale, userLocale.split('-')[0]]; + + const available = session.availableSpellCheckerLanguages; + const languages = userLocales.filter(l => available.includes(l)); + console.log(`spellcheck: user locale: ${userLocale}`); + console.log('spellcheck: available spellchecker languages: ', available); + console.log('spellcheck: setting languages to: ', languages); + session.setSpellCheckerLanguages(languages); + + browserWindow.webContents.on('context-menu', (_event, params) => { + const { editFlags } = params; + const isMisspelled = Boolean(params.misspelledWord); + const showMenu = params.isEditable || editFlags.canCopy; + + // Popup editor menu + if (showMenu) { + const template = []; + + if (isMisspelled) { + if (params.dictionarySuggestions.length > 0) { + template.push( + ...params.dictionarySuggestions.map(label => ({ + label, + click: () => { + browserWindow.webContents.replaceMisspelling(label); + }, + })) + ); + } else { + template.push({ + label: messages.contextMenuNoSuggestions.message, + enabled: false, + }); + } + template.push({ type: 'separator' }); + } + + if (params.isEditable) { + if (editFlags.canUndo) { + template.push({ label: messages.editMenuUndo.message, role: 'undo' }); + } + // This is only ever `true` if undo was triggered via the context menu + // (not ctrl/cmd+z) + if (editFlags.canRedo) { + template.push({ label: messages.editMenuRedo.message, role: 'redo' }); + } + if (editFlags.canUndo || editFlags.canRedo) { + template.push({ type: 'separator' }); + } + if (editFlags.canCut) { + template.push({ label: messages.editMenuCut.message, role: 'cut' }); + } + } + + if (editFlags.canPaste) { + template.push({ label: messages.editMenuPaste.message, role: 'paste' }); + } + + if (editFlags.canPaste) { + template.push({ + label: messages.editMenuPasteAndMatchStyle.message, + role: 'pasteAndMatchStyle', + }); + } + + // Only enable select all in editors because select all in non-editors + // results in all the UI being selected + if (editFlags.canSelectAll && params.isEditable) { + template.push({ + label: messages.editMenuSelectAll.message, + role: 'selectall', + }); + } + + const menu = Menu.buildFromTemplate(template); + menu.popup(browserWindow); + } + }); +}; diff --git a/integration_test/common.js b/integration_test/common.js index 0a2daca5b..cda7f29e8 100644 --- a/integration_test/common.js +++ b/integration_test/common.js @@ -67,7 +67,6 @@ module.exports = { ELECTRON_ENABLE_STACK_DUMPING: true, ELECTRON_DISABLE_SANDBOX: 1, }, - startTimeout: 10000, requireName: 'electronRequire', // chromeDriverLogPath: '../chromedriverlog.txt', chromeDriverArgs: [ diff --git a/js/background.js b/js/background.js index 60b74b6ca..4ee9fe0da 100644 --- a/js/background.js +++ b/js/background.js @@ -320,7 +320,6 @@ getSpellCheck: () => storage.get('spell-check', true), setSpellCheck: value => { storage.put('spell-check', value); - startSpellCheck(); }, addDarkOverlay: () => { @@ -419,19 +418,6 @@ } }); - const startSpellCheck = () => { - if (!window.enableSpellCheck || !window.disableSpellCheck) { - return; - } - - if (window.Events.getSpellCheck()) { - window.enableSpellCheck(); - } else { - window.disableSpellCheck(); - } - }; - startSpellCheck(); - const themeSetting = window.Events.getThemeSetting(); const newThemeSetting = mapOldThemeToNew(themeSetting); window.Events.setThemeSetting(newThemeSetting); @@ -1039,6 +1025,11 @@ window.toggleSpellCheck = () => { const newValue = !window.getSettingValue('spell-check'); window.Events.setSpellCheck(newValue); + window.pushToast({ + description: window.i18n('spellCheckDirty'), + type: 'info', + id: 'spellCheckDirty', + }); }; window.toggleLinkPreview = () => { diff --git a/js/spell_check.js b/js/spell_check.js deleted file mode 100644 index 9878ab438..000000000 --- a/js/spell_check.js +++ /dev/null @@ -1,185 +0,0 @@ -/* global require, process, _ */ - -/* eslint-disable strict */ - -const electron = require('electron'); - -const Typo = require('typo-js'); -const fs = require('fs'); -const osLocale = require('os-locale'); -const path = require('path'); - -const { remote, webFrame } = electron; - -// `remote.require` since `Menu` is a main-process module. -const buildEditorContextMenu = remote.require('electron-editor-context-menu'); - -const EN_VARIANT = /^en/; - -// Prevent the spellchecker from showing contractions as errors. -const ENGLISH_SKIP_WORDS = [ - 'ain', - 'couldn', - 'didn', - 'doesn', - 'hadn', - 'hasn', - 'mightn', - 'mustn', - 'needn', - 'oughtn', - 'shan', - 'shouldn', - 'wasn', - 'weren', - 'wouldn', -]; - -function setupLinux(locale) { - if (process.env.HUNSPELL_DICTIONARIES || locale !== 'en_US') { - // apt-get install hunspell- can be run for easy access - // to other dictionaries - const location = process.env.HUNSPELL_DICTIONARIES || '/usr/share/hunspell'; - const affDataPath = path.join(location, `${locale}.aff`); - const dicDataPath = path.join(location, `${locale}.dic`); - - window.log.info( - 'Detected Linux. Setting up spell check with locale', - locale, - 'and dictionary location', - location - ); - - if (fs.existsSync(affDataPath) && fs.existsSync(dicDataPath)) { - const affData = fs.readFileSync(affDataPath, 'utf-8'); - const dicData = fs.readFileSync(dicDataPath, 'utf-8'); - - return new Typo(locale, affData, dicData); - } - - window.log.error( - `Could not find one of ${affDataPath} or ${dicDataPath} on filesystem` - ); - } - - window.log.info('Detected Linux. Using default en_US spell check dictionary'); - - return new Typo(locale); -} - -// We load locale this way and not via app.getLocale() because this call returns -// 'es_ES' and not just 'es.' And hunspell requires the fully-qualified locale. -const locale = osLocale.sync().replace('-', '_'); - -// The LANG environment variable is how node spellchecker finds its default language: -// https://github.com/atom/node-spellchecker/blob/59d2d5eee5785c4b34e9669cd5d987181d17c098/lib/spellchecker.js#L29 -if (!process.env.LANG) { - process.env.LANG = locale; -} - -let spellchecker = null; - -if (process.platform === 'linux') { - spellchecker = setupLinux(locale); -} else { - spellchecker = new Typo(locale); - // OSX and Windows 8+ have OS-level spellcheck APIs - window.log.info( - 'Using OS-level spell check API with locale', - process.env.LANG - ); -} - -const simpleChecker = { - spellCheck(words, callback) { - let mispelled; - if (Array.isArray(words)) { - mispelled = words.filter(word => this.isMisspelled(word)); - } else { - mispelled = this.isMisspelled(words); - } - - callback(mispelled); - }, - isMisspelled(word) { - const misspelled = !spellchecker.check(word); - - // The idea is to make this as fast as possible. For the many, many calls which - // don't result in the red squiggly, we minimize the number of checks. - if (!misspelled) { - return false; - } - - // Only if we think we've found an error do we check the locale and skip list. - if (locale.match(EN_VARIANT) && _.contains(ENGLISH_SKIP_WORDS, word)) { - return false; - } - - return true; - }, - getSuggestions(text) { - return spellchecker.suggest(text); - }, - add() {}, -}; - -const dummyChecker = { - spellCheck(words, callback) { - callback([]); - }, - isMisspelled() { - return false; - }, - getSuggestions() { - return []; - }, - add() { - // nothing - }, -}; - -window.spellChecker = simpleChecker; -window.disableSpellCheck = () => { - window.removeEventListener('contextmenu', spellCheckHandler); - window.addEventListener('contextmenu', defaultContextMenuHandler); - webFrame.setSpellCheckProvider('en-US', dummyChecker); -}; - -window.enableSpellCheck = () => { - webFrame.setSpellCheckProvider('en-US', simpleChecker); - window.addEventListener('contextmenu', spellCheckHandler); - window.removeEventListener('contextmenu', defaultContextMenuHandler); -}; - -const defaultContextMenuHandler = () => { - const menu = buildEditorContextMenu({}); - - // @see js/spell_check.js:183 - setTimeout(() => { - menu.popup(remote.getCurrentWindow()); - }, 30); -}; - -const spellCheckHandler = e => { - // Only show the context menu in text editors. - if (!e.target.closest('textarea, input, [contenteditable="true"]')) { - return; - } - - const selectedText = window.getSelection().toString(); - const isMisspelled = selectedText && simpleChecker.isMisspelled(selectedText); - const spellingSuggestions = - isMisspelled && simpleChecker.getSuggestions(selectedText).slice(0, 5); - const menu = buildEditorContextMenu({ - isMisspelled, - spellingSuggestions, - }); - - // The 'contextmenu' event is emitted after 'selectionchange' has fired - // but possibly before the visible selection has changed. Try to wait - // to show the menu until after that, otherwise the visible selection - // will update after the menu dismisses and look weird. - setTimeout(() => { - menu.popup(remote.getCurrentWindow()); - }, 30); -}; diff --git a/main.js b/main.js index 092fd52a4..ca2a1558a 100644 --- a/main.js +++ b/main.js @@ -9,7 +9,7 @@ const crypto = require('crypto'); const _ = require('lodash'); const pify = require('pify'); const electron = require('electron'); - +const { setup: setupSpellChecker } = require('./app/spell_check'); const packageJson = require('./package.json'); const GlobalErrors = require('./app/global_errors'); @@ -82,6 +82,18 @@ const { } = require('./app/protocol_filter'); const { installPermissionsHandler } = require('./app/permissions'); +let appStartInitialSpellcheckSetting = true; + +async function getSpellCheckSetting() { + const json = await sql.getItemById('spell-check'); + // Default to `true` if setting doesn't exist yet + if (!json) { + return true; + } + + return json.value; +} + function showWindow() { if (!mainWindow) { return; @@ -166,6 +178,7 @@ function prepareURL(pathSegments, moreKeys) { contentProxyUrl: config.contentProxyUrl, importMode: importMode ? true : undefined, // for stringify() serverTrustRoot: config.get('serverTrustRoot'), + appStartInitialSpellcheckSetting, defaultFileServer: config.get('defaultFileServer'), ...moreKeys, }, @@ -216,7 +229,7 @@ function isVisible(window, bounds) { ); } -function createWindow() { +async function createWindow() { const { screen } = electron; const windowOptions = Object.assign( { @@ -233,7 +246,7 @@ function createWindow() { contextIsolation: false, preload: path.join(__dirname, 'preload.js'), nativeWindowOpen: true, - spellcheck: false, + spellcheck: await getSpellCheckSetting(), }, icon: path.join(__dirname, 'images', 'session', 'icon_64.png'), }, @@ -284,6 +297,8 @@ function createWindow() { // Create the browser window. mainWindow = new BrowserWindow(windowOptions); + setupSpellChecker(mainWindow, locale.messages); + // Disable system main menu mainWindow.setMenu(null); @@ -775,6 +790,7 @@ async function showMainWindow(sqlKey, passwordAttempt = false) { messages: locale.messages, passwordAttempt, }); + appStartInitialSpellcheckSetting = await getSpellCheckSetting(); await sqlChannels.initialize(); try { diff --git a/preload.js b/preload.js index 9e3144c16..08c73ed67 100644 --- a/preload.js +++ b/preload.js @@ -376,34 +376,17 @@ window.Signal.Backup = require('./js/modules/backup'); window.Signal.Debug = require('./js/modules/debug'); window.Signal.Logs = require('./js/modules/logs'); -// Add right-click listener for selected text and urls -const contextMenu = require('electron-context-menu'); - -const isQR = params => - params.mediaType === 'image' && params.titleText === 'Scan me!'; - -// QR saving doesn't work so we just disable it -contextMenu({ - showInspectElement: false, - shouldShowMenu: (event, params) => { - const isRegular = - params.mediaType === 'none' && (params.linkURL || params.selectionText); - return Boolean(!params.isEditable && (isQR(params) || isRegular)); - }, - menu: (actions, params) => { - // If it's not a QR then show the default options - if (!isQR(params)) { - return actions; - } - - return [actions.copyImage()]; - }, +window.addEventListener('contextmenu', e => { + const editable = e.target.closest( + 'textarea, input, [contenteditable="true"]' + ); + const link = e.target.closest('a'); + const selection = Boolean(window.getSelection().toString()); + if (!editable && !selection && !link) { + e.preventDefault(); + } }); -// We pull this in last, because the native module involved appears to be sensitive to -// /tmp mounted as noexec on Linux. -require('./js/spell_check'); - window.shortenPubkey = pubkey => `(...${pubkey.substring(pubkey.length - 6)})`; window.pubkeyPattern = /@[a-fA-F0-9]{64,66}\b/g; diff --git a/test/index.html b/test/index.html index 54d1dd46f..13477e485 100644 --- a/test/index.html +++ b/test/index.html @@ -586,7 +586,6 @@ - diff --git a/test/spellcheck_test.js b/test/spellcheck_test.js deleted file mode 100644 index b51daa4e9..000000000 --- a/test/spellcheck_test.js +++ /dev/null @@ -1,15 +0,0 @@ -describe('spellChecker', () => { - it('should work', () => { - let result = null; - - window.spellChecker.spellCheck(['correct'], answer => { - result = answer; - }); - assert.deepEqual(result, []); - - window.spellChecker.spellCheck(['fhqwgads'], answer => { - result = answer; - }); - assert.deepEqual(result, ['fhqwgads']); - }); -});