If you click the big ad in the middle of http://www.totalsexmovies.com/nkamat/nkamat02tfu.html, you'll get to a page with a yellow background (http://www.karasamateurs.com/guests/index.shtml). This page is blank if you have set the dom.disable_open_during_load pref. While the page has a pop-up ad, it's not the failure of the pop-up to appear that makes the page show up blank. The problem is that the site relies on one of the frames to use window.open("realsite.html","main frame"), and Mozilla doesn't check whether the window.open actually opens a new window before blocking it. Mozilla doesn't block other methods of replacing frames, so it shouldn't block this one. (Side note: I never imagined I'd help someone who makes pop-up ads...)

I made this bug "major" because it hurts the oft-posted (and oft-moderated-up) argument "users should be able to choose which ads they interact with, so features like dom.disable_open_during_load should be easy to find and/or enabled by default." I don't believe that breaking the href of this banner ad was intentional, but it needs to be fixed before the user-choice crowd can fully accept this feature. I'm sure the "all ads are evil" crowd loves this bug, but they're just bitter about the state of the banner ad market.

Confirmed. Unfortunately, there's no easy way to distinguish a window.open targeted at an existing frame from one targeted at a new window. It could be done, but that would be a pretty big code change. I'm eventually going to make the disable-open-during-load mechanism be site-specific, so you can just exclude some sites from the blocking. I'm inclined to mark this WONTFIX, but for now I'll mark it Future until we see what becomes of the event-based JS filters.

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

Since 127209 was a dup I will add here that it is *not only popup ads* that suffer from this bug but *proper sites* that want to differentate their main site from a UI to their service (eg the bank www.smile.co.uk -- click on 'service account')

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

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

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

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

Thanks to bzbarsky for marking Bug 145372 as a duplicate of this and adding me to the cc list. Bug 145372 contains a testcase that is the inverse case of a pop-up ad. It models a site whose main window is created intentionally by the user via a click on a link. The dilemma is that with this brain-dead pop-up killer preference, the user cannot return to this window from a SSL transaction in another window (again opened intentionally). There is no way to do this after this window.open() API glitch with any other technique. Obviously one cannot cross-reference two windows in different domains due to the JavaScript cross-domain security restrictions. I would personally opt to either remove the whole pop-up killer preference entirely or to do it properly. I do not like automatic pop-up ads at all. But it's a problem with the sites that use this technique and not with any browser. If Mozilla/Netscape implements this feature in the current sub-standard fashion then they just add to the mess. Mozilla should be a correct browser not a geek tool. If a user sees this feature then one cannot blame him to switch it on. But one can easily blame Mozilla for not working correctly on existing correct sites and that IMHO should not happen.

I'm sure if we took a vote, the overwhelming majority would vote for keeping the feature in at this point :) BHT, if it offends you so much that we occasionally block legitimate sites, then help us come up with a smarter blocking algorithm.

For me this is the reason not to use the blocking feature. I would really like to. The point is to block only pop-up windows, not calls to already open ones. pi

I'm still trying to figure out what the problem is. Maybe it is really complex to find windows in Mozilla. You may have to ask someone at Netscape who knows that part of the code. I am adding jst to the cc list; maybe he can help. As a suggestion, I would go through the collection of all open windows and compare the JavaScript name attribute of each of them with the requested window name. If a window with a matching name is found I would not block the "pop-up" request for that window.

A simpler testcase: Don't allow to "open unrequested windows". Go to .univie.ac.at/~3.14/mozilla-bugs-frames/frameset2.html. Now allow to "open unrequested windows". Again visit that page. If both frames have the same color, the script was blocked. pi

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

The testcase from comment 14 should read as follows, sorry: http://www.logic.univie.ac.at/~3.14/mozilla-bugs-frames/frameset2.html pi

Not sure, this is the right bug for it. If you don't allow "open a link in a new window", the JavaScript menu on the following page does not work: http://www.d-a-g.de/ (click on Aktuell to see it). pi

Pi: This bug is about "Open unrequested windows"; the "Open a link in a new window" checkbox is completely unrelated to this bug.

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

Another testcase site: http://www.fh-konstanz.de/Studium/fachb/in/wwwin/ti.htm (from bug 158469)

jst, can you come up with a patch that allows existing frames and windows to be targeted? /be

->me

Better fix. Removes a little extra cruft.

Comment on attachment 92337 [details] [diff] [review] Patch v.1.1 Nit of the day: + wwatch->GetWindowByName(name.get(), + this, + getter_AddRefs(namedWindow)); Since there's room for |this| on the first line here, pull it up :-) sr=jst

Comment on attachment 92337 [details] [diff] [review] Patch v.1.1 What if argc == 1, but a bad page is being abusive? Don't we want to CheckForAbusePoint() in that case too? /be

Comment on attachment 92337 [details] [diff] [review] Patch v.1.1 Oh, duh, yes of course...

Yeah, good catch. Silly me. New patch on the way...

Better fix. I definitely don't want to be known as the guy who broke pop-up blocking :-)

Comment on attachment 92344 [details] [diff] [review] Patch v.1.2 >+ if (CheckForAbusePoint()) { >+ if (!name.IsEmpty()) { Change this to an |if (name.IsEmpty())| that'll remove one level of nesting. >+ nsCOMPtr<nsIWindowWatcher> wwatch(do_GetService(sWindowWatcherContractID, &rv)); >+ if (NS_SUCCEEDED(rv)) { Change this to NS_ENSURE_SUCCESS(rv, rv), that's neater and will remove another level. With that, r=sicking

Of course. This seems saner. Thanks, sicking.

Comment on attachment 92345 [details] [diff] [review] Patch v.1.3 Much better :-) sr=jst

Comment on attachment 92345 [details] [diff] [review] Patch v.1.3 adding r=sicking

Comment on attachment 92345 [details] [diff] [review] Patch v.1.3 a=asa (on behalf of drivers) for checkin to 1.1

Checked in. It appears drivers want this for 1.0.1, so nominating for nsbeta1

I wonder whether Netscape should worry about this patch, given that they disable popup blocking in their Mozilla derived product. I think we should get this into 1.0.1 ASAP, and not wait for ADT approval, if Netscape engineers agree with me on the technical content here. The code is not going to regress any build in which CheckForAbusePoint returns false. /be

Brendan, I agree with all your points. I'm new to the game on this side of the fence and was just trying to play by the rules. I'll go ahead request branch approval.

Brendan, you've got my agreement on all technical issues here.

Let's get this into the branch, too. Please change mozilla1.0.1+ to fixed1.0.1 when it's in. Thanks, /be

Landed on branch, too. Thanks, Brendan!

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

bsharma - can you verify this bug fix in 1.01 branch? When verified, pls replace fixed1.0.1 keyword with verified1.0.1. Thanks.

Verified on 2002-08-24-branch build on Win 2000. Loaded the test case in comment #20. When clicked on the links the frames on the right are loaded fine.

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