diff --git a/browser/actors/ContextMenuChild.jsm b/browser/actors/ContextMenuChild.jsm index 26e2c90daf02..16a3f8ecbc81 100644 --- a/browser/actors/ContextMenuChild.jsm +++ b/browser/actors/ContextMenuChild.jsm @@ -1227,4 +1227,20 @@ class ContextMenuChild extends JSWindowActorChild { } } } + + _destructionObservers = new Set(); + registerDestructionObserver(obj) { + this._destructionObservers.add(obj); + } + + unregisterDestructionObserver(obj) { + this._destructionObservers.delete(obj); + } + + didDestroy() { + for (let obs of this._destructionObservers) { + obs.actorDestroyed(this); + } + this._destructionObservers = null; + } } diff --git a/browser/base/content/test/contextMenu/browser.ini b/browser/base/content/test/contextMenu/browser.ini index f715e426c39b..f0a208793d63 100644 --- a/browser/base/content/test/contextMenu/browser.ini +++ b/browser/base/content/test/contextMenu/browser.ini @@ -16,6 +16,8 @@ support-files = [browser_contextmenu_loadblobinnewtab.js] support-files = browser_contextmenu_loadblobinnewtab.html +[browser_contextmenu_spellcheck.js] +skip-if = toolkit == "gtk" || (os == "win" && processor == "aarch64") # disabled on Linux due to bug 513558, aarch64 due to 1533161 [browser_view_image.js] support-files = test_view_image_revoked_cached_blob.html diff --git a/browser/base/content/test/contextMenu/browser_contextmenu_input.js b/browser/base/content/test/contextMenu/browser_contextmenu_input.js index 6e30b0580f1b..94e9465fd58c 100644 --- a/browser/base/content/test/contextMenu/browser_contextmenu_input.js +++ b/browser/base/content/test/contextMenu/browser_contextmenu_input.js @@ -45,146 +45,6 @@ add_task(async function test_text_input() { ]); }); -add_task(async function test_text_input_spellcheck() { - await test_contextmenu( - "#input_spellcheck_no_value", - [ - "context-undo", - false, - "---", - null, - "context-cut", - true, - "context-copy", - true, - "context-paste", - null, // ignore clipboard state - "context-delete", - false, - "---", - null, - "context-selectall", - false, - "---", - null, - "spell-check-enabled", - true, - "spell-dictionaries", - true, - [ - "spell-check-dictionary-en-US", - true, - "---", - null, - "spell-add-dictionaries", - true, - ], - null, - ], - { - waitForSpellCheck: true, - async preCheckContextMenuFn() { - await SpecialPowers.spawn( - gBrowser.selectedBrowser, - [], - async function() { - let doc = content.document; - let input = doc.getElementById("input_spellcheck_no_value"); - input.setAttribute("spellcheck", "true"); - input.clientTop; // force layout flush - } - ); - }, - } - ); -}); - -add_task(async function test_text_input_spellcheckwrong() { - await test_contextmenu( - "#input_spellcheck_incorrect", - [ - "*prodigality", - true, // spelling suggestion - "spell-add-to-dictionary", - true, - "---", - null, - "context-undo", - false, - "---", - null, - "context-cut", - true, - "context-copy", - true, - "context-paste", - null, // ignore clipboard state - "context-delete", - false, - "---", - null, - "context-selectall", - true, - "---", - null, - "spell-check-enabled", - true, - "spell-dictionaries", - true, - [ - "spell-check-dictionary-en-US", - true, - "---", - null, - "spell-add-dictionaries", - true, - ], - null, - ], - { waitForSpellCheck: true } - ); -}); - -add_task(async function test_text_input_spellcheckcorrect() { - await test_contextmenu( - "#input_spellcheck_correct", - [ - "context-undo", - false, - "---", - null, - "context-cut", - true, - "context-copy", - true, - "context-paste", - null, // ignore clipboard state - "context-delete", - false, - "---", - null, - "context-selectall", - true, - "---", - null, - "spell-check-enabled", - true, - "spell-dictionaries", - true, - [ - "spell-check-dictionary-en-US", - true, - "---", - null, - "spell-add-dictionaries", - true, - ], - null, - ], - { waitForSpellCheck: true } - ); -}); - add_task(async function test_text_input_disabled() { await test_contextmenu( "#input_disabled", @@ -204,7 +64,7 @@ add_task(async function test_text_input_disabled() { "---", null, "context-selectall", - true, + false, "---", null, "spell-check-enabled", diff --git a/browser/base/content/test/contextMenu/browser_contextmenu_spellcheck.js b/browser/base/content/test/contextMenu/browser_contextmenu_spellcheck.js new file mode 100644 index 000000000000..3a150fecffad --- /dev/null +++ b/browser/base/content/test/contextMenu/browser_contextmenu_spellcheck.js @@ -0,0 +1,251 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +let contextMenu; + +const example_base = + "http://example.com/browser/browser/base/content/test/contextMenu/"; +const MAIN_URL = example_base + "subtst_contextmenu_input.html"; + +add_task(async function test_setup() { + await BrowserTestUtils.openNewForegroundTab(gBrowser, MAIN_URL); + + const chrome_base = + "chrome://mochitests/content/browser/browser/base/content/test/contextMenu/"; + const contextmenu_common = chrome_base + "contextmenu_common.js"; + /* import-globals-from contextmenu_common.js */ + Services.scriptloader.loadSubScript(contextmenu_common, this); + + // Ensure screenshots is really disabled (bug 1498738) + const addon = await AddonManager.getAddonByID("screenshots@mozilla.org"); + await addon.disable({ allowSystemAddons: true }); +}); + +add_task(async function test_text_input_spellcheck() { + await test_contextmenu( + "#input_spellcheck_no_value", + [ + "context-undo", + false, + "---", + null, + "context-cut", + null, // ignore the enabled/disabled states; there are race conditions + // in the edit commands but they're not relevant for what we're testing. + "context-copy", + null, + "context-paste", + null, // ignore clipboard state + "context-delete", + null, + "---", + null, + "context-selectall", + null, + "---", + null, + "spell-check-enabled", + true, + "spell-dictionaries", + true, + [ + "spell-check-dictionary-en-US", + true, + "---", + null, + "spell-add-dictionaries", + true, + ], + null, + ], + { + waitForSpellCheck: true, + async preCheckContextMenuFn() { + await SpecialPowers.spawn( + gBrowser.selectedBrowser, + [], + async function() { + let doc = content.document; + let input = doc.getElementById("input_spellcheck_no_value"); + input.setAttribute("spellcheck", "true"); + input.clientTop; // force layout flush + } + ); + }, + } + ); +}); + +add_task(async function test_text_input_spellcheckwrong() { + await test_contextmenu( + "#input_spellcheck_incorrect", + [ + "*prodigality", + true, // spelling suggestion + "spell-add-to-dictionary", + true, + "---", + null, + "context-undo", + null, + "---", + null, + "context-cut", + null, + "context-copy", + null, + "context-paste", + null, // ignore clipboard state + "context-delete", + null, + "---", + null, + "context-selectall", + null, + "---", + null, + "spell-check-enabled", + true, + "spell-dictionaries", + true, + [ + "spell-check-dictionary-en-US", + true, + "---", + null, + "spell-add-dictionaries", + true, + ], + null, + ], + { waitForSpellCheck: true } + ); +}); + +const kCorrectItems = [ + "context-undo", + false, + "---", + null, + "context-cut", + null, + "context-copy", + null, + "context-paste", + null, // ignore clipboard state + "context-delete", + null, + "---", + null, + "context-selectall", + null, + "---", + null, + "spell-check-enabled", + true, + "spell-dictionaries", + true, + [ + "spell-check-dictionary-en-US", + true, + "---", + null, + "spell-add-dictionaries", + true, + ], + null, +]; + +add_task(async function test_text_input_spellcheckcorrect() { + await test_contextmenu("#input_spellcheck_correct", kCorrectItems, { + waitForSpellCheck: true, + }); +}); + +add_task(async function test_text_input_spellcheck_deadactor() { + await test_contextmenu("#input_spellcheck_correct", kCorrectItems, { + waitForSpellCheck: true, + keepMenuOpen: true, + }); + let wgp = gBrowser.selectedBrowser.browsingContext.currentWindowGlobal; + + // Now the menu is open, and spellcheck is running, switch to another tab and + // close the original: + let tab = gBrowser.selectedTab; + await BrowserTestUtils.openNewForegroundTab(gBrowser, "https://example.org"); + BrowserTestUtils.removeTab(tab); + // Ensure we've invalidated the actor + await TestUtils.waitForCondition( + () => wgp.isClosed, + "Waiting for actor to be dead after tab closes" + ); + contextMenu.hidePopup(); + + // Now go back to the input testcase: + BrowserTestUtils.loadURI(gBrowser.selectedBrowser, MAIN_URL); + await BrowserTestUtils.browserLoaded( + gBrowser.selectedBrowser, + false, + MAIN_URL + ); + + // Check the menu still looks the same, keep it open again: + await test_contextmenu("#input_spellcheck_correct", kCorrectItems, { + waitForSpellCheck: true, + keepMenuOpen: true, + }); + + // Now navigate the tab, after ensuring there's an unload listener, so + // we don't end up in bfcache: + await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() { + content.document.body.setAttribute("onunload", ""); + }); + wgp = gBrowser.selectedBrowser.browsingContext.currentWindowGlobal; + + const NEW_URL = MAIN_URL.replace(".com", ".org"); + BrowserTestUtils.loadURI(gBrowser.selectedBrowser, NEW_URL); + await BrowserTestUtils.browserLoaded( + gBrowser.selectedBrowser, + false, + NEW_URL + ); + // Ensure we've invalidated the actor + await TestUtils.waitForCondition( + () => wgp.isClosed, + "Waiting for actor to be dead after onunload" + ); + contextMenu.hidePopup(); + + // Check the menu *still* looks the same (and keep it open again): + await test_contextmenu("#input_spellcheck_correct", kCorrectItems, { + waitForSpellCheck: true, + keepMenuOpen: true, + }); + + // Check what happens if the actor stays alive by loading the same page + // again; now the context menu stuff should be destroyed by the menu + // hiding, nothing else. + wgp = gBrowser.selectedBrowser.browsingContext.currentWindowGlobal; + BrowserTestUtils.loadURI(gBrowser.selectedBrowser, NEW_URL); + await BrowserTestUtils.browserLoaded( + gBrowser.selectedBrowser, + false, + NEW_URL + ); + contextMenu.hidePopup(); + + // Check the menu still looks the same: + await test_contextmenu("#input_spellcheck_correct", kCorrectItems, { + waitForSpellCheck: true, + }); + // And test it a last time without any navigation: + await test_contextmenu("#input_spellcheck_correct", kCorrectItems, { + waitForSpellCheck: true, + }); +}); + +add_task(async function test_cleanup() { + BrowserTestUtils.removeTab(gBrowser.selectedTab); +}); diff --git a/browser/base/content/test/contextMenu/contextmenu_common.js b/browser/base/content/test/contextMenu/contextmenu_common.js index e881ba5ef6dd..5ffb84f84931 100644 --- a/browser/base/content/test/contextMenu/contextmenu_common.js +++ b/browser/base/content/test/contextMenu/contextmenu_common.js @@ -352,6 +352,8 @@ let lastElementSelector = null; * preCheckContextMenuFn: callback to run before opening menu * onContextMenuShown: callback to run when the context menu is shown * postCheckContextMenuFn: callback to run after opening menu + * keepMenuOpen: if true, we do not call hidePopup, the consumer is + * responsible for calling it. * @return {Promise} resolved after the test finishes */ async function test_contextmenu(selector, menuItems, options = {}) { @@ -472,6 +474,8 @@ async function test_contextmenu(selector, menuItems, options = {}) { info("Completed postCheckContextMenuFn"); } - contextMenu.hidePopup(); - await awaitPopupHidden; + if (!options.keepMenuOpen) { + contextMenu.hidePopup(); + await awaitPopupHidden; + } } diff --git a/toolkit/actors/InlineSpellCheckerParent.jsm b/toolkit/actors/InlineSpellCheckerParent.jsm index 755a9215ede9..84b8a616cc43 100644 --- a/toolkit/actors/InlineSpellCheckerParent.jsm +++ b/toolkit/actors/InlineSpellCheckerParent.jsm @@ -28,6 +28,25 @@ class InlineSpellCheckerParent extends JSWindowActorParent { } uninit() { + // This method gets called by InlineSpellChecker when the context menu + // goes away and the InlineSpellChecker instance is still alive. + // Stop referencing it and tidy the child end of us. this.sendAsyncMessage("InlineSpellChecker:uninit", {}); } + + _destructionObservers = new Set(); + registerDestructionObserver(obj) { + this._destructionObservers.add(obj); + } + + unregisterDestructionObserver(obj) { + this._destructionObservers.delete(obj); + } + + didDestroy() { + for (let obs of this._destructionObservers) { + obs.actorDestroyed(this); + } + this._destructionObservers = null; + } } diff --git a/toolkit/modules/InlineSpellChecker.jsm b/toolkit/modules/InlineSpellChecker.jsm index 23dff64056a7..ffc58714b0d9 100644 --- a/toolkit/modules/InlineSpellChecker.jsm +++ b/toolkit/modules/InlineSpellChecker.jsm @@ -27,7 +27,15 @@ InlineSpellChecker.prototype = { initFromRemote(aSpellInfo, aWindowGlobalParent) { if (this.mRemote) { - throw new Error("Unexpected state"); + // We shouldn't get here, but let's just recover instead of bricking the + // menu by throwing exceptions: + Cu.reportError(new Error("Unexpected remote spellchecker present!")); + try { + this.mRemote.uninit(); + } catch (ex) { + Cu.reportError(ex); + } + this.mRemote = null; } this.uninit(); @@ -490,6 +498,7 @@ function RemoteSpellChecker(aSpellInfo, aWindowGlobalParent) { this._spellInfo = aSpellInfo; this._suggestionGenerator = null; this._actor = aWindowGlobalParent.getActor("InlineSpellChecker"); + this._actor.registerDestructionObserver(this); } RemoteSpellChecker.prototype = { @@ -571,6 +580,15 @@ RemoteSpellChecker.prototype = { this._actor.recheckSpelling(); }, uninit() { - this._actor.uninit(); + if (this._actor) { + this._actor.uninit(); + this._actor.unregisterDestructionObserver(this); + } + }, + + actorDestroyed() { + // The actor lets us know if it gets destroyed, so we don't + // later try to call `.uninit()` on it. + this._actor = null; }, }; diff --git a/toolkit/modules/InlineSpellCheckerContent.jsm b/toolkit/modules/InlineSpellCheckerContent.jsm index 7284dca69003..205585c4543f 100644 --- a/toolkit/modules/InlineSpellCheckerContent.jsm +++ b/toolkit/modules/InlineSpellCheckerContent.jsm @@ -17,6 +17,7 @@ var InlineSpellCheckerContent = { initContextMenu(event, editFlags, actor) { this._actor = actor; + this._actor.registerDestructionObserver(this); let spellChecker; if (!(editFlags & (SpellCheckHelper.TEXTAREA | SpellCheckHelper.INPUT))) { @@ -75,10 +76,17 @@ var InlineSpellCheckerContent = { }, uninitContextMenu() { + if (this._actor) { + this._actor.unregisterDestructionObserver(this); + } this._actor = null; this._spellChecker = null; }, + actorDestroyed() { + this.uninitContextMenu(); + }, + _generateSpellSuggestions() { let spellChecker = this._spellChecker.mInlineSpellChecker.spellChecker; try {