Closed Bug 813801 Opened 8 years ago Closed 8 years ago

null-check blocklistWindow in nsBlocklistService.js


(Toolkit :: Add-ons Manager, defect)

Not set





(Reporter: Unfocused, Assigned: car)


(Whiteboard: [good first bug][][lang=js])


(1 file)

It should only be possible to hit this in xpcshell tests, but it'd be nice to at least not have this littering the logs:

*** WARN addons.manager: Exception calling callback: TypeError: blocklistWindow is null

This happens because the xpcshell tests can't open windows, so the open() call returns null. 

Just need a null check in there:

No test needed, though bonus points if you manually verify that the above warning shops showing when running the following xpcshell test:
Attached patch add a null checkSplinter Review
I didn't test, can you please guide me how to run test? Thanks!
Attachment #684837 - Flags: review?(bmcbride)
Great :) I'll give you a chance to test it before I review your patch.

(In reply to zeyu [:car] from comment #1)
> I didn't test, can you please guide me how to run test? Thanks!

You can run just that test via running the following in your repository:

  mach xpcshell-test toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js

Or if you remove just the filename, keeping the path, it'll run all xpcshell tests in that directory.

The following page has all the gritty details, though it describes the old (more complex) way of running this type of test:
Assignee: nobody → zeyufly
Hi Blair,

I tried to run the test, however it seems that the number of lines that can be displayed in terminal is limited so that I cannot see the full log. I also tried 

./mach -l log.log xpcshell-test toolkit/mozapps/extensions/test/xpcshell /test_blocklistchange.js

it only gave me an empty file. So how can I see the full test log? Thanks! BTW, I am running Ubuntu 12.04
Mach doesn't currently log test output. I suppose we should file a bug on that.

You can always do something like:

./mach xpcshell-test path/to/test > log.log
I've run the test and I believe the warning was gone.
Comment on attachment 684837 [details] [diff] [review]
add a null check

Taking this since Blair is out sick today
Attachment #684837 - Flags: review?(bmcbride) → review?(dtownsend+bugmail)
Comment on attachment 684837 [details] [diff] [review]
add a null check

Looks good. One minor style nit, the rest of this file doesn't use braces around single statements like this. I've gone ahead and corrected that and will land it in a moment.

Thanks for the patch!
Attachment #684837 - Flags: review?(dtownsend+bugmail) → review+
Landed on inbound:
Target Milestone: --- → mozilla20
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.