setTimeout() is still called after window is closed

VERIFIED DUPLICATE of bug 15261

Status

()

defect
P1
blocker
VERIFIED DUPLICATE of bug 15261
20 years ago
18 years ago

People

(Reporter: dougt, Assigned: rogerl)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Reporter

Description

20 years ago
This javascript crashes in nsJSEnvironment.cpp, line 80 or so, because 'owner'
is invalid:

<html>
<script language=javascript>

var doclose=false;

function closewin()
{
    doclose = true;
}

function closeif()
{
    if (doclose)
    {
        window.close();
    }
    setTimeout("closeif()", 100);
}

setTimeout("closeif()", 100);

</script>

<form>
<input type="button" value="TouchMe" onClick="closewin()">
</form>
</body>
</html>
Reporter

Updated

20 years ago
Priority: P3 → P1
Target Milestone: M11
Reporter

Comment 1

20 years ago
proposed fix:

Index: nsJSEnvironment.cpp
===================================================================
RCS file: /cvsroot/mozilla/dom/src/base/nsJSEnvironment.cpp,v
retrieving revision 1.48
diff -c -4 -r1.48 nsJSEnvironment.cpp
*** nsJSEnvironment.cpp 1999/09/17 20:13:24     1.48
--- nsJSEnvironment.cpp 1999/09/30 21:36:00
***************
*** 65,73 ****
    nsIScriptContextOwner* owner;

    if (context) {
      nsresult result = context->GetOwner(&owner);
!     if (NS_SUCCEEDED(result)) {
        const char* error;
        if (message) {
          error = message;
        }
--- 65,73 ----
    nsIScriptContextOwner* owner;

    if (context) {
      nsresult result = context->GetOwner(&owner);
!     if (NS_SUCCEEDED(result) && owner) {
        const char* error;
        if (message) {
          error = message;
        }

Updated

20 years ago
Status: NEW → ASSIGNED

Comment 2

20 years ago
Well, I'd like to see why the timeout isn't canceled in time. Looking at the bug
right now...

Comment 3

20 years ago
The window.close() in the timeout is running synchronously (this is different
from 4.x, where the JS<->UI thread separation changed the timing of this) and
causing a JS_ClearScope to be called while there's still more script to be
executed. In this case, the setTimeout following the if block in the callback
fails (it wouldn't if it were surrounded by an else clause) because it's been
cleared. As a result, an exception is thrown. We crash in jsexn.c, trying to
convert the uncaught exception into a string. The following patch, along with
dougt's into nsJSException will fix the crash:

Index: jsexn.c
===================================================================
RCS file: /cvsroot/mozilla/js/src/jsexn.c,v
retrieving revision 3.8
diff -r3.8 jsexn.c
617a618
>     const char* bytes;
637a639,645
>     if (str != NULL) {
>         bytes = js_GetStringBytes(str);
>     }
>     else {
>         bytes = "null";
>     }
>
651c659
<                              JSMSG_UNCAUGHT_EXCEPTION, js_GetStringBytes(str))
;
---
>                              JSMSG_UNCAUGHT_EXCEPTION, bytes);
655c663
<         js_ReportErrorAgain(cx, js_GetStringBytes(str), reportp);
---
>         js_ReportErrorAgain(cx, bytes, reportp);

Comment 4

20 years ago
Reassigning to rogerl to review, update and checkin the suggested patch to
js/src.

DougT - please check in your suggested change citing me as the reviewer.

Still need to think about other ramifications of running the window.close()
synchronously. Brendan, any thoughts?
Assignee

Updated

20 years ago
Status: NEW → RESOLVED
Last Resolved: 20 years ago
Resolution: --- → DUPLICATE
Assignee

Comment 5

20 years ago
This seems to be a duplicate of bug 15261? while the suggested fix is kosher, my
concern is tracking down the origin of the null string itself.

*** This bug has been marked as a duplicate of 15261 ***
any ETA on when this patch (or something like it) will be checked in?

this it bitting me in a number of places, and window.close() related.
Assignee

Comment 7

20 years ago
So far I'm getting nowhere on reproducing the null exception string here at
home, if you want to apply vidur's patch to get you going I have no objection,
but I expect to have this resolved tonight or tomorrow morning.
I've checked in vidur's patch, with rogerl's blessing.

rogerl said he would continue investigating to see why its happening.
Reporter

Updated

20 years ago
Status: RESOLVED → VERIFIED
Reporter

Comment 9

20 years ago
doesn't happen for me anymore.  thanks.

Updated

20 years ago
Component: Java APIs to WebShell → Javascript Engine
You need to log in before you can comment on or make changes to this bug.