1133185 - Remove nonstandard let blocks from toolkit/webapps

Status: RESOLVED FIXED

(Firefox Graveyard :: Web Apps, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: remove-let-blocks.patch

let blocks are a nonstandard SpiderMonkey feature we would like to remove (in bug 1023609).

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 8564495 [details] [diff] [review]
remove-let-blocks.patch

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

This is ok, but we used some of these blocks to limit the scope of same-named variables, and I would prefer to avoid mixing declaration/assignments with non-declaration/assignments of such variables, so I would replace the blocks that enclose them with non-let blocks, f.e.:

  {
    let deferred = Promise.defer();
    request.onerror = function() {
      deferred.reject(this.error.name);
    };
    request.onsuccess = deferred.resolve;
    yield deferred.promise;
  }

  let appObject = request.result;
  ok(appObject, "app is non-null");

  {
    let deferred = Promise.defer();
    appObject.ondownloaderror = function() {
      deferred.reject(appObject.downloadError.name);
    };
    appObject.ondownloadapplied = deferred.resolve;
    yield deferred.promise;
  }

(Alternately, you could give them unique names or declare them separately.)

Comment 2

[Chris Peterson [:cpeterson]]

Do you prefer the non-let blocks or unique names (like `deferred2`)? The non-let blocks would preserve the existing code indentation and variable names. The unique names are less ambiguous, but a little ugly.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(In reply to Chris Peterson [:cpeterson] from comment #2)
> Do you prefer the non-let blocks or unique names (like `deferred2`)? The
> non-let blocks would preserve the existing code indentation and variable
> names. The unique names are less ambiguous, but a little ugly.

I'm ok with either, but I would prefer the non-let blocks.

Comment 4

[Chris Peterson [:cpeterson]]

Thanks. I landed with the non-let blocks you suggested:

https://hg.mozilla.org/integration/mozilla-inbound/rev/a179c7531bca

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a179c7531bca