1206514 - Remove remaining use of expression closure from mail/.

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Tooru Fujisawa [:arai]]

I overlooked some use of expression closure in bug 1151475 patches.

--

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.

will post patch shortly.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Remove remaining use of expression closure from mail/.

All overlooked use are in mail/test/mozmill/.

converting rules are following:
  * standalone function expression contans and receives `this` (waitFor, etc)
    convert to arrow function
  * standalone function expression contans no `this`
    convert to arrow function

Comment 2

[:aceman]

Comment on attachment 8663422 [details] [diff] [review]
Remove remaining use of expression closure from mail/.

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

Thanks for the cleanup.

::: mail/test/mozmill/content-tabs/test-about-support.js
@@ +55,5 @@
>    let tab = open_content_tab_with_click(mc.menus.helpMenu.aboutsupport_open,
>                                          "about:support");
>    // We have one variable that's asynchronously populated -- wait for it to be
>    // populated.
> +  mc.waitFor(() => tab.browser.contentWindow.gExtensions !== undefined,

Would this work with brackets, like () => (tab.browser.contentWindow.gExtensions !== undefined) ?
I find it weird, that the comma terminates the function declaration and starts the next argument to waitFor(). Yes, it may be correct syntax, but is not that readable to me.

Comment 3

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

Comment on attachment 8663422 [details] [diff] [review]
Remove remaining use of expression closure from mail/.

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

Thanks!

::: mail/test/mozmill/shared-modules/test-window-helpers.js
@@ +400,2 @@
>                    "Timeout waiting for window to close!",
> +      WINDOW_CLOSE_TIMEOUT_MS, WINDOW_CLOSE_CHECK_INTERVAL_MS);

While you're here, might as well fix the indentation here:

utils.waitFor(() => this.monitorizeClose(),
              "Timeout waiting for window to close!",
              WINDOW_CLOSE_TIMEOUT_MS,
              WINDOW_CLOSE_CHECK_INTERVAL_MS);

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Remove remaining use of expression closure from mail/. r=mconley

Thank you for reviewing
addressed review comments.

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 8665842 [details] [diff] [review]
Remove remaining use of expression closure from mail/. r=mconley

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

mconley already reviewed this, no?

Comment 6

[Tooru Fujisawa [:arai]]

oh, I thought that review request has another meaning.
anyway, thanks :D

Comment 7

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/306b67ce6ad9ea4ac54c64a3a98f5b8da01e0081
Bug 1206514 - Remove remaining use of expression closure from mail/. r=mkmelin