Closed
Bug 608165
Opened 14 years ago
Closed 14 years ago
Lots of 'reference to undefined property this.mAddon.applyBackgroundUpdates' warnings when running browser_bug562797.js in a debug build
Categories
(Toolkit :: Add-ons Manager, defect)
Tracking
()
VERIFIED
FIXED
mozilla2.0b7
People
(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)
Details
Attachments
(1 file, 2 obsolete files)
1.45 KB,
patch
|
robert.strong.bugs
:
review+
mossop
:
approval2.0+
|
Details | Diff | Splinter Review |
With a windows debug build the following two lines are printed to the console around 200 times each when running browser_bug562797.js JavaScript strict warning: chrome://mozapps/content/extensions/extensions.xml, line 1042: reference to undefined property this.mAddon.applyBackgroundUpdates JavaScript strict warning: chrome://mozapps/content/extensions/extensions.xml, line 1043: reference to undefined property this.mAddon.applyBackgroundUpdates the following two lines are printed to the console and the mochitest-browser-chrome.log around 200 times each when running browser_bug562797.js TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug562797.js | Console message: [JavaScript Warning: "reference to undefined property this.mAddon.applyBackgroundUpdates" {file: "chrome://mozapps/content/extensions/extensions.xml" line: 1042}] TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug562797.js | Console message: [JavaScript Warning: "reference to undefined property this.mAddon.applyBackgroundUpdates" {file: "chrome://mozapps/content/extensions/extensions.xml" line: 1043}] I didn't check if they were printed in other tests but I would assume they are
Assignee | ||
Comment 1•14 years ago
|
||
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
Attachment #486822 -
Flags: review?(dtownsend)
Assignee | ||
Updated•14 years ago
|
Summary: Losts of reference to undefined property this.mAddon.applyBackgroundUpdates warnings when running browser_bug562797.js in a debug build → Losts of 'reference to undefined property this.mAddon.applyBackgroundUpdates' warnings when running browser_bug562797.js in a debug build
Comment 2•14 years ago
|
||
Comment on attachment 486822 [details] [diff] [review] patch Elsewhere we treat a missing applyBackgroundUpdates property as equivalent to AUTOUPDATE_DISABLE so this should be: if (!("applyBackgroundUpdates" in this.mAddon) || ...
Attachment #486822 -
Flags: review?(dtownsend) → review-
Assignee | ||
Comment 3•14 years ago
|
||
The code in question with this change if (!("applyBackgroundUpdates" in this.mAddon) || (this.mAddon.applyBackgroundUpdates == AddonManager.AUTOUPDATE_DISABLE || (this.mAddon.applyBackgroundUpdates == AddonManager.AUTOUPDATE_DEFAULT && !AddonManager.autoUpdateDefault))) { var self = this; AddonManager.getAllInstalls(function(aInstallsList) { // This can return after the binding has been destroyed, // so try to detect that and return early if (!("onNewInstall" in self)) return; for (let i = 0; i < aInstallsList.length; i++) { let install = aInstallsList[i]; if (install.existingAddon && install.existingAddon.id == self.mAddon.id && install.state == AddonManager.STATE_AVAILABLE) { self.onNewInstall(install); self.onIncludeUpdateChanged(); } } }); } Doing it that way would not fix this bug since both sides of || would be evaluated Another option would be let applyBackgroundUpdates = !("applyBackgroundUpdates" in this.mAddon) ? AddonManager.AUTOUPDATE_DISABLE : this.mAddon.applyBackgroundUpdates; if (applyBackgroundUpdates == AddonManager.AUTOUPDATE_DISABLE || (applyBackgroundUpdates == AddonManager.AUTOUPDATE_DEFAULT && !AddonManager.autoUpdateDefault)) { var self = this; AddonManager.getAllInstalls(function(aInstallsList) { // This can return after the binding has been destroyed, // so try to detect that and return early if (!("onNewInstall" in self)) return; for (let i = 0; i < aInstallsList.length; i++) { let install = aInstallsList[i]; if (install.existingAddon && install.existingAddon.id == self.mAddon.id && install.state == AddonManager.STATE_AVAILABLE) { self.onNewInstall(install); self.onIncludeUpdateChanged(); } } }); }
Comment 4•14 years ago
|
||
(In reply to comment #3) > The code in question with this change > > if (!("applyBackgroundUpdates" in this.mAddon) || > (this.mAddon.applyBackgroundUpdates == AddonManager.AUTOUPDATE_DISABLE || > (this.mAddon.applyBackgroundUpdates == AddonManager.AUTOUPDATE_DEFAULT && > !AddonManager.autoUpdateDefault))) { > var self = this; > AddonManager.getAllInstalls(function(aInstallsList) { > // This can return after the binding has been destroyed, > // so try to detect that and return early > if (!("onNewInstall" in self)) > return; > for (let i = 0; i < aInstallsList.length; i++) { > let install = aInstallsList[i]; > if (install.existingAddon && > install.existingAddon.id == self.mAddon.id && > install.state == AddonManager.STATE_AVAILABLE) { > self.onNewInstall(install); > self.onIncludeUpdateChanged(); > } > } > }); > } > > Doing it that way would not fix this bug since both sides of || would be > evaluated I don't think so, the first half would be true if there was no property and so it'd go straight into the if block then, otherwise it will test the property as expected.
Updated•14 years ago
|
Summary: Losts of 'reference to undefined property this.mAddon.applyBackgroundUpdates' warnings when running browser_bug562797.js in a debug build → Lots of 'reference to undefined property this.mAddon.applyBackgroundUpdates' warnings when running browser_bug562797.js in a debug build
Assignee | ||
Comment 5•14 years ago
|
||
Each time I work til the wee hours and then make a stupid comment after waking up I tell myself that next time I should wait until after I've had a cup of coffee before making a comment. The trouble is that I'm half asleep and forgot my earlier advice. ;)
Attachment #486822 -
Attachment is obsolete: true
Attachment #486959 -
Flags: review?(dtownsend)
Comment 6•14 years ago
|
||
Comment on attachment 486959 [details] [diff] [review] patch >+ if (!("applyBackgroundUpdates" in this.mAddon) && I think you mean || ;)
Attachment #486959 -
Flags: review?(dtownsend) → review+
Assignee | ||
Comment 7•14 years ago
|
||
bah... need to wakeup!
Attachment #486959 -
Attachment is obsolete: true
Attachment #486962 -
Flags: review+
Assignee | ||
Comment 8•14 years ago
|
||
I verified that the tests work with the latest patch without the debug spew. btw: it also passed with the other two patches.
Assignee | ||
Comment 9•14 years ago
|
||
stops commenting until after coffee
Comment 10•14 years ago
|
||
Comment on attachment 486962 [details] [diff] [review] patch Explicit approval for b7 approved!
Attachment #486962 -
Flags: approval2.0+
Assignee | ||
Comment 11•14 years ago
|
||
Pushed to mozilla-central http://hg.mozilla.org/mozilla-central/rev/87e6cf44d406
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b7
You need to log in
before you can comment on or make changes to this bug.
Description
•