Closed
Bug 1293008
Opened 8 years ago
Closed 8 years ago
Replace in-tree consumer of non-standard Iterator() with Object.{values,entries} in suite/ in comm-central
Categories
(SeaMonkey :: General, defect)
SeaMonkey
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: arai, Assigned: dwivedi.aman96, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [good first bug] [lang=js])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
iannbugzilla
:
review+
arai
:
feedback+
|
Details | Diff | Splinter Review |
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%3Asuite%2F&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
Reporter | ||
Updated•8 years ago
|
Assignee | ||
Comment 1•8 years ago
|
||
Hi Tooru! I would like to take this up. Please assign it to me.
Flags: needinfo?(arai.unmht)
Reporter | ||
Comment 2•8 years ago
|
||
Great!
Assignee: nobody → dwivedi.aman96
Status: NEW → ASSIGNED
Flags: needinfo?(arai.unmht)
Assignee | ||
Comment 3•8 years ago
|
||
Please have a look at the patch! :)
Attachment #8823331 -
Flags: review?(arai.unmht)
Reporter | ||
Comment 4•8 years ago
|
||
Comment on attachment 8823331 [details] [diff] [review]
BugFix1293008.patch
Review of attachment 8823331 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks!
It's almost there.
Please update the patch and post again :)
::: suite/mailnews/tabmail.xml
@@ +353,4 @@
> this.setTabTitle(firstTabInfo);
> if (this.tabMonitors.length)
> {
> + for (let [i, tabMonitor] of Object.entries(this.tabMonitors))
(this is pre-existing tho)
since `i` is not used and `this.tabMonitors` is an array, you can use `for (let tabMonitor of this.tabMonitors)` here and below 3 places.
Attachment #8823331 -
Flags: review?(arai.unmht) → feedback+
Assignee | ||
Comment 5•8 years ago
|
||
This patch should work. Thanks for help. :)
Attachment #8823331 -
Attachment is obsolete: true
Attachment #8823620 -
Flags: review?(arai.unmht)
Reporter | ||
Comment 6•8 years ago
|
||
Comment on attachment 8823620 [details] [diff] [review]
BugFix1293008.patch
Review of attachment 8823620 [details] [diff] [review]:
-----------------------------------------------------------------
Thank you again!
it looks good.
forwarding to IanN.
Attachment #8823620 -
Flags: review?(iann_bugzilla)
Attachment #8823620 -
Flags: review?(arai.unmht)
Attachment #8823620 -
Flags: feedback+
Comment on attachment 8823620 [details] [diff] [review]
BugFix1293008.patch
r/a=me Thanks!
Attachment #8823620 -
Flags: review?(iann_bugzilla) → review+
Assignee | ||
Comment 8•8 years ago
|
||
Hi Tooru! What do we need to do now? This patch needs to be landed to mozilla code?
Flags: needinfo?(arai.unmht)
Reporter | ||
Comment 9•8 years ago
|
||
thank you for the heads up :)
given that the patch got review+ and approval, I'll push the patch to comm-central shortly.
Flags: needinfo?(arai.unmht)
Reporter | ||
Comment 10•8 years ago
|
||
https://hg.mozilla.org/comm-central/rev/2865d0a1c2a217952a6e5315b39835c2849d02b1
Bug 1293008 - Replace in-tree consumer of non-standard Iterator() with Object.{values,entries} in suite/ in comm-central, r=IanN, a=IanN
Reporter | ||
Updated•8 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•