Closed Bug 1215201 Opened 7 years ago Closed 7 years ago

callback is null in ext-alarms.

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: bwinton, Assigned: bwinton)

Details

Attachments

(1 file)

So, I'm digging in to why my alarms aren't firing, and I came across this.
It looks like we should be using for…of instead of for…in.
(I even added tests!  ;)
Attachment #8674395 - Flags: review?(wmccloskey)
Comment on attachment 8674395 [details] [diff] [review]
The first attempt.

Review of attachment 8674395 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: toolkit/components/extensions/test/mochitest/test_ext_alarms.html
@@ +13,5 @@
> +<script type="application/javascript;version=1.8">
> +
> +add_task(function* test_contentscript() {
> +  function backgroundScript() {
> +    browser.test.log('running alarm script');

Please use double quotes for all string literals.

@@ +22,5 @@
> +    });
> +    chrome.alarms.create('test_ext_alarms', {delayInMinutes: 0.02});
> +    setTimeout(() => {
> +      browser.test.notifyFail('alarms test failed, took too long');
> +    }, 3000);

This doesn't give us a lot of margin for error on slow test machines. Please make this 10 seconds or remove it entirely.
Attachment #8674395 - Flags: review?(wmccloskey) → review+
Done and done.

I also changed the:
  add_task(function* test_contentscript() {
line to
  add_task(function* test_alarm_fires() {
cause we're not really testing the contentscript here.
(It looks like that line was copied and pasted in a bunch of files, but I don't think it's worth cleaning up in this patch… :)
https://hg.mozilla.org/mozilla-central/rev/b0fc271e4897
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.