Closed Bug 597887 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Layout: Form Controls, defect, minor)

defect
Not set
minor

Tracking

()

VERIFIED FIXED
mozilla2.0b7

People

(Reporter: sgautherie, Assigned: Callek)

References

()

Details

Attachments

(1 file, 1 obsolete file)

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.
(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: nobody → bugspam.Callek
Attached patch v1 (obsolete) — Splinter Review
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-
(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+"?
(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?
Attached patch v2Splinter Review
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)
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
Closed: 10 years ago
Flags: in-testsuite+
OS: Windows 2000 → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
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.