1293005 - Replace in-tree consumer of non-standard Iterator() with Object.{values,entries} in im/ in comm-central

Status: RESOLVED FIXED

(Instantbird Graveyard :: Other, defect)

Description

[Tooru Fujisawa [:arai]]

separated from bug 1290637.
see bug 1290637 for the details.

Required code changes are following:
  * Check each usage of non-standard Iterator [1] function,
    and replace it with Object.entries [2] or Object.values [3],
    or something appropriate for the specific usage

Here's the rough list for Iterator() usage (not exhaustive)
https://dxr.mozilla.org/comm-central/search?q=%22+Iterator(%22+path%3Aim%2Fcontent&redirect=false

for example:
  for (let [k, v] in Iterator(obj)) {
    ...
  }
can be replaced to:
  for (let [k, v] of Object.entries(obj)) {
    ...
  }

another example:
  for (let [, v] in Iterator(array)) {
    ...
  }
can be replaced to:
  for (let v of array) {
    ...
  }


[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Iterator
[2] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries
[3] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/values

Comment 1

[Aman Dwivedi]

Hi Tooru! I would like to take this up. Please assign it to me.

Comment 2

[Tooru Fujisawa [:arai]]

Thank you for your comment :)

Comment 3

[Aman Dwivedi]

Attachment: bugfix.patch

I hope this patch works. :)

Comment 4

[Tooru Fujisawa [:arai]]

Comment on attachment 8823222 [details] [diff] [review]
bugfix.patch

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

Thank you for your patch :)
It looks almost good.
Please address the following comment and post updated patch.

::: im/content/jsTreeView.js
@@ +124,4 @@
>      // for the next add item at our own level.
>      let currentCount = this._rowMap.length;
>      if (aChild.children.length && aChild.open) {
> +      for (let [i, child] of Object.entries(this._rowMap[aNewIndex].children)) {

If you use Object.entries, `i` becomes a string and the following expression `aNewIndex + i + 1` becomes string concatenation, instead of numeric operation.

Since `this._rowMap[aNewIndex].children` is HTMLCollection,
you can convert it to an array with Array.from, and then you can use use Array.prototype.entries instead,
so, `Array.from(this._rowMap[aNewIndex].children).entries()`

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/from
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/entries

Comment 5

[Aman Dwivedi]

Attachment: BugFix1293005.patch

Thanks for help :)

Comment 6

[Tooru Fujisawa [:arai]]

Comment on attachment 8823227 [details] [diff] [review]
BugFix1293005.patch

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

Thanks again :)

::: im/content/jsTreeView.js
@@ +124,4 @@
>      // for the next add item at our own level.
>      let currentCount = this._rowMap.length;
>      if (aChild.children.length && aChild.open) {
> +      for (let [i, child] of Object.entries(Array.from(this._rowMap[aNewIndex].children).entries())) {

You don't need Object.entries, Array.prototype.entries returns iterator

Comment 7

[Aman Dwivedi]

Attachment: BugFix1293005.patch

Oh thanks! Well this should work then. :)

Comment 8

[Tooru Fujisawa [:arai]]

Comment on attachment 8823228 [details] [diff] [review]
BugFix1293005.patch

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

Thanks!
forwarding review to aleth.

Comment 9

[aleth [:aleth]]

Comment on attachment 8823228 [details] [diff] [review]
BugFix1293005.patch

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

Thanks!

::: im/content/jsTreeView.js
@@ -124,4 @@
>      // for the next add item at our own level.
>      let currentCount = this._rowMap.length;
>      if (aChild.children.length && aChild.open) {
> -      for (let [i, child] in Iterator(this._rowMap[aNewIndex].children)) {

Please check the forked file mailnews/base/content/jsTreeView.js gets matching changes (in another bug).

Comment 10

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/eef804a8d9cf40c678c1d14c186ec6e9c913818a