102914 - mailnews and address book on component bar temporaraly disabled after closing a tab when dom.disable_open_during_load is set

Status: VERIFIED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[James Paige]

On Windows 95, build 2001100203 and 2001100303

Open a tab (or two or three, it doesnt matter how many)

Close a tab (any tab, it doesnt matter if it was the original tab, or the one
you just opened)
Now try clicking the buttons in the component bar for opening a new mailnews
window or address-book window. No response

The buttons for new browser window and new composer window are fine

The mailnews and address-book buttons will be re-enabled if you load a new page
or reload the current page in any open tab.

Comment 1

[John Morrison]

worksforme win2k 2001100308 trunk.

Comment 2

[James Paige]

hmmm... still seeing it on build 2001100403

Comment 3

[James Paige]

Hmmm. I tried this ona different computer, Windows 98 build 2001100303 and I
couldnt reproduce it, but it is still happening on my Win 95 machine. Can
anybody else with Win 95 try this out?

Comment 4

[Eric Gerlach]

I'm getting this on Win98 with the 0.9.5 release build.

Comment 5

[Peter Trudelle]

worksforme

Comment 6

[James Paige]

I can still reproduce this with build 2001102903 on Windows 95.

However, I will not re-open, because I now realize that I cannot reproduce this
on a new clean profile, only on my existing profile. So, this must be some sort
of stale profile data issue. I am still trying to figure out what i can
delete/edit in my profile to make it go away.

Comment 7

[James Paige]

Aha! I figured out what in my profile was causing this. It wasnt mysterious
stale data after-all. It was the pref

  user_pref("dom.disable_open_during_load", true);

in my prefs.js file. When I added that line to the prefs.js of a brand new
profile, I was able to reproduce this bug.

That pref is to prevent window.open from inside an onload handler. The only
place it is read is at
http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#2309

That code was put in by mstoltz on Aug 30 21:06 to fix bug 92955

Comment 8

[Niklas Mehner]

Marking NEW. I can reproduce this on Linux build 2001112308 and there is a
duplicate.

Comment 9

[Niklas Mehner]

*** Bug 111882 has been marked as a duplicate of this bug. ***

Comment 10

[Niklas Mehner]

ok, what I think this is: When a tab is closed, a NS_PAGE_UNLOAD event, but no
NS_PAGE_LOAD event is fired. Therefor nsGlobalWindow.mIsDocumentLoaded is false,
and JavaScript is not allowed to open a window (because of the
dom.disable_open_during_load). This could probably be fixed, by firing a page
load event somewhere, BUT I think this approach is fundamentally flawed,
because, not the window, but the document the script belongs to has to decide,
if the JavaScript code is allowed to open a window.

With a current implementation, a page beeing loaded in one tab, can prevent a
script running for another tab (or not for any webpage at all) from opening a
window. This does not make sense. Currently it is not possible to start Mail,
while a page is beeing loaded (without working with tabs at all).

I might be wrong with this, but this is how it looks to me.

Comment 11

[Brendan Eich [:brendan]]

jst: can we make the logic in nsGlobalWindow.cpp for abuse-checking apply only
to content windows?  That should fix any chrome window opening problems, I hope.

/be

Comment 12

[Johnny Stenback (:jst)]

Attachment: Only ever disable window.open() from content.

Sounds reasonable to me, does this patch fix the problem?

Comment 13

[Niklas Mehner]

*** Bug 112599 has been marked as a duplicate of this bug. ***

Comment 14

[Martin Poirier]

*** Bug 114986 has been marked as a duplicate of this bug. ***

Comment 15

[WD]

*** Bug 116280 has been marked as a duplicate of this bug. ***

Comment 16

[Markus Gerstel]

Does this patch (assuming it's not a rotten one) also fix bug 101189 ?

Comment 17

[Boris Zbarsky [:bzbarsky]]

adding bugs that this patch should fix to dependency list.

Comment 18

[Geraint]

*** Bug 118357 has been marked as a duplicate of this bug. ***

Comment 19

[Jonas Jørgensen]

Changing priority to normal based on Geraint's comments in bug 118357.

Comment 20

[Brendan Eich [:brendan]]

Why is this on hyatt's list?  Reassigning to jst, who owns the dom, and who will
likely fix it quickly.

/be

Comment 21

[Johnny Stenback (:jst)]

Attachment: Slight modification of the above patch.

Comment 22

[Johnny Stenback (:jst)]

Mitch, would you review this please?

Comment 23

[Brendan Eich [:brendan]]

Comment on attachment 63840 [details] [diff] [review]
Slight modification of the above patch.

sr=[e-mail]

Comment 24

[Johnny Stenback (:jst)]

Comment on attachment 63840 [details] [diff] [review]
Slight modification of the above patch.

Rick says r=rpotts

Comment 25

[Johnny Stenback (:jst)]

Fixed.

Comment 26

[Jonas Jørgensen]

Verified on build 2002-01-08-03.

Comment 27

[Jonas Jørgensen]

*** Bug 117122 has been marked as a duplicate of this bug. ***