mochitest-plain-1: "Error: useless setTimeout call (missing quotes around argument?)" at "file_bug546995.html : 28"

VERIFIED FIXED in mozilla2.0b7

Status

()

Core
Layout: Form Controls
--
minor
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: sgautherie, Assigned: Callek)

Tracking

Trunk
mozilla2.0b7
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

v2
798 bytes, patch
sicking
: review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
Noticed in Error Console, when run on my local Win2K:
{
Error: useless setTimeout call (missing quotes around argument?)
Source File: http://mochi.test:8888/tests/content/html/content/test/file_bug546995.html
Line: 28
}

Code is
{
28   setTimeout(parent.gGen.next(), 0);
}
It probably should not have parenthesis. (Or is setTimeout() actually useless?)
Maybe SimpletTest.executeSoon would be more appropriate.
(Reporter)

Comment 2

7 years ago
(In reply to comment #1)
> Maybe SimpletTest.executeSoon would be more appropriate.

I usually prefer it. Yet this "bug" should be the same with either function.
(Assignee)

Updated

7 years ago
Assignee: nobody → bugspam.Callek
(Assignee)

Comment 3

7 years ago
Created attachment 478991 [details] [diff] [review]
v1

Use executeSoon and fix
Attachment #478991 - Flags: review?(mounir.lamouri)
Attachment #478991 - Flags: review?(mounir.lamouri) → review+
Comment on attachment 478991 [details] [diff] [review]
v1

Does this really work? Looks to me like it'll execute the 'next' function but with the global object as 'this'. You have to do something like:

SimpleTest.executeSoon(parent.gGen.next.bind(parent.gGen.next))
Attachment #478991 - Flags: review-
(Assignee)

Comment 5

7 years ago
(In reply to comment #4)
> Comment on attachment 478991 [details] [diff] [review]
> v1
> 
> Does this really work? Looks to me like it'll execute the 'next' function but
> with the global object as 'this'. You have to do something like:
> 
> SimpleTest.executeSoon(parent.gGen.next.bind(parent.gGen.next))

I thought executeSoon did that magic for us... to make sure the right scopes are used. Is that not so? and/or should I take your r- as "use this code I provided and if you do r+"?
(Assignee)

Comment 6

7 years ago
(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 478991 [details] [diff] [review] [details]
> > v1
> > 
> > Does this really work? Looks to me like it'll execute the 'next' function but
> > with the global object as 'this'. You have to do something like:
> > 
> > SimpleTest.executeSoon(parent.gGen.next.bind(parent.gGen.next))
> 
> I thought executeSoon did that magic for us... to make sure the right scopes
> are used. Is that not so? and/or should I take your r- as "use this code I
> provided and if you do r+"?

yea, I'm not sure you are right sicking, but I'm a bit too tired to triple check the JS Syntax differences and if/how they matter in this case: http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#512
I don't see that executeSoon could do the magic we want since it's only given a reference to Iterator.next, from which there is no way to track back to the actual parent.gGen object.

That said, my r- means "this looks fishy, please double-check that it does what you want it to".

In fact, I'm surprised that the test is passing both before and after this patch? Shouldn't it be failing if the next() function isn't called when it's supposed to?
(Assignee)

Comment 8

7 years ago
Created attachment 479720 [details] [diff] [review]
v2

Turns out this bind stuff is actually needed, and an OMG I forgot to test this notice when I get errors on SimpleTest not being defined in scope.
Attachment #478991 - Attachment is obsolete: true
Attachment #479720 - Flags: review?(jonas)
(Assignee)

Comment 9

7 years ago
sicking, ping? [only because you told me over IRC you would get to it within 24 hours (so on the first)]
Pushed:
http://hg.mozilla.org/mozilla-central/rev/a88c80b1b471
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
OS: Windows 2000 → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
(Reporter)

Comment 11

7 years ago
V.Fixed, per
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286468200.1286471496.20397.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2010/10/07 09:16:40
Status: RESOLVED → VERIFIED
Summary: mochitests: "Error: useless setTimeout call (missing quotes around argument?)" at "file_bug546995.html : 28" → mochitest-plain-1: "Error: useless setTimeout call (missing quotes around argument?)" at "file_bug546995.html : 28"
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.