1039764 - spellchecker context menu broken in Nightly

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[Allison Naaktgeboren :ally]

STR:
0) have multiple dictionaries installed (I have french & german for testing), running single process fx (no e10s)
1) type some text into bug comment
2) right click
3) click on Languages
4) select a different language

Expected
When the popup is reopened the other language is selected and the spellchecker uses that dictionary

Actual
Popup re-opens and original language is still selected. Spellcheck has not been run in the new language, and there is a js undefined error in the console on line 194.

Doing some digging, I caused this when bug 693555 landed, so fix it I shall.

Comment 1

[Allison Naaktgeboren :ally]

Attachment: fixContextMenu

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8457493 [details] [diff] [review]
fixContextMenu

Review of attachment 8457493 [details] [diff] [review]:
-----------------------------------------------------------------

Conditional r+. :) Thanks ally!

::: toolkit/modules/InlineSpellChecker.jsm
@@ +195,1 @@
>          item.addEventListener("command", callback(this, i), true);

Out of curiosity, is there anything preventing us from using:

item.addEventListener("command", () => {
  this.selectDictionary(i, menu.ownerDocument.defaultView);
}, true);

? That, to me, seems more readable, but I might be missing something here...

If that's for some reason not possible, I'm fine with this change, but I would suggest taking this opportunity to increase its readability by formatting it like:

var callback = function(me, val) {
  return function(evt) {
    me.selectDictionary(val, menu.ownerDocument.defaultView);
  }
};
item.addEventListener("command", callback(this, i), true);

Comment 5

[Clochix]

For the record, I just runned MozRegression and found:

Last good revision: 095d2a9c2be5
First bad revision: 06a47123670e
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=095d2a9c2be5&tochange=06a47123670e

Comment 6

[Allison Naaktgeboren :ally]

Waiting for trees to re-open from Bug 1040308

Comment 7

[Allison Naaktgeboren :ally]

https://hg.mozilla.org/integration/fx-team/rev/3b9f8506d2cf

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3b9f8506d2cf