1151475 - Remove use of expression closures in mail/

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Tooru Fujisawa [:arai]]

Similar to bug 1123124.

Need to replace non-standard expression closure `function() expr` with one of function declaration, function expression, and arrow function, before fixing bug 1083458.

I have draft patch for this.

Comment 1

[:aceman]

This seems to be a search query (thanks to jcranmer) that returns many of the occurrences:
https://dxr.mozilla.org/comm-central/search?q=regexp%3A%22\bfunction\s*\%28[^%29]*\%29\s*[a-zA-Z0-9_%28\%22%27.~!%2B-]%22+path%3Amail%2F&case=true&redirect=true

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Arai has already pushed patches to Try-comm-central, e.g. https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=acc724e0ed19

Comment 3

[:aceman]

OK:) If only mozmill tests worked so the patches could actually be tested :(

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Remove use of expression closure from mail/base/.

Prepared 5 patches for mail/. they're not so large (same as mailnews/), so I guess it doesn't matter on backporting future patches to esr.

converting rules are following:
  * function declaration
    add `return` and braces
  * standalone function expression contans and receives `this` (Array.filter, bind, etc)
    convert to arrow function
  * standalone function expression contans no `this`
    convert to arrow function
  * method-like property contains `this`
    add `return` and braces
  * method-like property contains no `this` with short body
    convert to arrow function
  * method-like property contains no `this` with long body
    add `return` and braces
  * method-like property with named function expression
    add `return` and braces
  * getter property
    add `return` and braces
  * setter property
    add braces

Seems no regression on try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3ea958f2cad9
also, ran mozmill locally on linux 64bit, and no regression found with the patch series (19 failures with/without patches, for me).

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Remove use of expression closure from mail/components/im/.

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Part 3: Remove use of expression closure from mail/components/cloudfile/.

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Part 4: Remove use of expression closure from mail/components/.

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Part 5: Remove use of expression closure from mail/test/mozmill/.

Comment 9

[Florian Quèze [:florian]]

Comment on attachment 8589729 [details] [diff] [review]
Part 2: Remove use of expression closure from mail/components/im/.

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

Thanks!

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 8589728 [details] [diff] [review]
Part 1: Remove use of expression closure from mail/base/.

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

Looks good, thx! r=mkmelin

::: mail/base/content/mailWidgets.xml
@@ +29,5 @@
>                  .forEach(this.selectedItems.push, this.selectedItems);
>  
> +        children.filter(aChild => !aChild.hasAttribute("context"))
> +                .forEach(aChild => aChild.setAttribute("context",
> +                         this.getAttribute("itemcontext")));

please re-indent this as last line two more spaces so it's not so confusing

Comment 11

[Tooru Fujisawa [:arai]]

(In reply to Magnus Melin from comment #10)
> > +        children.filter(aChild => !aChild.hasAttribute("context"))
> > +                .forEach(aChild => aChild.setAttribute("context",
> > +                         this.getAttribute("itemcontext")));
> 
> please re-indent this as last line two more spaces so it's not so confusing

Thanks! fixed locally :)

Comment 12

[Tooru Fujisawa [:arai]]

Landed Part 1 and 2 :)

https://hg.mozilla.org/comm-central/rev/07a6ec81889a
https://hg.mozilla.org/comm-central/rev/1f40bc6a4a87

Comment 13

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

Comment on attachment 8589730 [details] [diff] [review]
Part 3: Remove use of expression closure from mail/components/cloudfile/.

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

Sorry for the delay! Thank you!

Comment 14

[Tooru Fujisawa [:arai]]

Thank you for reviewing :D

https://hg.mozilla.org/comm-central/rev/5eb45f784b5b
https://hg.mozilla.org/comm-central/rev/ecee3826ae91
https://hg.mozilla.org/comm-central/rev/3f5395827a5c

Comment 15

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.