Opened on 02/18/2015 at 06:14:03 PM
Closed on 04/01/2015 at 11:36:12 AM
Last modified on 05/20/2015 at 02:22:39 PM
#2013 closed change (duplicate)
Replace "for each" loops with "for of" loops (Buildtools)
| Reported by: | trev | Assignee: | ngduonglam |
|---|---|---|---|
| Priority: | P4 | Milestone: | |
| Module: | Automation | Keywords: | goodfirstbug |
| Cc: | Blocked By: | ||
| Blocking: | Platform: | Firefox | |
| Ready: | yes | Confidential: | no |
| Tester: | Verified working: | no | |
| Review URL(s): | |||
Description
Background
Using for each is now deprecated and shows a warning in current Firefox nightlies. We are still using it in keySelector.js. Related: #2010.
What to change
Replace all usage of for each loops in lib/keySelector.js by for .. of loops.
Attachments (1)
Change History (9)
comment:1 Changed on 02/28/2015 at 08:24:06 AM by ngduonglam
comment:2 Changed on 02/28/2015 at 07:23:26 PM by trev
- Owner set to ngduonglam
Sure, feel free to fix this one. You can ask in our IRC channel if you have questions. See also https://adblockplus.org/en/contribute-code.
comment:3 Changed on 03/01/2015 at 03:23:38 AM by ngduonglam
@trev: where can I find that keySelector.js in GitHub? I asked in IRC but no response :(
comment:4 Changed on 03/01/2015 at 07:52:00 AM by trev
Well, it was at 4 AM on a weekend ;)
This issue is about the buildtools repository. It is a dependency of Adblock Plus for Firefox.
comment:5 Changed on 03/01/2015 at 08:00:12 AM by ngduonglam
Ops - sorry, I'm in Vietnam, GMT+7 :)
Changed on 03/01/2015 at 08:11:12 AM by ngduonglam
Replaced "for each" loops with "for of" loops
comment:6 Changed on 03/02/2015 at 12:17:12 PM by trev
That's not a patch, not something anybody can review. You can export your commit with hg export (Mercurial) or git format-patch (Git). Also, just in case you didn't do that already, please make sure to test your changes - this code is used to select available shortcut keys in Adblock Plus for Firefox.
comment:7 Changed on 04/01/2015 at 11:36:12 AM by sebastian
- Blocking 312 removed
- Resolution set to duplicate
- Status changed from new to closed
- Verified working unset
Duplicate of #1434.
comment:8 Changed on 05/20/2015 at 02:22:39 PM by philll
- Platform changed from Firefox/Firefox Mobile to Firefox
Made Firefox and Firefox mobile available as seperate platforms.
Hi - I would like to start my contribution with this one - is that ok?