291672 - Javascript strict warnings in update.js

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Jason Barnabe (np)]

Warning: redeclaration of var sbs
Source File: file:///C:/mozilla/bin/dist/bin/components/nsUpdateService.js
Line: 205, Column: 12
Source Code:
        var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]

Warning: redeclaration of var bundle
Source File: file:///C:/mozilla/bin/dist/bin/components/nsUpdateService.js
Line: 207, Column: 12
Source Code:
        var bundle =
sbs.createBundle("chrome://mozapps/locale/update/update.properties");
        
Warning: redeclaration of var ps
Source File: file:///C:/mozilla/bin/dist/bin/components/nsUpdateService.js
Line: 212, Column: 12
Source Code:
        var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
        
Warning: trailing comma is not legal in ECMA-262 object initializers
Source File: chrome://mozapps/content/update/update.js
Line: 86, Column: 4
Source Code:
    },
    
Warning: trailing comma is not legal in ECMA-262 object initializers
Source File: chrome://mozapps/content/update/update.js
Line: 90, Column: 4
Source Code:
    }
    
Warning: redeclaration of var item
Source File: chrome://mozapps/content/update/update.js
Line: 376, Column: 12
Source Code:
        var item = aSubject.QueryInterface(Components.interfaces.nsIUpdateItem);
        
Warning: redeclaration of var progress
Source File: chrome://mozapps/content/update/update.js
Line: 386, Column: 10
Source Code:
      var progress = document.getElementById("checking.progress");
      
Warning: redeclaration of var progress
Source File: chrome://mozapps/content/update/update.js
Line: 406, Column: 10
Source Code:
      var progress = document.getElementById("checking.progress");
      
Warning: redeclaration of var progress
Source File: chrome://mozapps/content/update/update.js
Line: 413, Column: 10
Source Code:
      var progress = document.getElementById("checking.progress");
      
Warning: redeclaration of var languages
Source File: chrome://mozapps/content/update/update.js
Line: 655, Column: 14
Source Code:
          var languages = this._newestInfo.getCollection("languages", { });
          
Warning: redeclaration of var i
Source File: chrome://mozapps/content/update/update.js
Line: 676, Column: 19
Source Code:
          for (var i = 0; i < components.length; ++i) {
          
Warning: redeclaration of var cr
Source File: chrome://mozapps/content/update/update.js
Line: 683, Column: 14
Source Code:
          var cr = Components.classes["@mozilla.org/chrome/chrome-registry;1"]
          
Warning: redeclaration of var selectedLocale
Source File: chrome://mozapps/content/update/update.js
Line: 685, Column: 14
Source Code:
          var selectedLocale = cr.getSelectedLocale("global");
          
Warning: redeclaration of var languages
Source File: chrome://mozapps/content/update/update.js
Line: 687, Column: 14
Source Code:
          var languages = this._newestInfo.getCollection("languages", { });
          
Warning: redeclaration of var i
Source File: chrome://mozapps/content/update/update.js
Line: 707, Column: 13
Source Code:
    for (var i = 0; i < kids.length; ++i) {
    
Warning: redeclaration of var i
Source File: chrome://mozapps/content/update/update.js
Line: 859, Column: 15
Source Code:
      for (var i = 0; i < gUpdateWizard.appComps.upgraded.core.length; ++i) {
      
Warning: redeclaration of var i
Source File: chrome://mozapps/content/update/update.js
Line: 863, Column: 15
Source Code:
      for (var i = 0; i < gUpdateWizard.appComps.upgraded.languages.length; ++i) {
      
Warning: redeclaration of var i
Source File: chrome://mozapps/content/update/update.js
Line: 867, Column: 15
Source Code:
      for (var i = 0; i < gUpdateWizard.appComps.upgraded.optional.length; ++i) {
      
Warning: redeclaration of var label
Source File: chrome://mozapps/content/update/update.js
Line: 893, Column: 10
Source Code:
      var label = strings.getFormattedString("installingPrefix",
[this._objs[aIndex].name]);
      
Warning: redeclaration of var actionItem
Source File: chrome://mozapps/content/update/update.js
Line: 894, Column: 10
Source Code:
      var actionItem = document.getElementById("actionItem");

Comment 1

[Jason Barnabe (np)]

Attachment: patch

This patch also removes/refactors some redundant code.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Why did you file a new bug instead of posting it in bug 249012? New EM, sure,
but there are a number of people CC'ed at the other bug already. Specifically,
Brendan's made the point that variable redeclaration isn't such a bad thing, and
that the solution might be to eliminate that warning. Until that decision has
been made, I think it's best to not touch the redeclarations.

Comment 3

[Jason Barnabe (np)]

(In reply to comment #2)
> Why did you file a new bug instead of posting it in bug 249012?
Because they deal with seperate files in different folders?

> Specifically,
> Brendan's made the point that variable redeclaration isn't such a bad thing, and
> that the solution might be to eliminate that warning. Until that decision has
> been made, I think it's best to not touch the redeclarations.
He said "I've talked to Ben about this, and we agree that such warnings should
not be "fixed" by minimizing var declarations to the earliest point in the
file". Many of the the fixes for variable declarations in this patch are
removing and refactoring redundant code. If it's decided (actually decided and
implemented, not someone saying they *might* do something a month ago) that
variable redeclarations are fine, I'll revise this patch to not include the
earliest-point-in-file fixes, but to keep the this-code-should-be-refactored fixes.

Comment 4

[Jason Barnabe (np)]

Attachment: patch v2

Takes care of the trailing comma errors and refactors some redundant code that
used to cause warnings.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 182915 [details] [diff] [review]
patch v2

This is bitrotted, thanks to the Extension work recently checked in. Jason, did
you want to look if there was anything left to fix?

Comment 6

[Jason Barnabe (np)]

The only thing left from the original patch is the redundant progress update
stuff, and even that has been reduced from four occurences to two. There may be
some more warnings that come up with the new code, but I'd rather just wait
until the major changes are done to check for those things.