Closed Bug 1039764 Opened 5 years ago Closed 5 years ago

spellchecker context menu broken in Nightly

Categories

(Core :: Spelling checker, defect)

x86_64
Windows 8
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
e10s + ---
firefox33 + fixed

People

(Reporter: ally, Assigned: ally)

References

Details

Attachments

(1 file)

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.
Attached patch fixContextMenuSplinter Review
Assignee: nobody → ally
Status: NEW → ASSIGNED
Attachment #8457493 - Flags: review?(mconley)
Blocks: e10s-m1
tracking-e10s: --- → +
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);
Attachment #8457493 - Flags: review?(mconley) → review+
Duplicate of this bug: 1040022
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
Blocks: old-e10s-m2
No longer blocks: e10s-m1
Waiting for trees to re-open from Bug 1040308
https://hg.mozilla.org/mozilla-central/rev/3b9f8506d2cf
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.