132904 - Clearing timeout without setting one & without passing timeoutID throws exception.

Status: VERIFIED FIXED

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

Description

[Prashant Desale]

If we use "clearTimeout()" method without passing "timeoutID" to it. [Which is  
wrong because "clearTimeout()" method expects "timeoutID" as argument in order 
to clear that particular timeout.]

It throws an exception. Filing this bug is wrong in itself because throwing 
exception for it is correct thing to do.
However 4.x browser works fine even if we use "clearTimeout()" method without 
passing "timeoutID", so it raises questions about all those webpages out there 
which use "clearTimeout()" method without passing "timeoutID" & without setting 
timeout.

for examples, please have a look at "bugscape bug# 9521".

I know best component to assign this bug to is "Evangelism", but I just talked 
to Johnney & he said he could fix it, so I'm assgining it to dom-level-0.

STEPS TO REPRODUCE:
1] Visit http://bubblegum/desale/bug9521.html
OR
1] Load testcase I'm going to attach.
2] Click button 'Try clearing Timeout without setting one'

You will see alert showing exception we are catching. 
Please have a look at source of testcase & you will see that I'm calling 
cleartimeout() without passing timeoutID to it. [Absolutely wrong thing to do]

Well Standards say we should pass timeoutID.

So now we have to decide if we want to stick to standards or we want not to 
throw exception, so all those webpages out there work fine as they used to work 
with 4.x.

Testcase attachment following.

Comment 1

[Prashant Desale]

Attachment: same testcase as in above URL

Comment 2

[Prashant Desale]

nsbeta1, priority p2. ccing Bob, Arun, Lisa & paul.

Comment 3

[Bob Clary [:bc] (inactive)]

IE doesn't throw an exception when calling clearTimeout with no argument either.
Note that the test case throws an exception in IE due to it's use of 'start'.

Do we know of any pages/apis which rely on this behavior? I haven't seen this in
my incidents. 

Comment 4

[Johnny Stenback (:jst)]

Transfering nsbeta1+ from the bugscape bug.

Comment 5

[Johnny Stenback (:jst)]

Attachment: Proposed fix.

This patch makes clearTimeout() and clearInterval() callable w/o an argument,
or with any type of argument w/o us throwing an exception, just like 4x and IE
do.

Comment 6

[Johnny Stenback (:jst)]

I've seen bugs on this earlier, I can't say how common it is, but it's easy
enough to fix and it's one less backwards incompatibility to worry about, so I
say let's fix it.

Comment 7

[Prashant Desale]

I also think fixing this one would be right call since we move one step forward
towards backward compatibility. I also say Lets fix it.

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 75680 [details] [diff] [review]
Proposed fix.

jband says sr=jband

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 75680 [details] [diff] [review]
Proposed fix.

You might wanna keep the reference to bug 30700 (and this one). Either way

r=sicking

Comment 10

[Phil Schwartau]

See bug 58101 and all its duplicates for important examples
of this problem -

Comment 11

[Phil Schwartau]

Johnny: will this patch allow users to do clearTimeout(TimeoutID)
where TimeoutID is uninitialized?  i.e. where the variable provided
contains the value |undefined|? That crops up from time to time ...

Comment 12

[Johnny Stenback (:jst)]

Yes, this patch takes care of that problem too.

Comment 13

[lchiang]

Thank you for the fast action on this.

Comment 14

[Fabian Guisset]

Comment on attachment 75680 [details] [diff] [review]
Proposed fix.

>+  /**
>+   * These methods take one optional argument that's the timer ID to
>+   * clear. Often in existing code these methods afre passed
>+   * undefined, which is a nop so we need to support that as well.
>+   */
Please fix this comment (s/afre/are), also is "nop" a real word?

>+  if (argc < 1) {
>+    // No arguments, return early.

Is this what other browsers do? What's the point of calling it without argument
if it does nothing?

Comment 15

[Johnny Stenback (:jst)]

Calling clearTimeout() w/o arguments is obviously pointless, but we need to not
throw an exception to be compatible with other browsers.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 75680 [details] [diff] [review]
Proposed fix.

a=dbaron for trunk checkin

Comment 17

[Johnny Stenback (:jst)]

Fixed.

Comment 18

[Prashant Desale]

Verified on 2002-05-23-08-trunk.

Comment 19

[Dzmitry Malyshau [:kvark]]

Attachment: _irrelevant_

Comment 20

[Dzmitry Malyshau [:kvark]]

Comment on attachment 8825096 [details]
_irrelevant_

attached by mistake, marking as obsolete