1117060 - remove deprecated let expressions in comm-central

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[:aceman]

Warning: JavaScript 1.7's let expressions are deprecated
Source File: chrome://gloda/content/glodacomplete.xml
Line: 133, Column: 12
Source Code:
            start == 0 ? a[1] - b[1] : start);

Comment 1

[:aceman]

There seem to be some more ocurrences of this pattern:
	calendar/test/mozmill/testLocalICS.js
93 	let (str = {}) {
	calendar/test/unit/test_bug350845.js
19 	let (passed = false) {
36 	let (passed = false) {
	im/content/preferences/applications.js
1045 	let (preferredApp = aHandlerInfo.preferredApplicationHandler) {
	mail/components/preferences/applications.js
1797 	let (preferredApp = aHandlerInfo.preferredApplicationHandler) {
	mailnews/db/gloda/content/glodacomplete.xml
132 	regions = regions.sort(function(a, b) let (start = a[0] - b[0])
	suite/common/places/tests/browser/head.js
4 	let (getter = PlacesUIUtils.__lookupGetter__("leftPaneFolderId")) {
10 	let (getter = PlacesUIUtils.__lookupGetter__("leftPaneFolderId")) {
	suite/common/places/tests/unit/head_bookmarks.js
14 	let (commonFile = do_get_file("../../../../../toolkit/components/places/tests/head_common.js", false)) {
33 	let (XULAppInfo = {
	suite/common/pref/pref-applications.js
1685 	let (preferredApp = aHandlerInfo.preferredApplicationHandler) {

Comment 2

[:aceman]

Attachment: patch for IM

Comment 3

[:aceman]

Attachment: patch for calendar

Comment 4

[:aceman]

Attachment: patch for mailnews

Comment 5

[:aceman]

Attachment: patch for suite

Comment 6

[:aceman]

Attachment: patch for mail

Comment 7

[:aceman]

Aryx, please push all to the try server, all tests. Thanks.

Comment 8

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

Pushed: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central

Comment 9

[:aceman]

So I didn't spot anything in the test runs so I mark the review requests.

Comment 10

[Florian Quèze [:florian]]

Comment on attachment 8549197 [details] [diff] [review]
patch for IM

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

Thanks!

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8549198 [details] [diff] [review]
patch for calendar

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

Thanks for the patch, r=philipp with these minor nits:

::: calendar/test/mozmill/testLocalICS.js
@@ +91,5 @@
>    cstream.init(fstream, "UTF-8", 0, 0);
> +
> +  let str = {};
> +  cstream.readString(-1, str);
> +  contents = str.value;

Please just remove the contents variable and then this shortly below:

controller.assertJS(str.value.indexOf("SUMMARY:" + title) != -1);

::: calendar/test/unit/test_bug350845.js
@@ +20,5 @@
> +    try {
> +        event.setPropertyParameter("X-UNKNOWN", "UNKNOWN", "VALUE");
> +    } catch (e) {
> +        passed = true;
> +    }

I think you can just use this (untested):

do_check_throws(function() {
  event.setPropertyParameter("X-UNKNOWN", "UNKNOWN", "VALUE");
}, "Property X-UNKNOWN not set");

That would save us from that extra |passed| variable.

@@ +38,5 @@
> +    } catch (e) {
> +        passed = true;
> +    }
> +    if (!passed) {
> +        do_throw("Getting parameter enumerator on unset property unexpectedly succeeded");

do_check_throws(function() {
  event.getParameterEnumerator("X-UNKNOWN");
}, "Property X-UNKNOWN not set");

Comment 12

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8549199 [details] [diff] [review]
patch for mailnews

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

::: mailnews/db/gloda/content/glodacomplete.xml
@@ +131,5 @@
>            // Sort the regions by start position then end position
> +          regions = regions.sort(function(a, b) {
> +            let start = a[0] - b[0];
> +            return (start == 0) ? a[1] - b[1] : start; }
> +          );

Nit: Prefer }); over }\n);. Otherwise, okay.

Comment 13

[:aceman]

Attachment: patch for mailnews v1.1

Comment 14

[:aceman]

Attachment: patch for calendar v1.1

Comment 15

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: patch for calendar v1.2

I've tested the xpcshell calendar tests locally with this patch and it works. mozmill looks fine codewise but I can't run them locally. I think this is fine, I can fix it if it should break. r+ for the calendar part.

Comment 16

[:aceman]

Thanks to all.

Comment 17

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/e78870a4d9ef
https://hg.mozilla.org/comm-central/rev/6d9c6075c650
https://hg.mozilla.org/comm-central/rev/75bbd54c34fc
https://hg.mozilla.org/comm-central/rev/0b909ec42a34
https://hg.mozilla.org/comm-central/rev/2c086b05da4b