Closed
Bug 1122430
Opened 8 years ago
Closed 8 years ago
Use for..of iteration instead of Array.forEach in ctrlTab.suspendGUI
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 38
People
(Reporter: dao, Assigned: vikramcse.10, Mentored)
Details
(Whiteboard: [good first bug][lang=js])
Attachments
(1 file)
911 bytes,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
In the ctrlTab.suspendGUI method (https://hg.mozilla.org/mozilla-central/annotate/cac6192956ab/browser/base/content/browser-ctrlTab.js#l399), instead of: Array.forEach(x, function (y) { ... }, this); we can do: for (let y of x) { ... }
Assignee | ||
Comment 1•8 years ago
|
||
Where is the test file for testing this code? and how can i find test file for any code for my future knowledge? thanks.
Assignee | ||
Comment 2•8 years ago
|
||
All tests passed. Is this right patch? Thanks.
Attachment #8550317 -
Flags: review?(dao)
Reporter | ||
Comment 4•8 years ago
|
||
(In reply to shreyas from comment #3) > Hi, I would like to work on it. Vikram already submitted a patch. How about bug 1122433?
Assignee: nobody → vikramcse.10
Reporter | ||
Comment 5•8 years ago
|
||
Comment on attachment 8550317 [details] [diff] [review] Bug1122430_use_for_iteration.diff Thanks!
Attachment #8550317 -
Flags: review?(dao) → review+
Reporter | ||
Comment 6•8 years ago
|
||
(In reply to Vikram Jadhav from comment #1) > Where is the test file for testing this code? browser/base/content/test/general/browser_ctrlTab.js > and how can i find test file for any code for my future knowledge? > thanks. This is a little hard to tell. When modifying something in browser/base/, running all tests in browser/base/content/test/general/ or even browser/base/content/test/ is generally a good idea.
Assignee | ||
Comment 7•8 years ago
|
||
Hi, Is the patch is good? Working is yes then what should i do next.
Reporter | ||
Comment 8•8 years ago
|
||
Since you already ran tests, we can just add the checkin-needed keyword.
Keywords: checkin-needed
Assignee | ||
Comment 9•8 years ago
|
||
May i know what is the term "checkin" is? and i don't know how to add
Reporter | ||
Comment 10•8 years ago
|
||
(In reply to Vikram Jadhav from comment #9) > May i know what is the term "checkin" is? It means that the patch lands on one of our integration branches (and would eventually be merged to mozilla-central). > and i don't know how to add I already did it along with comment 8. The "Keywords" field is almost at the top of this page.
Comment 11•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/65325797108c
Keywords: checkin-needed
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/65325797108c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][lang=js][fixed-in-fx-team] → [good first bug][lang=js]
Target Milestone: --- → Firefox 38
You need to log in
before you can comment on or make changes to this bug.
Description
•