Closed Bug 1450066 Opened 4 years ago Closed 3 years ago

setInterval fires only once when no delay argument provided


(Core :: DOM: Core & HTML, defect, P2)

59 Branch



Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox60 --- wontfix
firefox61 --- fixed


(Reporter: aaron.peltz, Assigned: farre)



(Keywords: regression)


(2 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.66 Safari/537.36

Steps to reproduce:

It seems that failing to provide a delay value to setInterval results in the callback being called once. I would have expected an error or warning, if not the minimum delay as a default if no delay argument is not provided.

To reproduce:
// In console
setInterval(function(){ console.log(}, 0) // works

setInterval(function(){ console.log(}) // called once

Similar to
This is reproducible for me. Putting this under Core:Dom so dev can look into it.
Component: Untriaged → DOM
Product: Firefox → Core
This seems intentional.  I traced it back as far as bug 918345, but it seems doubtful that introduced it.
At first glance it does look like bug 918345 might have changed the semantics...  Certainly nightlies from 2013-10-27 don't show the "called once" behavior... though neither do ones from 2013-10-31.
Looks like bug 918345 added the new codepaths but did not enable them by default.  That happened for the first time in bug 789261.

That said, if I take the first nightly after bug 1005966 lands (not showing this issue) and flip the pref, it still does not show the issue afaict.
Both chrome and edge fire repeated intervals when the argument is left off.  I haven't tested safari.  We should probably remove the code in IsInterval() and just treat a missing arg as zero.
Priority: -- → P2
farre, do you think you had time for this?
Flags: needinfo?(afarre)
Assignee: nobody → afarre
Flags: needinfo?(afarre)

Easy enough to fix. The spec glances over this, so I filed a spec bug to ask for clarification[1]

Attachment #8972849 - Flags: review?(bkelly)
Scratch that. The spec is actually very clear, default timeout value for setInterval is 0:
Comment on attachment 8972849 [details] [diff] [review]

I'll update the WindowOrWorkerGlobalScope idl to have default arguments for setInterval.
Attachment #8972849 - Flags: review?(bkelly) → review-
Comment on attachment 8972850 [details] [diff] [review]

Review of attachment 8972850 [details] [diff] [review]:

::: testing/web-platform/tests/html/webappapis/timers/missing-timeout-setinterval.any.js
@@ +1,4 @@
> +function timeout_trampoline(t, timeout, message) {
> +  t.step_timeout(function() {
> +    // Yield in case we managed to be called before the second interval callback.
> +    t.step_timeout(function() {

Can you explain why we need two timeouts here?

@@ +10,5 @@
> +async_test(function(t) {
> +  let ctr = 0;
> +  let h = setInterval(t.step_func(function() {
> +    if (++ctr == 2) {
> +      clearTimeout(h);

Shouldn't these be clearInterval()?
Attachment #8972850 - Flags: review?(bkelly) → review+
Comment on attachment 8972865 [details] [diff] [review]

Review of attachment 8972865 [details] [diff] [review]:

Attachment #8972865 - Flags: review?(bkelly) → review+

Avoid using the fact that clearTimeout is interchangeable with clearInterval, it's confusing.

Carrying over r+.
Attachment #8972850 - Attachment is obsolete: true
Attachment #8973165 - Flags: review+
Keywords: checkin-needed
Pushed by
Fall back to 0 if setInterval interval not supplied. r=bkelly
Test that setInterval handles missing interval argument. r=bkelly
Keywords: checkin-needed
Created web-platform-tests PR for changes under testing/web-platform/tests
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.