866733 - Remove `for each` usage from Add-on SDK code base

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P3)

Description

[Matteo Ferretti [:zer0] [:matteo]]

The `for each` statement is considered deprecated (see: https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Statements/for_each...in), we should stop to use it in our code.

Comment 1

[anant.asty]

I would like to try fixing this bug as my first bug.
Thanks

Comment 2

[Matteo Ferretti [:zer0] [:matteo]]

That would be awesome! Unfortunately this bug depends partially from bug 865489, in order to properly remove `for each` statement we should implement `for…of` iterator before, so that we can easily replace the first with the second, for our APIs. I will try to get that bug fixed fist.

Comment 3

[anant.asty]

I would still be willing to fix it. Please let me know when bug 865489 is fixed and i will get on it.

Comment 4

[Matteo Ferretti [:zer0] [:matteo]]

Sure, thanks!

Comment 5

[Jordan Santell [:jsantell] [@jsantell]]

Anant, are you still interested in fixing this bug? Bug 865489 appears to be fixed. Thanks!

Comment 6

[anant.asty]

I sure am still interested in fixing this bug since it seems like an easy first bug.

Comment 7

[Jordan Santell [:jsantell] [@jsantell]]

Awesome! Assign the bug to yourself, and let me know here or in IRC if you have any questions about it, and thanks!

Comment 8

[anant.asty]

I am not sure how to change the assignee since it is a link to an email address. Also are you in the introduction channel in irc?

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

Putting your email in the 'assignee' field should have it auto detect your email address and just click that and hit save -- Myself (and others who can help) are in #jetpack

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to anant.asty from comment #8)
> I am not sure how to change the assignee since it is a link to an email
> address. Also are you in the introduction channel in irc?

You'd need to have editbugs privileges to make these changes, as per:
https://bugzilla.mozilla.org/page.cgi?id=get_permissions.html

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

http://mxr.mozilla.org/mozilla-central/search?string=for+each%28&find=%2Faddon-sdk%2F&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

http://mxr.mozilla.org/mozilla-central/search?string=for+each+%28&find=%2Faddon-sdk%2F&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

These two searches should list all of the instances of for each in the SDK codebase. This probably crosses over with what bug 878507 would fix.

Comment 12

[Jordan Santell [:jsantell] [@jsantell]]

Hey Anant, just pinging you to see if you have any questions or need any help re: this bug. Let me know! :)

Comment 13

[Giovanny Gongora [:gioyik]]

Hi, 

I am interested in work on this but seems that needs to be fixed first 925989. Is that right?

Regards,
Gio

Comment 14

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Giovanny Andres Gongora Granada from comment #13)
> Hi, 
> 
> I am interested in work on this but seems that needs to be fixed first
> 925989. Is that right?
> 
> Regards,
> Gio

This bug needs to be fixed so that bug 925989 can be fixed.  In other words bug 925989 depends on this bug, so the work here happens first.

Comment 15

[Giovanny Gongora [:gioyik]]

Ok, i am going to see if can i generate a patch for this. The patch needs to be for mozilla-central, right?

Comment 16

[Jordan Santell [:jsantell] [@jsantell]]

Giovanny, the SDK works through github.com (http://github.com/mozilla/addon-sdk), so you can fork the repo there, and send a pull request. More details here!
https://github.com/mozilla/addon-sdk/wiki/contribute

Comment 17

[robertknight]

Github PR to fix this issue: https://github.com/mozilla/addon-sdk/pull/1557

This also fixes an issue I ran into where frequent use of self.port.once() could result in errors due to a 'for each' loop over event listeners in content-worker.js where the event handler removed itself from the loop when called.

Using the 'for each' syntax this resulted in the loop yielding undefined values. Using 'for of' avoids the problem - see the first example in https://gist.github.com/robertknight/8f3f6f87fdeb4129a6a9

Comment 18

[robertknight]

Attachment: PR replacing 'for each' with 'for of'

Comment 19

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/026a76dc5cf435b95d3bcbedc22d6d1d1cba6216
Bug 866733 - Replace 'for each (v in iterable)' loops

Replace deprecated 'for each (v in iterable)' syntax
with 'for (v of iterable)'

There are several remaining uses of 'for each' which
iterate over objects that are not iterables.

https://github.com/mozilla/addon-sdk/commit/712a4874e0d1600cbb9d3311e79073dcf4ac14e1
Merge pull request #1557 from robertknight/866733-replace_for_each_loops

fix Bug 866733 - Replace 'for each (v in iterable)' loops r=@ZER0