Closed
Bug 132609
Opened 22 years ago
Closed 22 years ago
js window.close() called from event handler does not do that
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla1.0
People
(Reporter: chado_moz, Assigned: jst)
References
Details
(Keywords: embed, regression, topembed-, Whiteboard: [HAVE FIX] [ADT3 RTM],custrtm-)
Attachments
(3 files)
6.65 KB,
text/html
|
Details | |
Call nsIScriptContext::ScriptEvaluated() after event handler is fired if there's no JS on the stack.
1.21 KB,
patch
|
hjtoi-bugzilla
:
review+
|
Details | Diff | Splinter Review |
1.29 KB,
patch
|
hjtoi-bugzilla
:
review+
jband_mozilla
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
I'm not sure about Compornent. Though bug 121586 has been VERIFIED as WORKSFORME, there is still remaining the case to fail closing window. When I try to close the window, opened by js, from keypress event, window is not closed. Here is a testcase: http://www2.wbs.ne.jp/%7Echado/work/js_window_close/js_window_close_3.html Steps to reproduce: 1) Open the url above in Mozilla browser. For instructions to follow, open screen space on left-top. 2) Click "open test window" link. 300 x 300 sized new test-window will be opened by js. 3) Click "close test window" link on first window. Function including window.close() will be called, then test-window will be closed. (this is the part of fixed in bug 121586) 4) Repeat step 2. 5) Click "enable keypress" link on first window. EventListner will start to get keypress event. 6) Press "x" key while test-window has focus. Event handler will call the function including window.close(), used in step 3. Expected result: test-window should be closed. Actual result: test-window is still there. Additional info: In step 6, pressing "w" key, instead of "x" key, will excute window.close() enclosed with setTimeout. Whether delayed time is not zero or be, test-window will now be closed. So, current workaround for me is to enclose the window.close() with setTimeout. (strange, is there any serialization problem or some...?) Same on click event. NC4x works as expected. Builds I have tested: works: Win98 milestones 0.9, 0.9.1, and Mac9 N6.01. do not works: Win98 milestones 0.9.2, 0.9.3, 0.9.4, 0.9.5, Mac9 milestones 0.9.6, 0.9.7, 0.9.8, 0.9.9, Mac9 N6.1J, N6.2.1J, Win98 2002-03-21-08-trunk and Mac9 2002-03-21-08-trunk. seems to be regression from far away.
Comment 1•22 years ago
|
||
Assignee | ||
Comment 3•22 years ago
|
||
The problem here is that when we're closing a window through window.close() we don't really close the window until the script that executed windnow.close() is done executing and in the case where window.close() called from an event handler that was added with addEventListener() we don't end up telling the script context that a "script" is done executing once the event handler is called. This patch fixes this problem by making the event listener manager tell the script context that a script is done executing if there's no JS on the stack when the event handler is called.
Assignee | ||
Comment 4•22 years ago
|
||
Heikki, Jband, would you guys r/sr?
Severity: minor → normal
Status: NEW → ASSIGNED
Whiteboard: [HAVE FIX]
Target Milestone: --- → mozilla1.0
*** Bug 107321 has been marked as a duplicate of this bug. ***
Comment on attachment 76650 [details] [diff] [review] Call nsIScriptContext::ScriptEvaluated() after event handler is fired if there's no JS on the stack. r=heikki, but I would prefer if you inverted the logic regarding the parameter name, i.e. use |js_running|, and later you can test |if (!js_running)|
Attachment #76650 -
Flags: review+
Assignee | ||
Comment 7•22 years ago
|
||
Comment on attachment 76674 [details] [diff] [review] Same as above, only different (per heikki's request) r=heikki
Attachment #76674 -
Flags: review+
Comment 9•22 years ago
|
||
Comment on attachment 76674 [details] [diff] [review] Same as above, only different (per heikki's request) sr=jband
Attachment #76674 -
Flags: superreview+
Updated•22 years ago
|
Keywords: regression
Updated•22 years ago
|
Comment 10•22 years ago
|
||
Comment on attachment 76674 [details] [diff] [review] Same as above, only different (per heikki's request) a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #76674 -
Flags: approval+
Comment 11•22 years ago
|
||
adt1.0.0-. Let's take a look at this for MachV RTM.
Comment 12•22 years ago
|
||
topembed- not an embedding blocker, but it would be a Good Thing to have so embed
Comment 13•22 years ago
|
||
Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt. If you have any questions about this, please email adt@netscape.com. You can search for "changing adt3 bugs" to quickly find and delete these bug mails.
Keywords: nsbeta1-
Comment 14•22 years ago
|
||
Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt. If you have any questions about this, please email adt@netscape.com. You can search for "changing adt3 bugs" to quickly find and delete these bug mails.
Keywords: nsbeta1+
Updated•22 years ago
|
Updated•22 years ago
|
Comment 15•22 years ago
|
||
Comment on attachment 76674 [details] [diff] [review] Same as above, only different (per heikki's request) a=chofmann,brendan please check in to 1.0 branch asap to get this in rc3.
Assignee | ||
Comment 16•22 years ago
|
||
Fixed on branch too.
Reporter | ||
Comment 17•22 years ago
|
||
I don't know why this has been marked as FIXED, but trunk builds still have this problem. e.g. mac9 2002-05-20-03-trunk
Argh, this wasn't landed on the trunk! Reopening.
Really this time...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 20•22 years ago
|
||
"Oops" on my part for that, fixed on the trunk now.
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Comment 23•22 years ago
|
||
Posthumus adt1.0.0+, pls get ADT approval prior to checking in to the 1.0 branch.
You need to log in
before you can comment on or make changes to this bug.
Description
•