Need to replace non-standard expression closure with one of: * function declaration * function expression * arrow function before fixing bug 1083458. converting rules are following: * function declaration add `return` and braces * standalone named function expression add `return` and braces * standalone anonymous function expression contans and receives `this` (Array.filter, bind, etc) convert to arrow function, and remove code passing |this| * standalone anonymous function expression contans no `this` convert to arrow function * property with anonymous function expression, contains `this` add `return` and braces * property with anonymous function expression, contains no `this`, short body convert to arrow function * property with anonymous function expression, contains no `this`, long body add `return` and braces * property with named function expression add `return` and braces * getter property add `return` and braces * setter property add braces Since there are a lot of patches, separated into 8 bugs, each bug corresponds to one of following directories: * browser/, except browser/components/. * browser/components/. * dom/. * layout/. * services/. * toolkit/, except toolkit/components/. * toolkit/components/. * b2g/, chrome/, docshell/, mobiles/, modules/, netwerk/, parser/, security/, storage/, testing/, webapprt/, widget/, xpcom/ (not yet touched addon-sdk) I have draft patches, will post them (may take some time to prepare and post).

Bug 1207497 - Part 1: Remove use of expression closure from toolkit/, exept tests. r?Gijs

Bug 1207497 - Part 2: Remove use of expression closure from tests in toolkit/. r?Gijs

Comment on attachment 8665397 [details] MozReview Request: Bug 1207497 - Part 1: Remove use of expression closure from toolkit/, exept tests. r?Gijs https://reviewboard.mozilla.org/r/20225/#review18201 Basically, I think if we are using filter/some/map array methods, and pass in the function expression there, even if it does have a name it should be fine to replace it with an (anonymous) arrow function. If that breaks reference, then the function shouldn't be declared inside the filter/some/map array method call. :-\ ::: toolkit/mozapps/extensions/AddonManager.jsm:1101 (Diff revision 1) > - this.types[type].providers = this.types[type].providers.filter(function filterProvider(p) p != aProvider); > + this.types[type].providers = this.types[type].providers.filter(function filterProvider(p) { > + return p != aProvider; > + }); filter(p => p != aProvider) ::: toolkit/mozapps/extensions/AddonManager.jsm:1685 (Diff revision 1) > this.startupChanges[aType] = this.startupChanges[aType].filter( > - function filterItem(aItem) aItem != aID); > + function filterItem(aItem) { > + return aItem != aID; > + }); aItem => aItem != aID ::: toolkit/mozapps/extensions/content/extensions.js:2338 (Diff revision 1) > var self = this; > this._listBox.addEventListener("keydown", function listbox_onKeydown(aEvent) { > if (aEvent.keyCode == aEvent.DOM_VK_RETURN) { > var item = self._listBox.selectedItem; > if (item) > item.showInDetailView(); > } > }, false); bonus points for rm'ing var self = this and using aEvent => { } and s/self/this/ inside the function body. ::: toolkit/mozapps/extensions/content/extensions.js:2347 (Diff revision 1) > - this._filter.addEventListener("command", function filter_onCommand() self.updateView(), false); > + this._filter.addEventListener("command", function filter_onCommand() { > + return self.updateView(); > + }, false); () => this.updateView() (I think?) ::: toolkit/mozapps/extensions/internal/AddonRepository.jsm:991 (Diff revision 1) > let elementsList = Array.filter(aElement.children, > - function arrayFiltering(aChild) aChild.tagName == aTagName); > + function arrayFiltering(aChild) { > + return aChild.tagName == aTagName; > + }); aChild => aChild.tagName == aTagName ::: toolkit/mozapps/extensions/internal/AddonRepository.jsm:1308 (Diff revision 1) > - if (requiredAttributes.some(function parseAddons_attributeFilter(aAttribute) !result.addon[aAttribute])) > + if (requiredAttributes.some(function parseAddons_attributeFilter(aAttribute) { > + return !result.addon[aAttribute]; > + })) aAttribute => !resulrt.addon[aAttribute] ::: toolkit/mozapps/extensions/internal/AddonRepository.jsm:1435 (Diff revision 1) > - .filter(function compatRangesFilter(aItem) !!aItem); > + .filter(function compatRangesFilter(aItem) { > + return !!aItem; > + }); aItem => !!aItem

(more generally, I expect that this is all leftovers from when stacks and debuggers didn't deal well with anonymous functions. That's all much better now, and there is generally no reason anymore to name these functions, IMO.)

Comment on attachment 8665398 [details] MozReview Request: Bug 1207497 - Part 2: Remove use of expression closure from tests in toolkit/. r?Gijs https://reviewboard.mozilla.org/r/20227/#review18203

https://hg.mozilla.org/integration/mozilla-inbound/rev/94d51a413fba https://hg.mozilla.org/integration/mozilla-inbound/rev/e4f5f909df68

Thank you for reviewing :) Replaced all those named function expression with arrow.

(In reply to Jan de Mooij [:jandem] from bug 1207491 comment #44) > (In reply to Tooru Fujisawa [:arai] from bug 1207491 comment #0) > > * property with anonymous function expression, contains `this` > > add `return` and braces > > Just to be sure, you know `arguments` is also lexical right? See bug 889158 > and > https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/ > Arrow_functions#Lexical_arguments Fixed wrong replacement in toolkit/mozapps/downloads/tests/unit/test_lowMinutes.js, which uses arguments.

https://hg.mozilla.org/mozilla-central/rev/94d51a413fba https://hg.mozilla.org/mozilla-central/rev/e4f5f909df68 https://hg.mozilla.org/mozilla-central/rev/fb3defaea56e