Skip to content

fix(spell-check): stop service views crashing while typing #723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
manusa merged 2 commits into from
Sep 14, 2026
Merged

fix(spell-check): stop service views crashing while typing #723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d5172d
fix(spell-check): stop service views crashing while typing
manusa Sep 14, 2026
23bf821
test(spell-check): guard against an idle callback timeout
manusa Sep 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
175 changes: 133 additions & 42 deletions src/service-manager/__tests__/preload.spell-check.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,33 @@ describe('Browser Spell Check test suite', () => {
let electron;
let settings;
let browserSpellCheck;
let idleCallbacks;
let idleCallbackOptions;
const runIdleCallbacks = () => {
for (const idleCallback of idleCallbacks.splice(0)) {
idleCallback();
}
};
beforeEach(async () => {
jest.resetModules();
globalThis.APP_EVENTS = require('../../constants').APP_EVENTS;
// jsdom has no requestIdleCallback: queue the callbacks so each test decides when the renderer is idle
idleCallbacks = [];
idleCallbackOptions = [];
globalThis.requestIdleCallback = (idleCallback, options) => {
idleCallbacks.push(idleCallback);
idleCallbackOptions.push(options);
};
electron = require('../../__tests__').testElectron();
settings = await require('../../__tests__').testSettings();
electron.ipcMain.on('settingsLoad', settings.loadSettings);
browserSpellCheck = require('../preload.spell-check');
// Set the browser language to Esperanto
Object.defineProperty(navigator, 'language', {value: 'eo'});
});
afterEach(() => {
delete globalThis.requestIdleCallback;
});
describe('initSpellChecker', () => {
test('not-native, should load settings and set SpellCheckProvider in webFrame for navigator language', async () => {
// Given
Expand Down Expand Up @@ -62,49 +79,123 @@ describe('Browser Spell Check test suite', () => {
expect(electron.ipcRenderer.invoke).toHaveBeenCalledWith('settingsLoad');
});
});
test('spellCheck, should invoke dictionaryGetMisspelled and trigger callback', async () => {
// Given
const dictionaryGetMisspelled = jest.fn();
electron.ipcMain.on('dictionaryGetMisspelled', dictionaryGetMisspelled);
browserSpellCheck.initSpellChecker();
await waitFor(() => expect(electron.webFrame.setSpellCheckProvider).toHaveBeenCalledTimes(1));
const spellCheckCallback = jest.fn();
// When
await electron.webFrame.spellCheckProviders.eo.spellCheck([], spellCheckCallback);
// Then
expect(dictionaryGetMisspelled).toHaveBeenCalledWith([]);
expect(spellCheckCallback).toHaveBeenCalledTimes(1);
});
test('spellCheck, should trigger callback even if dictionaryGetMisspelled fails', async () => {
// Given
browserSpellCheck.initSpellChecker();
await waitFor(() => expect(electron.webFrame.setSpellCheckProvider).toHaveBeenCalledTimes(1));
electron.ipcRenderer.invoke = jest.fn(async () => {
throw new Error('Script failed to execute');
describe('spellCheck', () => {
let dictionaryGetMisspelled;
let spellCheck;
beforeEach(async () => {
dictionaryGetMisspelled = jest.fn(async words => words.filter(word => word.startsWith('mis')));
electron.ipcMain.handle('dictionaryGetMisspelled', dictionaryGetMisspelled);
browserSpellCheck.initSpellChecker();
await waitFor(() => expect(electron.webFrame.setSpellCheckProvider).toHaveBeenCalledTimes(1));
({spellCheck} = electron.webFrame.spellCheckProviders.eo);
});
const spellCheckCallback = jest.fn();
// When
await electron.webFrame.spellCheckProviders.eo.spellCheck(['word'], spellCheckCallback);
// Then
// Blink leaves the request pending forever if the callback never runs
await waitFor(() => expect(spellCheckCallback).toHaveBeenCalledWith([]));
});
test('spellCheck, should answer a request only once even if the callback throws', async () => {
// Given
const dictionaryGetMisspelled = jest.fn();
electron.ipcMain.on('dictionaryGetMisspelled', dictionaryGetMisspelled);
browserSpellCheck.initSpellChecker();
await waitFor(() => expect(electron.webFrame.setSpellCheckProvider).toHaveBeenCalledTimes(1));
const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {});
const spellCheckCallback = jest.fn(() => {
throw new Error('Blink blew up');
describe('with a single request', () => {
let callback;
beforeEach(async () => {
callback = jest.fn();
spellCheck(['correct', 'misspelled'], callback);
await waitFor(() => expect(idleCallbacks).toHaveLength(1));
});
test('sends the words to the dictionary', () => {
expect(dictionaryGetMisspelled).toHaveBeenCalledWith(['correct', 'misspelled']);
});
test('does not answer before the renderer is idle', () => {
expect(callback).not.toHaveBeenCalled();
});
// A timed-out idle callback runs as a regular task, which could beat Electron's hand-over of a
// newer request and answer a superseded one
test('waits for the renderer to be idle without a timeout', () => {
expect(idleCallbackOptions[0]?.timeout).toBeUndefined();
});
test('answers with the misspelled words once the renderer is idle', () => {
runIdleCallbacks();
expect(callback).toHaveBeenCalledExactlyOnceWith(['misspelled']);
});
});
// Answering a superseded request crashes the renderer: Electron completes the newer request with
// it, and the newer request's own answer then finds nothing pending
describe('with a request superseded while its dictionary lookup is in flight', () => {
let supersededCallback;
let newerCallback;
beforeEach(async () => {
let resolveSuperseded;
dictionaryGetMisspelled.mockImplementationOnce(() => new Promise(resolve => {
resolveSuperseded = resolve;
}));
supersededCallback = jest.fn();
newerCallback = jest.fn();
spellCheck(['misspelled'], supersededCallback);
spellCheck(['correct', 'mistake'], newerCallback);
await waitFor(() => expect(idleCallbacks).toHaveLength(1));
resolveSuperseded(['misspelled']);
await waitFor(() => expect(idleCallbacks).toHaveLength(2));
runIdleCallbacks();
});
test('never answers the superseded request', () => {
expect(supersededCallback).not.toHaveBeenCalled();
});
test('answers the newer request', () => {
expect(newerCallback).toHaveBeenCalledExactlyOnceWith(['mistake']);
});
});
// Electron posts a task to hand a request over, so Blink may have issued a newer request while the
// answer to the previous one is waiting for the renderer to be idle
describe('with a request superseded while its answer waits for the renderer to be idle', () => {
let supersededCallback;
let newerCallback;
beforeEach(async () => {
supersededCallback = jest.fn();
newerCallback = jest.fn();
spellCheck(['misspelled'], supersededCallback);
await waitFor(() => expect(idleCallbacks).toHaveLength(1));
spellCheck(['correct', 'mistake'], newerCallback);
await waitFor(() => expect(idleCallbacks).toHaveLength(2));
runIdleCallbacks();
});
test('never answers the superseded request', () => {
expect(supersededCallback).not.toHaveBeenCalled();
});
test('answers the newer request', () => {
expect(newerCallback).toHaveBeenCalledExactlyOnceWith(['mistake']);
});
});
describe('with a dictionary that fails', () => {
let callback;
beforeEach(async () => {
dictionaryGetMisspelled.mockImplementation(async () => {
throw new Error('Script failed to execute');
});
callback = jest.fn();
spellCheck(['misspelled'], callback);
await waitFor(() => expect(idleCallbacks).toHaveLength(1));
runIdleCallbacks();
});
// Blink leaves the request pending forever if the callback never runs
test('answers with nothing misspelled', () => {
expect(callback).toHaveBeenCalledExactlyOnceWith([]);
});
});
describe('with a callback that throws', () => {
let consoleError;
let callback;
beforeEach(async () => {
consoleError = jest.spyOn(console, 'error').mockImplementation(() => {});
callback = jest.fn(() => {
throw new Error('Blink blew up');
});
spellCheck(['misspelled'], callback);
await waitFor(() => expect(idleCallbacks).toHaveLength(1));
runIdleCallbacks();
});
afterEach(() => {
consoleError.mockRestore();
});
test('reports the error', () => {
expect(consoleError).toHaveBeenCalledWith('Spell check callback failed', expect.any(Error));
});
test('answers the request only once', () => {
expect(callback).toHaveBeenCalledTimes(1);
});
});
// When
await electron.webFrame.spellCheckProviders.eo.spellCheck(['word'], spellCheckCallback);
// Then
// Catching a throw from the callback would answer the very same request a second time
await waitFor(() => expect(consoleError).toHaveBeenCalledWith('Spell check callback failed', expect.any(Error)));
expect(spellCheckCallback).toHaveBeenCalledTimes(1);
consoleError.mockRestore();
});
});
37 changes: 30 additions & 7 deletions src/service-manager/preload.spell-check.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,39 @@
/* eslint-disable no-undef */
const {ipcRenderer, webFrame} = require('electron');

// Electron's SpellCheckClient tracks a single pending request, and dereferences it unchecked when a
// callback runs. Answering a request that is no longer the pending one completes the wrong request,
// and the next answer finds nothing pending: the renderer segfaults and the service turns blank.
// Blink supersedes requests routinely since Chromium 152 (Electron 44). It cancels its in-flight check
// whenever a script moves the caret in a field the user didn't focus themselves (Slack and Telegram
// focus their composers by script), and issues a new one on the next keystroke.
// So only the most recent request is ever answered, exactly once, and only when the renderer is idle.
// Electron hands a request over to this function from a task it posts, so Blink may have issued a
// request that hasn't reached it yet. That task has normal priority and always runs before an idle
// callback, which then finds its own request superseded. No timeout: a timed-out idle callback runs as
// a regular task and could beat Electron's hand-over.
let pendingRequest = null;

const spellCheckFunction = (words, callback) => {
const request = {};
pendingRequest = request;
const answer = misspelled => requestIdleCallback(() => {
if (pendingRequest !== request) {
return;
}
pendingRequest = null;
try {
callback(misspelled);
} catch (error) {
// Blink's side of the contract breaking. The request is no longer pending, so it can't be
// answered twice, only reported.
console.error('Spell check callback failed', error);
}
});
ipcRenderer.invoke(APP_EVENTS.dictionaryGetMisspelled, words)
// Blink keeps the request pending until the callback runs, so a rejection (the dictionary
// renderer failing to load, say) must still answer, reporting nothing as misspelled. The
// rejection handler is passed to then rather than chained through catch, which would also
// catch a throw from the callback itself and answer the same request a second time.
.then(misspelled => callback(misspelled), () => callback([]))
// Only ever reached if the callback itself threw, which is Blink's side of the contract
// breaking. Report it rather than leaving an unhandled rejection, and never answer again.
.catch(error => console.error('Spell check callback failed', error));
// renderer failing to load, say) must still answer, reporting nothing as misspelled.
.then(answer, () => answer([]));
};

const initSpellChecker = () => new Promise(resolve => {
Expand Down
Loading