Closed
Bug 479634
Opened 16 years ago
Closed 16 years ago
Multiple declarations of loop variable in tabbrowser.xml
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 3.6a1
People
(Reporter: john.p.baker, Assigned: john.p.baker)
References
Details
Attachments
(1 file, 1 obsolete file)
13.17 KB,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; riscos arm-acorn; en-US; rv:1.8b4) Gecko/20050723 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; riscos arm-acorn; en-US; rv:1.8b4) Gecko/20050723 Firefox/1.0+
Bug 463387 introduced progress notifications for tabs by copying similar existing code; This means that there are now multiple declarations for var i:
for (var i = 0; i < this.mTabBrowser.mProgressListeners.length; i++) {
...
for (var i = 0; i < this.mTabBrowser.mTabsProgressListeners.length; i++) {
IIUC this is not good practice and in all these cases both the |var i = 0;| should be replaced by |let i = 0;|
Reproducible: Always
Assignee | ||
Comment 1•16 years ago
|
||
In fact the same applies to the var p declaration in the loops.
Assignee | ||
Comment 2•16 years ago
|
||
Assignee | ||
Comment 3•16 years ago
|
||
As well as replacing 'var' with 'let' - as comment #0 - I have made the comments standard as they have become inconsistent with the code duplication; Originally I wrote them (a) to indicate if there was any following code which could get broken on a Javascript error and (b) so that the catch block wouldn't be empty.
Attachment #367759 -
Attachment is obsolete: true
Attachment #373623 -
Flags: review?(gavin.sharp)
Updated•16 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•16 years ago
|
Attachment #373623 -
Flags: review?(gavin.sharp) → review?(dao)
Assignee | ||
Comment 4•16 years ago
|
||
Comment on attachment 373623 [details] [diff] [review]
patch
Rotating review request to Dao.
Updated•16 years ago
|
Attachment #373623 -
Flags: review?(dao) → review+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Updated•16 years ago
|
Assignee: nobody → john.p.baker
Comment 5•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.6a1
You need to log in
before you can comment on or make changes to this bug.
Description
•