337951 - Action of setTimeout("...",Infinity) is inconsistent between MS Win(timeout not fire) and Linux(timeout fires as if Infinity==0)

Status: RESOLVED FIXED

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

Description

[WADA:World Anti-bad-Duping Agency]

Spin off of Bug 332759 Comment #49 Remaining questions (2).

Action of setTimeout("...",Infinity) is inconsistent between MS Win and Linux.
  MS Win: Timeout will not fire (possibly long timeout value)
  Linux : Timeout fires as if Infinity==0
 (Mac OS: Sorry but I don't know)
This may be a result of difference of spec of timer routines of OS.

I think non number millisecond value for setTimeout and setInterval is to be (A) rejected as "Invalid parameter" or (B) defaulted to several seconds or longer if non number is acepted.  
This will reduce impact of(when B) or never cause(when A) problem of infinite loop by "function x(){setTimeout(x);}x();" or variations of it.
And illegal number for millisecond value such as Infinity/-Infinity/Negative number is also to be rejected or defaulted to apprriate value instead of ZERO.
Further, ZERO as millisecond value is to be rejected or defaulted to appropriate(safer) value.

Comment 1

[Fibonacci]

Attachment: Minimum test case

Timeout does not fire in this testcase.

Comment 2

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #1)
> Minimum test case
> Timeout does not fire in this testcase.

Funny, your test case looks to be same as test case used in Bug 332759 Comment #45 and Bug 332759 Comment #50, where timeout fired for Infinity, except your case is "setTimeout with Infinity" only (and lack of test[0]=xx; in a function which has no relation to result.) 

Fibonacci, can you report result on Linux of simpler case?
(1)CTRL+T (open new tab)
(2)javascript:var msg="hello";var timerID=setTimeout("alert(msg);",Infinity);
(3)Wait for a moment
(4)javascript:alert(timerID);
And clearly write your Firefox's build ID, since test result sometimes depended on build ID in tests for Bug 332759. 

Comment 3

[Fibonacci]

(In reply to comment #2)
> Funny, your test case looks to be same as test case used in Bug 332759 Comment
> #45 and Bug 332759 Comment #50, where timeout fired for Infinity, except your
> case is "setTimeout with Infinity" only (and lack of test[0]=xx; in a function
> which has no relation to result.) 

That's what puzzles me the most.

> Fibonacci, can you report result on Linux of simpler case?
> (1)CTRL+T (open new tab)
> (2)javascript:var msg="hello";var timerID=setTimeout("alert(msg);",Infinity);
> (3)Wait for a moment
> (4)javascript:alert(timerID);

Only one alert box: 2.
But this may be due to the fact that Javascript:setTimeout("alert(...);",any_value_here); does not EVER fire timeout when run from the URL bar.

> And clearly write your Firefox's build ID, since test result sometimes depended
> on build ID in tests for Bug 332759. 

And where can I find that, excuse me?

Comment 4

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #3)
> > And clearly write your Firefox's build ID, since test result sometimes depended
> > on build ID in tests for Bug 332759. 
> And where can I find that, excuse me?
Oh sorry, you were not nightly tester.
Useragent string is sufficient. (Displayed in result of your "Minimum test case")

Comment 5

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #3)
> Javascript:setTimeout("alert(...);",any_value_here); does not EVER fire timeout
> when run from the URL bar.
Do you enable pop up blocker? If yes, please disable pop up blocker before test.
If other extentions such as AdBlock is enabled, please disable too.
Javascript:setTimeout("alert(...);",100); still does not display alert?

Or execute test without alert in script, please.
(1)CTRL+T (2)javascript:var x=0;var timerID=setTimeout('x++;',some_value);
(3)Wait for moment (4) javascript:alert(x);

Comment 6

[Fibonacci]

(In reply to comment #4)
> Useragent string is sufficient. (Displayed in result of your "Minimum test
> case")

userAgent: Mozilla/5.0 (X11; U; Linux i686; it; rv:1.8.0.1) Gecko/20060313 Fedora/1.5.0.1-9 Firefox/1.5.0.3 pango-text

(In reply to comment #5)
> Do you enable pop up blocker? If yes, please disable pop up blocker before
> test.

I do have it enabled, but still, Javascript:alert("something"); DOES work from the URL bar.

Still, it seems that timeout doesn't fire, since I see no alert box.

> If other extentions such as AdBlock is enabled, please disable too.

Don't have that.

> Javascript:setTimeout("alert(...);",100); still does not display alert?

It does. Even Javascript:setTimeout("alert(...);",zero_or_any_negative_number_including_minus_Infinity); displays an alert.

> 
> Or execute test without alert in script, please.
> (1)CTRL+T (2)javascript:var x=0;var timerID=setTimeout('x++;',some_value);
> (3)Wait for moment (4) javascript:alert(x);

It fires with any value strictly less than Infinity, but not with Infinity itself.

Comment 7

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #6)
> userAgent: Mozilla/5.0 (X11; U; Linux i686; it; rv:1.8.0.1) Gecko/20060313
> Fedora/1.5.0.1-9 Firefox/1.5.0.3 pango-text

> > Or execute test without alert in script, please.
> > (1)CTRL+T (2)javascript:var x=0;var timerID=setTimeout('x++;',some_value);
> > (3)Wait for moment (4) javascript:alert(x);
> It fires with any value strictly less than Infinity, but not with Infinity
> itself.

When Bug 332759 Comment #45, your agent string was:
> userAgent: Mozilla/5.0 (X11; U; Linux i686; it; rv:1.8.0.1) Gecko/20060313
> Fedora/1.5.0.1-9 Firefox/1.5.0.2 pango-text
and result was:
> 7. Timestamp=25 : Test_Case=7. Infinity/ timerID=8 
>                                / Timeout fired. ( Timestamp=121 )

When Bug 332759 Comment #50, your agent string was:
> userAgent: Mozilla/5.0 (X11; U; Linux i686; it; rv:1.8.0.1) Gecko/20060313
> Fedora/1.5.0.1-9 Firefox/1.5.0.3 pango-text
and result was:
> 7. Timestamp=8 : Test_Case=7. Infinity/ timerID=8 
>                               / Timeout fired. ( Timestamp=88 )

Even same Firefox as Bug 332759 Comment #50, why timeout won't fire when javascript:var x=0;var timerID=setTimeout('x++;',Infinity); ?
Why timeout won't fire when your test case?
Test on different machine or different OS or different OS version?
Garbage is used when Infinity?

Fibonacci, test with latest-trunk nightly, please.  

Comment 8

[Fibonacci]

(In reply to comment #7)
> (In reply to comment #6)
> > userAgent: Mozilla/5.0 (X11; U; Linux i686; it; rv:1.8.0.1) Gecko/20060313
> > Fedora/1.5.0.1-9 Firefox/1.5.0.3 pango-text
> 
> > > Or execute test without alert in script, please.
> > > (1)CTRL+T (2)javascript:var x=0;var timerID=setTimeout('x++;',some_value);
> > > (3)Wait for moment (4) javascript:alert(x);
> > It fires with any value strictly less than Infinity, but not with Infinity
> > itself.
> 
> When Bug 332759 Comment #45, your agent string was:
> > userAgent: Mozilla/5.0 (X11; U; Linux i686; it; rv:1.8.0.1) Gecko/20060313
> > Fedora/1.5.0.1-9 Firefox/1.5.0.2 pango-text
> and result was:
> > 7. Timestamp=25 : Test_Case=7. Infinity/ timerID=8 
> >                                / Timeout fired. ( Timestamp=121 )
> 
> When Bug 332759 Comment #50, your agent string was:
> > userAgent: Mozilla/5.0 (X11; U; Linux i686; it; rv:1.8.0.1) Gecko/20060313
> > Fedora/1.5.0.1-9 Firefox/1.5.0.3 pango-text
> and result was:
> > 7. Timestamp=8 : Test_Case=7. Infinity/ timerID=8 
> >                               / Timeout fired. ( Timestamp=88 )
> 
> Even same Firefox as Bug 332759 Comment #50, why timeout won't fire when
> javascript:var x=0;var timerID=setTimeout('x++;',Infinity); ?
> Why timeout won't fire when your test case?
> Test on different machine or different OS or different OS version?

But... the bug is supposed to be Linux-exclusive. The only way I can think of, would be to boot Knoppix and test it there... I'll try it when I reboot this machine.

> Garbage is used when Infinity?

Doubt it, it consistently fires in your test case, while it consistently fails in mine. Tested several times.

> Fibonacci, test with latest-trunk nightly, please.  

And where can I get that, pardon me?

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Could this be the result of Windows implementing comparison with NaN/Infinity differently from Linux?

nsGlobalWindow::SetTimeoutOrInterval currently calls JS_ValueToNumber, which could very well return jsNaN. I wonder if calling JS_ValueToECMAInt32 or kin might be better, if it isn't too much of a compatibility break.

Comment 10

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #8)
> where can I get that
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/

Comment 11

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #9)
> Could this be the result of Windows implementing comparison with NaN/Infinity
> differently from Linux?
You are right. This is bug report based on test result of a test case(test of
illegal millisecond value of setTimeout. i.e. test of NaN/Infinity) for problem
analysis of Bug 332759. (https://bugzilla.mozilla.org/attachment.cgi?id=220374)
  Bug 332759 Comment #44(MS Win) and Bug 332759 Comment #45(Linux).

> if it isn't too much of a compatibility break.

I could find only 2 compatibility break cases.
 This bug(Infinity) and Bug 337956(just be defined by "Variable Instantiation")
And as you seen, this bug is NOT a problem which always occurs on any build on
any Linux. And as described in Bug 332759, Bug 337956's case is not recreated
when my coding example, although original test case and javascript: case
produces the problem.
So there is no "too much compatibility break" currently, I think.

Comment 12

[Brendan Eich [:brendan]]

The code that depends on the return value of JS_ValueToNumber called within nsGlobalWindow::SetTimeoutOrInterval is not portable.  It tests < and > to bound interval, but if interval is a NaN both tests result in false.  Then it casts interval to (PRInt32), which IIRC has non-portable result.

At the least, non-finite values should be treated specially.  The JS API does not expose a way to test for these, but jsnum.h does.  I'm ok with including that file and using JSDOUBLE_IS_FINITE (Remember that you can't compare NaN == NaN and get true in order to detect NaNs).  Or what mrbkap proposed, if it's compatible enough with what we do in the cases that matter (NaN, -Infinity, Infinity, and anything else), and with what other browsers do.

Need a normative spec that standardizes the expected behavior here!

/be

Comment 13

[Hixie (not reading bugmail)]

As far as I can tell, IE just treats ±0, ±Infinity, and NaN as zero, and thus as "fire ASAP". It treats negative numbers as actual negative numbers. So the results for:
   http://www.hixie.ch/tests/adhoc/dom/level0/timers/002-demo.html
   http://www.hixie.ch/tests/adhoc/dom/level0/timers/003-demo.html
...are -100, -1, (-Infinity, -0, NaN, 0, Infinity, in the order specified), 1, 100. I think we should do that.

TESTCASES
   http://www.hixie.ch/tests/adhoc/dom/level0/timers/002.html
   http://www.hixie.ch/tests/adhoc/dom/level0/timers/003.html

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Stupid-easy fix

So, this doesn't really follow IE's example. Because we have the minimum timeout value, we can't really fire all of the negative values first, followed by 0 values, followed by "real" values without some more work that I don't think is worth it. JS_ValueToECMAInt32 will convert all NaN values to 0, so it'll give us the desired cross-platform compatibility, if not the cross-browser compatibility.

Comment 15

[Johnny Stenback (:jst)]

Comment on attachment 222352 [details] [diff] [review]
Stupid-easy fix

r+sr=jst

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Followup sign correctness patch

I noticed this too late, but I think we want this; mInterval's uses are all casts to PRTime, which is a signed  64 bit integer, so a PRUint32 should fit nicely (with the exception of the largest possible PRUint32, which is filtered out earlier anyway).

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Attachment 222352 [details] [diff] is checked into the trunk, marking this bug as FIXED.

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

Followup fix checked in.