Closed Bug 101190 Opened 18 years ago Closed 15 years ago

window.open() in onclick, click anchor with new window target, etc. while page is loading fail if popups are blocked

Categories

(SeaMonkey :: General, defect, major)

defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.7final

People

(Reporter: jruderman, Unassigned, NeedInfo)

References

(Blocks 1 open bug)

Details

(Keywords: conversion, fixed-aviary1.0)

Attachments

(4 files)

Steps to reproduce:
1. Put the linked images bookmarklet from 
http://www.squarefree.com/bookmarklets/pagelinks.html on your personal toolbar.
2. Start loading http://komodo.mozilla.org/buster/mkpg.cgi?lines=80000.
3. Activate the bookmarklet before the page finishes loading.

Result: "window.open() has no properties" in the javascript console.  The menu 
item Tasks>Tools>JS Console no longer works in this browser window (???).

Expected: new window opens listing the 0 images linked to by the page.

(Mitch mentioned that something similar occurs if you hit the Stop button and 
then trigger a window.open by clicking in a page, but I couldn't find a bug on 
that.)
so wait until the page loads before using your bookmarklet...

Actually I'd be OK with this, I'm just not sure how to distinguish bookmarklets
from JS on the page internally without adding some new piece of state, to the
principal maybe, and that would be a lot of work and added complexity.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Activating a bookmarklet stops the page from loading, so maybe you can just 
keep track of whether the page is still loading.  If that's possible, that 
would take care of the stop button problem as well.
I'll see if that's possible. Do you know of a way? 
This bug also affects javascript: links within the page (if the links use 
window.open).  For example, clicking a link to a room at 
http://www.playsite.com/games/list.gsp?root=playsite.word.tangle
when the page isn't fully loaded will fail.  The site notices that the open 
failed, and give the inaccurate error message "Not enough memory available to 
open game window.  Close some other applications and try again."
Related to Bug 101189
Summary: can't use bookmarklet while page is loading (disable_open_during_load) → javascript: link while page is loading (disable_open_during_load)
*** Bug 103597 has been marked as a duplicate of this bug. ***
Less important bugs retargeted to 0.9.9
Target Milestone: Future → mozilla0.9.9
Target Milestone: mozilla0.9.9 → Future
*** Bug 143979 has been marked as a duplicate of this bug. ***
*** Bug 144633 has been marked as a duplicate of this bug. ***
*** Bug 147431 has been marked as a duplicate of this bug. ***
Summary: javascript: link while page is loading (disable_open_during_load) → window.open() in onclick, etc. fails while page is loading (disable_open_during_load)
*** Bug 160262 has been marked as a duplicate of this bug. ***
Summary: window.open() in onclick, etc. fails while page is loading (disable_open_during_load) → window.open() in onclick, etc. fails while page is loading (if |dom.disable_open_during_load| pref is set to block popup ads)
Blocks: popups
I just visited a slow-loading site... one picture never loaded, in fact. I
couldn't load the main content which was in a window.open link until Mozilla
finally timed out on loading the one last image.

Would an easy fix here be to, for dom.disable_open_during_load's purposes,
consider the page loaded once the user clicks a link?

If there really was an unrequested link somewhere, they might still get it, but
blocking a REQUESTED window is much worse. 'Specially since 'stop' doesn't
really work, currently.
*** Bug 186743 has been marked as a duplicate of this bug. ***
*** Bug 186743 has been marked as a duplicate of this bug. ***
*** Bug 187547 has been marked as a duplicate of this bug. ***
OS -> All (just an excuse for adding myself to cc)
OS: Windows 98 → All
*** Bug 198319 has been marked as a duplicate of this bug. ***
I am setting this to major since this is a total failure of the ability to
follow a (javascript) link. Maybe even critical? There is no way to follow the
links but newly loading the page (which might be another form submission).
Looking at the number of dupes this is often encountered.

I don't see it mentioned here, but it is somehow related to bug 144587 which
shows the side effect that not only there is a failure with the link, but the
original report fails to load completely (like bug 144587 comment 8).

pi
Severity: normal → major
Hardware: PC → All
*** Bug 205400 has been marked as a duplicate of this bug. ***
*** Bug 207778 has been marked as a duplicate of this bug. ***
Over to XP Apps as this is a popup blocking bug
Assignee: mstoltz → shliang
Status: ASSIGNED → NEW
Component: Security: General → XP Apps
*** Bug 210121 has been marked as a duplicate of this bug. ***
*** Bug 206591 has been marked as a duplicate of this bug. ***
*** Bug 227411 has been marked as a duplicate of this bug. ***
Assignee: shliang → rginda
Here's a quick hack that should fix the problem:

In GlobalWindowImpl::CheckForAbusePoint, if !mIsDocumentLoaded, check to see if
mLastMouseButtonAction < 1 second ago, if so, allow the popup.

I'll try to squeeze out a patch.
Status: NEW → ASSIGNED
"Here's a quick hack that should fix the problem:"

I'm not familiar with the code, but the proposed patch sounds like it has a
serious problem.  It may well be less than a second since the user clicked to
start loading the document when a unwanted popup script starts.  Conversely, a
user might click somewhere before the page finishes loading, not intending to
activate something, but before an unwanted popup script starts.

As much as I want this false negative problem fixed so that desired popups pop,
DON'T do this in a manner that causes false positives and unwanted popups to pop.
"I'm not familiar with the code"

Then pipe down, I've got it under control.  If it doesn't work right, then it
doesn't go in.
It's fair to question whether we want such a hack, and a beauteous hack it is.
Thing is, any mouseclick will reset the timer. So if the user starts, say,
scrolling around in the big boring page as it slowly collects itself, or even
beats on the menus in a pique of boredom, it'll fool the popup blocker if the
timing works out just right. We don't want to offend our users, even the bored
ones. No sir, I don't like it!

(PS Summary adjusted in an attempt to catch more duplicates, like 227411).
Summary: window.open() in onclick, etc. fails while page is loading (if |dom.disable_open_during_load| pref is set to block popup ads) → window.open() in onclick, click anchor with new window target, etc. while page is loading fail if popups are blocked
oops - the menubar is obviously a different window and won't reset the timer.
The scrollbar remains a source of possible misreadings.
> oops - the menubar is obviously a different window and won't reset the timer.
> The scrollbar remains a source of possible misreadings.

But only if you click the scrollbar right about the time the page is trying to
load an unrequested popup.  When you click the mouse button during load, you'll
open a window (of time) for unrequested popups to sneak in.  The popup request
would have to come in during that timeframe, any request that comes in before or
after will still be blocked.  The window could be reduced to a fraction of a
second, because what we're measuring is the time between the mouse click and the
window.open call, which will be *extremely* small even on slow systems.

Like I said earlier though, if this doesn't work out, we don't have to check
anything in.  I'll bet that this hack, which shouldn't be more than two lines
(not counting the wacky LL_ math), will take care of the most common causes, and
wont introduce any significant source of "leaked" unrequested popups.  Maybe I'm
wrong, but I'm not going to be satisfied until I try it for myself.
*** Bug 228360 has been marked as a duplicate of this bug. ***
*** Bug 215691 has been marked as a duplicate of this bug. ***
*** Bug 206218 has been marked as a duplicate of this bug. ***
*** Bug 222948 has been marked as a duplicate of this bug. ***
*** Bug 196757 has been marked as a duplicate of this bug. ***
*** Bug 215100 has been marked as a duplicate of this bug. ***
*** Bug 217187 has been marked as a duplicate of this bug. ***
*** Bug 229510 has been marked as a duplicate of this bug. ***
*** Bug 229567 has been marked as a duplicate of this bug. ***
*** Bug 230163 has been marked as a duplicate of this bug. ***
*** Bug 232209 has been marked as a duplicate of this bug. ***
*** Bug 232209 has been marked as a duplicate of this bug. ***
The right way to fix this, imo is as follows:

1)  Split up mIsDocumentLoaded into two booleans:  mIsLoading and mInUnload
2)  Change the check that does:

    3041   // is the document being loaded or unloaded?
    3042   if (abuse == openAllowed && !mIsDocumentLoaded)
    3043     abuse = openAbused;

    To:
 
    3042   if (abuse == openAllowed && mInUnload)

3)  In the else clause of the |if (currentEvent)| add:

           if (abuse == openAllowed && !mIsLoading)
             abuse = openAbused;
Er, switch the parity on that last boolean.  ;)
Attached patch Totally untestedSplinter Review
Things that would need testing:

1)  Whether it fixes this bug
2)  Whether it keeps blocking popups all through unload (including timers set
in
    unload).
3)  Whether it correctly handles the mIsDocumentLoading state (especially for
    about:blank)
4)  Anything else people can think of.
with the patch applied, the popup from this link is still blocked during load.

<a href="http://www.mozilla.org/" target="_blank">click</a><br>

window.open() calls seem to work (fixed by the patch)
Hmm. Yeah.  The target="_blank" case is calling window.open() internally with no
event on the stack (since link clicks are processed asynchronously).  The right
thing there is to just add another arg to the internal open() it calls that will
be true if and only if the window.open is coming from trusted code (and hence
should not be subject to popup blocking).

Have to be a little careful, though -- <form target="_blank"> can be triggered
automatically via a submit() call.  Perhaps the right thing to do is to check in
the patch in bug 41907 and then propagate that new boolean down all the way to
here (after making <form> use it) so it can be used in deciding whether this is
a popup....
Though.... With this patch, if an onload handler dispatches a click event to an
anchor, what happens?
Attached file Like so.
In fact, this page creates a popup in a build _without_ my patch... I wonder
why.
*** Bug 172587 has been marked as a duplicate of this bug. ***
*** Bug 222466 has been marked as a duplicate of this bug. ***
*** Bug 238196 has been marked as a duplicate of this bug. ***
*** Bug 240742 has been marked as a duplicate of this bug. ***
*** Bug 241548 has been marked as a duplicate of this bug. ***
This also breaks the plugin installer for pages that are still loading.  See bug
241548 for details and a test case.
Also note that the solution suggested in comment 25 (checking the time since the
last mouseclick in the window's content) won't help bug 241548. This is not only
because the mouseclick in the embedded object's area does not bubble up to the
content window, but also because the intervening "do you want to look for
plugins?" dialog can inject an arbitrary time delay.
*** Bug 243426 has been marked as a duplicate of this bug. ***
*** Bug 244198 has been marked as a duplicate of this bug. ***
Can this be fixed in pieces or does something more major need to be done?  It
would be nice to have the links that can work with current patch do so instead
of waiting for all possible window opening paths to be tracked down and fixed,
if possible.
*** Bug 245858 has been marked as a duplicate of this bug. ***
*** Bug 246148 has been marked as a duplicate of this bug. ***
*** Bug 246330 has been marked as a duplicate of this bug. ***
*** Bug 248405 has been marked as a duplicate of this bug. ***
Keywords: conversion
jane, get me off this crazy thing.
Assignee: rginda → general
Status: ASSIGNED → NEW
Component: XP Apps → Browser-General
QA Contact: bsharma → general
Flags: blocking-aviary1.0?
Still have this bug on Windows XP and using Mozilla 2004062908.
Flags: blocking1.8a2?
Flags: blocking1.7.1?
Blocks: majorbugs
Flags: blocking1.8a2? → blocking1.8a2-
*** Bug 250133 has been marked as a duplicate of this bug. ***
*** Bug 245780 has been marked as a duplicate of this bug. ***
Just wanted to say one thing about the conversion keyword. 
An MSIE 6 user with a popup blocking add-on will stop window.open() links from
opening requested popups as well if the page is still in the process of being
loaded (in the opener/parent window). So, this behavior (actual results of this
bug or bug 232209) is NOT specific to Mozilla or Firefox. If needed, I can
create a reduced testcase with explicit instructions and with easy+clear steps
to reproduce which will demonstrate all this.
I do not think that the inferior feature set of a third-party addon to IE should
influence the development of Gecko-based browsers. In fact, Mozilla's pop-up
blocker performs better than any other I have seen, and we should keep it that
way. Besides, this is a regression bug; it used to work just fine.
Oops, sorry about the rant. Just noticed you were talking specifically about the
"conversion" keyword.
*** Bug 250836 has been marked as a duplicate of this bug. ***
*** Bug 252639 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.0? → blocking-aviary1.0+
*** Bug 246967 has been marked as a duplicate of this bug. ***
Is this a regression?
Should the regression keyword be added?
*** Bug 256200 has been marked as a duplicate of this bug. ***
(In reply to comment #68)
> Just wanted to say one thing about the conversion keyword. 
> An MSIE 6 user with a popup blocking add-on will stop window.open() links from
> opening requested popups as well if the page is still in the process of being
> loaded (in the opener/parent window). So, this behavior (actual results of this
> bug or bug 232209) is NOT specific to Mozilla or Firefox. If needed, I can
> create a reduced testcase with explicit instructions and with easy+clear steps
> to reproduce which will demonstrate all this.

Which popup-blocker addon have you tried? Google?

I tried the official popup blocker for IE in Windows XP SP2, with which a window
spawned by clicking a hyperlink while loading the original page is not blocked,
so this bug IS specific to Mozilla or Firefox now.

I have this bug on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3)
Gecko/20040905 Firefox/1.0 PR (NOT FINAL). Definitely one of annoying bugs to be
fixed before FF 1.0.
*** Bug 257899 has been marked as a duplicate of this bug. ***
From bug 257899: There's a testcase for this problem (blocker still active after
page load abort) at http://bitbrook.de/privat/st/popup.html , so you don't need
to play around with public sites hoping one will be slow enough.
This should be fixed on trunk now that bug 252326 is fixed.  Please retest.
Blocks: 180846
The testcase seems to work perfectly in Mozilla/5.0 (X11; U; Linux i686;
rv:1.7.3) Gecko/20040908 Firefox/0.10 on Linux.
Works for me on Windows too. Many thanks to Wladimir Palant and Johnny Stenback
for fixing this great annoyance!
Fixed by the checkin for Bug 252326
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: blocking1.7.x?
Keywords: fixed-aviary1.0
No longer blocks: 258457
*** Bug 258457 has been marked as a duplicate of this bug. ***
*** Bug 258717 has been marked as a duplicate of this bug. ***
The following page still seems to display this bug:
http://www.lonelyplanet.com/destinations/caribbean/cuba/

Using the following build:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a4) Gecko/20040908
Firefox/0.9.1+
Sorry, I left something out in the above post.

Click on the "view slideshow" link and/or image.
(In reply to comment #85)
> The following page still seems to display this bug:
> http://www.lonelyplanet.com/destinations/caribbean/cuba/
> 
> Using the following build:
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a4) Gecko/20040908
> Firefox/0.9.1+

That's a different issue that's been wontfixed - see Bug 239403
The bug summary also mentions pages that are blocked because they open a new
window (target="_blank" etc). This hasn't been fixed or so it seems.

If I try to click on a link like:

<a href="www.mozilla.org" target="_blank">Testlink</a>

this new page gets blocked which is rather annoying (understatement :D). For a
live example:

http://psi.affinix.com/forums/index.php?act=ST&f=5&t=1784

and click on the 'WWW' button in the first posting.

Chances are that my PC is being weird because I really can't believe that there
are not 500 duplicate bugs reporting this :\ Running Firefox 1.0PR1 on Linux
(Debian, KDE).
(In reply to comment #88)
> The bug summary also mentions pages that are blocked because they open a new
> window (target="_blank" etc). This hasn't been fixed or so it seems.

I can't reproduce with Mozilla 1.8a4. Try this testcase.
Weird :\
Can't reproduce anymore. The only thing that has changed is that I've switched
the popup blocker on and off (which required a restart of Firefox :\). Extremely
bizar.

Nevermind, sorry to bother you all :) I'll be back when I can properly reproduce
this...
Product: Browser → Seamonkey
*** Bug 247464 has been marked as a duplicate of this bug. ***
*** Bug 256809 has been marked as a duplicate of this bug. ***
No longer blocks: majorbugs
Status: RESOLVED → VERIFIED
Target Milestone: Future → mozilla1.7final
his bug also affects javascript: links within the page (if the links use 
window.open).  For example, clicking a link to a room at :http://hanoifreetoursbyfoot.com/
Flags: needinfo?(freewalkingtourshanoi)
You need to log in before you can comment on or make changes to this bug.