Closed Bug 1293003 Opened 5 years ago Closed 4 years ago

Replace in-tree consumer of non-standard Iterator() with Object.{values,entries} in calendar

Categories

(Calendar :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: arai, Assigned: MakeMyDay)

References

Details

Attachments

(1 file, 1 obsolete file)

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%3Acalendar%2F+-path%3Aobj-x86_64-pc-linux-gnu&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
Summary: Replace in-tree consumer of non-standard Iterator() with Object.{values,entries} in calendar/ in comm-central → Replace in-tree consumer of non-standard Iterator() with Object.{values,entries} in calendar
Here's a first patch to deal with it. There are some occurrences in [1] for which I'm not sure how you want to deal with it: The implemenation of Map, Set and MapSetForEach.

The patch would need to be updated if landing after bug 1277972. I also pushed a try build, even though the logs may be flooded with failures due to bug 1292569.

[1] https://dxr.mozilla.org/comm-central/source/calendar/providers/gdata/modules/shim/Loader.jsm
[2] https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f45e0787607f
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Attachment #8778665 - Flags: feedback?(philipp)
Forgot to note about calendar/providers/gdata.

the directory is shared with Postbox, that uses Gecko 8 (at least at the point of 8 months ago, not sure about current situation),
so most of new ES6 things (including Object.{values,entries}, even for-of) cannot be used there.
it's also mentioned in bug 1234048.
maybe we'd better address that directory separately.
(In reply to Tooru Fujisawa [:arai] from comment #2)
> Forgot to note about calendar/providers/gdata.
> 
> the directory is shared with Postbox, that uses Gecko 8 (at least at the
> point of 8 months ago, not sure about current situation),
> so most of new ES6 things (including Object.{values,entries}, even for-of)
> cannot be used there.
> it's also mentioned in bug 1234048.
> maybe we'd better address that directory separately.

Yes, I know. Therefore I asked Philipp how he wants to deal with that. But when reading my comment once again, it's not too obvious that this question was for Philipp - sorry for not being precise and thanks for the heads up.
Note to self: the patch here needs to be updated since landing bug 1277972 and friends.
There was a new Iterator landed in bug 1295525.
Updated patch to follow up the event-in-a-tab related changes. This still excludes changes to the gdata provider code.

Philipp, how do you want to proceed with that?
Attachment #8778665 - Attachment is obsolete: true
Attachment #8778665 - Flags: feedback?(philipp)
Attachment #8783304 - Flags: review?(philipp)
Comment on attachment 8783304 [details] [diff] [review]
ReplaceConsumersOfNonStandardIterators-V2.diff

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

r+ for this change. I'm pretty sure I will drop the gecko 8 requirement soon, but still need to clarify some things.
Attachment #8783304 - Flags: review?(philipp) → review+
https://hg.mozilla.org/comm-central/rev/17990a5e425ff6da0ea9bef7a58ddce2bdaf5e7e

I filed bug 1303700 as a follow-up bug for the Google provider.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Depends on: 1303700
Resolution: --- → FIXED
Target Milestone: --- → 5.3
You need to log in before you can comment on or make changes to this bug.