Closed Bug 1609760 Opened 4 years ago Closed 4 years ago

Stop assigning properties to the global `this` in JSMs in Thunderbird (port bug 1608278)

Categories

(Thunderbird :: General, task, P1)

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 74.0

People

(Reporter: mkmelin, Assigned: mkmelin)

Details

Attachments

(6 files, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1608278 +++

Do bug 1608278 for Thunderbird related code. For jsms,

this.EXPORTED_SYMBOLS = ["Foo"];

... should become

const EXPORTED_SYMBOLS = ["Foo"];

The mail/ part

Attachment #9124986 - Flags: review?(khushil324)
Attachment #9124986 - Attachment is patch: true
Comment on attachment 9124986 [details] [diff] [review]
bug1609760_stop_assigning_global_this_jsm.patch (mail/)

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

Looks good to me. r=khushil.
Can we remove the const EXPORTED_SYMBOLS from following places?

::: mail/components/activity/modules/activityModules.jsm
@@ +5,5 @@
>  
>  // This module is designed to be a central place to initialise activity related
>  // modules.
>  
> +const EXPORTED_SYMBOLS = [];

Do we need this?

::: mail/components/im/modules/index_im.jsm
@@ +1,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +const EXPORTED_SYMBOLS = [];

Do we need this?
Attachment #9124986 - Flags: review?(khushil324) → review+

I think we need them, or at least they do no harm. From searching, looks like tools and compilers are sometimes confused if there is no export.

Keywords: leave-open
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/a0dda319d59b
Stop assigning properties to the global `this` in JSMs in Thunderbird (port bug 1608278). r=khushil
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird 74.0
Attachment #9142807 - Flags: review?(paul)
Attachment #9142807 - Attachment is obsolete: true
Attachment #9142807 - Flags: review?(paul)
Attachment #9142808 - Flags: review?(paul)
Attachment #9142809 - Flags: review?(paul)
Attachment #9142811 - Flags: review?(paul)
Attachment #9142812 - Flags: review?(paul)
Comment on attachment 9142808 [details] [diff] [review]
bug1609760_global_this_ExtensionToolbarButtons.patch

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

LGTM.
Attachment #9142808 - Flags: review?(paul) → review+
Comment on attachment 9142809 [details] [diff] [review]
bug1609760_global_this_mailnews.patch

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

LGTM.
Attachment #9142809 - Flags: review?(paul) → review+
Comment on attachment 9142811 [details] [diff] [review]
bug1609760_global_this_chat.patch

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

LGTM.
Attachment #9142811 - Flags: review?(paul) → review+
Comment on attachment 9142812 [details] [diff] [review]
bug1609760_global_this_common.patch

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

LGTM.
Attachment #9142812 - Flags: review?(paul) → review+
Comment on attachment 9142813 [details] [diff] [review]
bug1609760_global_this_calendar.patch

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

LGTM.
Attachment #9142813 - Flags: review?(paul) → review+
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/bf4e3f27671f
Stop assigning properties to the global `this` in ExtensionToolbarButtons.jsm. r=pmorris DONTBUILD
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/91432c993ab3
Stop assigning properties to the global `this` in common/ JSMs (port bug 1608278). r=pmorris
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/518552b4540d
Stop assigning properties to the global `this` in calendar/ JSMs (port bug 1608278). r=pmorris DONTBUILD
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/7853102282e6
Stop assigning properties to the global `this` in chat/ JSMs (port bug 1608278). r=pmorris DONTBUILD
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/d2d158a8c71e
Stop assigning properties to the global `this` in mailnewx/ JSMs (port bug 1608278). r=pmorris DONTBUILD

We're done with this bug now.

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.