Closed
Bug 260385
Opened 20 years ago
Closed 20 years ago
beforeunload event can be used to open a popup window
Categories
(SeaMonkey :: UI Design, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: danm.moz, Assigned: jst)
References
Details
(Keywords: fixed-aviary1.0, fixed1.7.5)
Attachments
(4 files)
175 bytes,
text/html
|
Details | |
7.27 KB,
patch
|
danm.moz
:
review+
bzbarsky
:
superreview+
asa
:
approval-aviary+
asa
:
approval1.7.5+
asa
:
approval1.8a4+
|
Details | Diff | Splinter Review |
7.08 KB,
patch
|
Details | Diff | Splinter Review | |
8.84 KB,
patch
|
Details | Diff | Splinter Review |
Reload a page containing a beforeunload handler that opens a window, you get a new window. Note this is not the same as bug 259117.
Attachment #159412 -
Attachment mime type: text/plain → text/html
Ugh. More notes. Firefox Aviary branch build dated 20040918.
Hardware: PC → All
*** Bug 260581 has been marked as a duplicate of this bug. ***
Comment 5•20 years ago
|
||
Confirmed. This only works when clicking the refresh button, not with Ctrl-R. This seems to have the same reasons as bug 259117.
Assignee | ||
Comment 6•20 years ago
|
||
This blocks popups from beforeunload events. And it undoes a change done for the fix for the same problem for unload events by moving that code from the global window code into the document viewer code where the unload event is fired.
Assignee | ||
Updated•20 years ago
|
Attachment #159627 -
Flags: superreview?(bzbarsky)
Attachment #159627 -
Flags: review?(danm.moz)
Comment 7•20 years ago
|
||
Comment on attachment 159627 [details] [diff] [review] Don't permit onloads from onbeforeunload event handlers Yeah, I like this approach!
Attachment #159627 -
Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 159627 [details] [diff] [review] Don't permit onloads from onbeforeunload event handlers Does this not allow you to also back out bug 259117's changes to nsDocShell? I found them kind of worrisome: @@ -3129,11 +3129,11 @@ nsDocShell::Create() NS_IMETHODIMP nsDocShell::Destroy() { + mIsBeingDestroyed = PR_TRUE; + //Fire unload event before we blow anything away. (void) FireUnloadNotification(); - mIsBeingDestroyed = PR_TRUE; - // Stop any URLs that are currently being loaded... Stop(nsIWebNavigation::STOP_ALL); if (mDocLoader) { @@ -3771,10 +3771,6 @@ nsDocShell::ScrollByPages(PRInt32 numPag nsIScriptGlobalObject* nsDocShell::GetScriptGlobalObject() { - if (mIsBeingDestroyed) { - return nsnull; - } - NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nsnull); return mScriptGlobal;
Attachment #159627 -
Flags: review?(danm.moz) → review+
Assignee | ||
Comment 9•20 years ago
|
||
Yes, I could actually do that, but I'd rather not do that on the trunk as it's IMO the right thing to do. I'd be happy to back that out on the branch though, just in case. Sound ok?
Reporter | ||
Comment 10•20 years ago
|
||
I was especially concerned about the @@ -3771,10 +3771,6 change, which at first glance looks like it could lead to referenced deleted object crashes. I'm happy with what you think is best.
Assignee | ||
Updated•20 years ago
|
Attachment #159627 -
Flags: approval1.8a4?
Attachment #159627 -
Flags: approval1.7.x?
Attachment #159627 -
Flags: approval-aviary?
Comment 11•20 years ago
|
||
Comment on attachment 159627 [details] [diff] [review] Don't permit onloads from onbeforeunload event handlers a=asa for 1.8a4 checkin.
Attachment #159627 -
Flags: approval1.8a4? → approval1.8a4+
Assignee | ||
Comment 12•20 years ago
|
||
Fix checked in on the trunk.
Assignee | ||
Comment 13•20 years ago
|
||
Comment 14•20 years ago
|
||
Comment on attachment 159627 [details] [diff] [review] Don't permit onloads from onbeforeunload event handlers a=asa for branches checkin.
Attachment #159627 -
Flags: approval1.7.x?
Attachment #159627 -
Flags: approval1.7.x+
Attachment #159627 -
Flags: approval-aviary?
Attachment #159627 -
Flags: approval-aviary+
Assignee | ||
Comment 15•20 years ago
|
||
Assignee | ||
Comment 16•20 years ago
|
||
Fixed on trunk and aviary branch. Need to land other changes on the 1.7 branch before this one can go in there.
Assignee | ||
Updated•20 years ago
|
Keywords: fixed1.7.x
Assignee | ||
Comment 17•20 years ago
|
||
Fixed on 1.7 branch
Comment 18•20 years ago
|
||
*** Bug 263271 has been marked as a duplicate of this bug. ***
Comment 19•20 years ago
|
||
*** Bug 265886 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: Core → Mozilla Application Suite
You need to log in
before you can comment on or make changes to this bug.
Description
•