Closed
Bug 260264
Opened 20 years ago
Closed 15 years ago
Popups from a Site that is in the "Allowed List" (whitelist) are blocked, starting with the n-th popup (dom.popup_maximum)
Categories
(Core :: DOM: Events, defect, P2)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla1.9.2
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta2-fixed |
People
(Reporter: C.Voelker, Assigned: mozilla+ben)
References
()
Details
Attachments
(5 files, 5 obsolete files)
1.95 KB,
text/html
|
Details | |
1.29 KB,
application/x-zip-compressed
|
Details | |
1011 bytes,
application/zip
|
Details | |
25.42 KB,
patch
|
mozilla+ben
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
19.20 KB,
text/html
|
Details |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20040913 Firefox/0.10 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20040913 Firefox/0.10 I encounter this phenomenon on a Site where you have to be logged in, so it does not make sense to give a URL. With the first -lets say- twenty popups everthing works fine but after half an hour or so, the browser starts blocking popups and shows a related information bar. I cant tell exactly whether it is related to the time spent on the site or to the number of popups but the fact that popups are blocked after a while is reproducable. It might even have to do with the number of windows opened subsequently this way: Clicking opens a popup wherein I click another link that opens another popup and so on. You might think of that as poor interface design, and I would partly agree but I havent built the site and it is quite valuable for me anyway. Closing the tab or the whole window does not cure the problem. Closing all windows and opening the URL again does not do the job either, I have to restart Firefox to return to normal operation. When I go to "Edit Popup blocker Options…" I can see that the site is in the list already. Ah, obviously I have to mention that popup blocking is active ;-). This did not happen with version 0.9.3 that I downloaded on August, 3rd and other versions before. Reproducible: Always Steps to Reproduce: Clicking Popup Links extensively on a Site that is unblocked Actual Results: The browser stops opening user requested popups after a while Expected Results: the browser should open the popups
Reporter | ||
Comment 1•20 years ago
|
||
When clicking the menu entry "Show 'http://www.blockedsite.com/blockedlink.html' in the drop menu of the info bar at the top of the window, the popup is not shown either.
Reporter | ||
Updated•20 years ago
|
Version: unspecified → 1.0 Branch
Comment 2•20 years ago
|
||
I can reproduce it also... takes 20-25 clicks of opening and closing the popup within the unblocked site, then the site will become blocked until you restart the browser. Using latest Slackware Linux, Kernel 2.6.7, on x86 hardware. Never saw this in Firefox versions prior to 1.0 Preview Release.
I have also seen this bug. It even happens, if the option "block popup windows" is completely turned off.
Comment 4•20 years ago
|
||
According to the description something is wrong with dom.popup_maximum (this setting is respected even if the popup blocking is disabled). However I cannot reproduce this, neither in my Seamonkey debug build, nor in FF1.0PR. Tabbrowser Extensions (TBE) is known to interfere with the popup blocker in some way, maybe you have it installed? If so - does removing this extension solve the problem?
Reporter | ||
Comment 5•20 years ago
|
||
> Tabbrowser Extensions (TBE) is known to interfere with the popup blocker ...
Well, I deactivated several Extensions now and wil try to reproduce it again. I
could not do it so far. If I will find it to work fine from now on, I will
activate Extensions again step by step. In case I find a culprit I will update
it and try again. This whole process will certainly take a week or so, so dont
expect me to report soon.
I havent had such thing as a TBE installed. I thought that tabbed browsing was a
standard feature of Firefox not dependent of an extension. Currently, I run a
german language pack, the web developers toolbar and magpie as extensions.
Comment 6•20 years ago
|
||
Actually, if the site has to be in the whitelist for the popups to open, its not using a method that gets interpreted as "user-requested" otherwise the popups would always work. This could be because of redirects, or really weird javascript, or basically anything that isn't "if a user clicks on X, open popup." If someone can produce a testcase that replicates this problem, great, but I'm thinking this is INVALID. Dan, am I reading this right?
Might as well give Christian time to work through his extensions. Christian, if you discover that you can reproduce this problem without extensions, please provide a testcase. If you discover that an extension is responsible, let us know. But we'll be closing this bug and hopefully passing the info on to the extension writer. I anticipate closing this bug WFM. In my experience with FF 0.10 you are indeed restricted to 20 simultaneously open allowed popup windows, as expected. But close one and you can open another. Note you're generally not allowed to open the replacement popup immediately: it usually takes a few seconds. So if you're right against your 20 window limit you may notice some latency like that. Note you can encourage running into the limit more quickly (or more slowly) by adjusting the dom.popup_maximum pref.
Reporter | ||
Comment 8•20 years ago
|
||
Ok, this makes up for three more tasks on my side: - Try what happens exactly when I remove the Site from the "allowed" List. - I will ask the programmer of the Site to follow this bugs thread. - I will check the JavaScript Code as good as I can myself. I am not a programmer, but I have a working knowledge of so called "copy and paste programming" and reading code. The Site is heavily trying to push IE to extremes and I blamed them several times that they ignore Mac Users and others but it turned out wrong every time. It is made by a real good guy who knows what he does. Phew, never though, bugwriting could turn out to be work by itself ;-). Keep on coding! Yours, Christian
Comment 9•20 years ago
|
||
dom.popup_maximum proved very helpful... I upped this value from 20 to 10,000 and now I don't seem to have a problem... (of course, I'm not going to sit here and open the popup 10,000 times, so....) Prior to that, I could open and close the popup only 20 times before it would refuse to allow another to be opened. (no change with delay after closing last one) (I also had webdev extension installed, but uninstalling it did not change anything -- it was the only extension I had installed) Apparently the problem is with the counter not going back down when the popup is closed. But... it probably *is* related to some of my funky javascript (or the complexity of the popup), because I wasn't able to reproduce it with a very simple "click here and open a popup" example. However, I'd still call it a bug because IMHO, the popup control *should* work the same regardless of how the popup was opened. One more note -- I *wasn't* able to reproduce the problem on Windows at all - so maybe whatever there is checking to see if the popup has closed must be working correctly? -- at least for the testing I was doing. If the person (or people) responsible for confirming or closing this bug would like a test account in my system where the problem may be seen, or if you want to see the code which opens the popups in question, just let me know.
Comment 10•20 years ago
|
||
> However, I'd still call it a bug
> because IMHO, the popup control *should* work the same regardless of how the
> popup was opened.
This kind of misses the point on how popup blocking works. If the user clicks
on something with an onclick function that opens the window, that's definitely a
user-initated action. If the user does a mouseover, etc, that's trying to load
a popup via a sneaky way, so we block it. The 20 popups max is to ensure that
even if you whitelist a site, it can't crash your app/PC by killing you with a
massive number of popups.
A reduced testcase (only the code needed to reproduce the bug) would be good in
determining what's going on in your case.
Comment 11•20 years ago
|
||
Ok, I got a test case together that works to reveal the problem.. It also tells what it is about the problem that makes it appear or not... Apparently, if the popped-up window has a bunch of onclick's in it, then you have a problem. If it doesn't you're OK. Also, problem does not show up ever in windows. (xp pro) Test and a little more info is at: http://www.mutualgravity.com/testpopup.php
Reporter | ||
Comment 12•20 years ago
|
||
Hello, here are some interim results. in effect, they dont tell a thing. Probably I made the old mistake and changed too many things at once so I am confused about what relates to what. Currently I cant see bockup blocking when I am surfing the Site mentioned first. I have removed this Site from the allowed Sites list. I asked the Sites programmer to follow this bugs thread (no answer yet). I tried to reactivate my Extensions step by step but only one of them could be reactivated without updating. When updating I could not test which of them caused the problem. Most of them turned out to be of limited use so I dont want to activate them again. After activating the one Extension, no popup blocking appeared again. Hey, this great actually! The javaScript Code of the Site cant be understood cmpletely within five Minutes and yes, it contains onclick mehtods. The mutualgravity test does not work for me in my current config. I tried to implement this dom.popup_maximum stuff to ease testing, but I am not sure about how to use this. I created a file named user.js containing a single line 'user_pref("dom.popup_maximum", 5);' and a newline. Do I have to put this into ~/Library/Application\ Support/Firefox/Profiles/default.44h/ or ~/Library/Firefox/Profiles/default/famx1ns5.slt/ ? The thing most likely solution as of now is that one of the Extensions is broken. I dont know whether I will spend more time which of them. Christian
Reporter | ||
Comment 13•20 years ago
|
||
Hello, I have just learned about this cool about:config stuff (thanks Bill Sechrist) and configured the dom.maximum_popup to four. Well, everything starts to get weird. Now, I can open neither popup from mutualgravities sample page any more. It does not matter whether I open the page in a tab or in separate window. Putting it into the Allowed Sites list does not help either. Obviously, some very basic things are screwed up now. Well, it will take more time to test properly so that anybody can make sense of these results. Is there a safe way to return to a defined setup? My whole environment remains the same when I install a new version and I think of this to be rather comfortable. But now, I am not sure where Firefox stores all his extensions, preferences and so on. I would like to keep my Bookmarks (well I know how to do this), stored passwords and form fields and so on but remove Extensions to make sense of my testing. I havent found the extensions searching for extension_name.jar and I havent found parts of Firefox in the /Library folder. Everything seems to reside in the application bundle (extensions can hardly be there because they would be missing after updating) and in the ~/Library folder. Am I right? Still then, I cant find the extensions. I dont trust the Extension Manager because he shows texts like "will be disabled after restart" still after several restarts and he also mixes up the text for disabling with the disabled state and vice versa. The Programmer of my community Site is following this thread now. He has send me a snippet of code that he has found to be relevant. He says that it is "plain simple" JavaScript. I will put it here if he agrees with me to do so. It is formatted quite readable and contains only one german word "Fenster" which means Window.
Reporter | ||
Comment 14•20 years ago
|
||
function OpenHPWindow(URL){ iLeft = window.screen.availWidth-580; iHeight = window.screen.availHeight-200; iTop = 0; Fenster = "HP"; WinURL = new String(); WinURL = URL; WinURL = WinURL.toLowerCase() if (WinURL.indexOf("hp.asp") >= 0 || WinURL.indexOf("gu.asp") >= 0 || WinURL.indexOf("ug.asp") >= 0) { Fenster = "HP"; if (document.location.href.toLowerCase().indexOf("gb.asp") >= 0) Fenster = "HP2"; } if (WinURL.indexOf("msg") >= 0) { myDate = new Date() myWin = myDate.getMilliseconds()+"_"+myDate.getMinutes()+"_"+myDate.getHours()+"_"+myDate.getDay()+"_"+myDate.getMonth() Fenster = "MSG" + myWin; iLeft = 0; iTop = 0; } if (WinURL.indexOf("gb") >= 0) { Fenster = "GB"; iLeft -= 60; iTop += 60; } if (WinURL.indexOf("history") >= 0) { Fenster = "HIST"; iLeft -= 90; iTop += 90; } HP = window.open(URL,Fenster,'width=570,height='+iHeight+',screenX='+iLeft+',screenY=0,left='+iLeft+',top='+iTop+',scrollbars=yes,status=yes,resizable=yes,menubar=no,toolbar=no'); HP.focus(); }
Comment 15•20 years ago
|
||
comment 11: Myself I have access only to a Windows box so I can't verify your testcase. So the mutualgravity.com/testpopup.php testcase works as described exactly as given? I mean to ask, you've duplicated your findings using the testcase as posted, and the php script isn't handing back anything different to a Mac browser? That's a head-scratcher. By the way we do like to have testcases local to Bugzilla. For one thing it answers my question about what the server is up to. comment 14: I'm not sure what to do with all that. It's probably the context of the function call that's more important than the function itself. I would caution to be careful that Fenster always contains a unique value if you always want a new window. comment 13: Use a new profile to return to a defined setup. Search on http://forums.mozillazine.org/ or ask around for specific instructions.
Comment 16•20 years ago
|
||
I made the links pop up copies of itself at my site. See doc for details.
Comment 17•20 years ago
|
||
Yes DanM, it behaved as described for me just as it was. =) Also the new much simpler test case I just uploaded in the attachment works the same right from the bug server. It's just simple html with simple "onclick="window.open(blahbla..." code to open copies of itself in popups. See attachment for details.
Comment 18•20 years ago
|
||
The bug is that popups > dom.popup_maximum are blocked even if the site is in the popups-allowed list.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 19•20 years ago
|
||
*** Bug 261344 has been marked as a duplicate of this bug. ***
Comment 20•20 years ago
|
||
Happens in OS/2 also. I don't think it is OS specific.
Comment 21•20 years ago
|
||
This problem is occuring for me under Windows XP. Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20041001 Firefox/0.10.1 I have Chatzilla 0.9.66 and Mouse Gestures 1.0 installed.
Comment 22•20 years ago
|
||
THis also happens for me, e.g. on sites with the "Gallery" PHP-Skript (gallery.sf.net) installed. The popup that shows the progress bar while adding photos is blocked. Other popups like the login-popup or the "add photos" popup are not (they are triggerd by a mouse click). The site is in the "Allow popups from" list - if I try to open the popup using the "Show $URL" option in "the blocked popup"-warning, the popup still does not open, but the number of popups in the "Firefox prevented this site from opening $number popup windows" is increased by one. Also, one more option "Show $URL" is available. The site is in the allowed list, so in the warning message there is only to option to "block popups from this site".
Comment 23•20 years ago
|
||
Another website to reproduce it: -set dom.popup_maximum to 2 -goto http://www.friendscout24.de/ -on the right side, you'll see a list of logged in people. if not, do a "suche" (search) -click on 3 of these nicknames -the 3rd one should not open -close the opened windows -click on other nicknames. from now on, it is not possible to open the profile window any longer. I'll be blocked.
Comment 24•20 years ago
|
||
*** Bug 269535 has been marked as a duplicate of this bug. ***
Comment 25•20 years ago
|
||
*** Bug 262810 has been marked as a duplicate of this bug. ***
Comment 26•20 years ago
|
||
see also http://forums.mozillazine.org/viewtopic.php?t=175798&highlight=debug+popup+blocker
Summary: Popups from a Site that is in the "Allowed List" are blocked, starting with the n-th popup → Popups from a Site that is in the "Allowed List" (whitelist) are blocked, starting with the n-th popup (dom.popup_maximum)
Comment 27•20 years ago
|
||
*** Bug 272628 has been marked as a duplicate of this bug. ***
Comment 28•20 years ago
|
||
*** Bug 275915 has been marked as a duplicate of this bug. ***
Comment 29•20 years ago
|
||
*** Bug 276011 has been marked as a duplicate of this bug. ***
Comment 30•20 years ago
|
||
(In reply to comment #4) > ... Tabbrowser > Extensions (TBE) is known to interfere with the popup blocker in some way, maybe > you have it installed? If so - does removing this extension solve the problem? I experience this behavior on Windows XP (SP2) with FF1.0, and have no extensions installed.
Comment 31•20 years ago
|
||
OK, I've hit this bug one too many times. (It makes our product look silly, because we pop up windows....and after the 20th pop-up...boom....no more pop-ups allowed.) So, I stripped and stripped and now I have a simple case that reproduces the bug every time. I'm running Windows XP SP2 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 Extract the 3 HTML files. In Firefox, using "about:config" set dom.maximum_popup to a low value. (I used 5). Open page1.html, and close the resulting pop-up window. Refresh the window 5 times (each time, closing the pop-up). On the 6th refresh...no more pop-ups are allowed, when they should be.
Comment 32•20 years ago
|
||
linux, ff1.0. Problem gone after uninstalling TabExtensions.
Comment 33•19 years ago
|
||
*** Bug 257501 has been marked as a duplicate of this bug. ***
Comment 34•19 years ago
|
||
Running into this problem on www.dba.dk which is a private market place. Hovering over a link shows a preview picture of the item clicking the link will open a new window with a picture of the item. The mouse over preview does not work in FireFox which is not that critical. The real critical thing is that all of a sudden the Popup blocker will block all atempts at view an item. To solve this I disabled the popup blocker altogether this had no effect at all - the popup blocker still prevents me from viewing any items after some time - i just get the yellow info that popups have been blocked.
Comment 35•19 years ago
|
||
I have had the exact same problem for a long time now. I'm running Windows 2000 SP4, Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.5) Gecko/20041110 Firefox/1.0. All popups become blocked after a while (even if I have turned off popup blocking completely).
Comment 36•19 years ago
|
||
*** Bug 284528 has been marked as a duplicate of this bug. ***
Comment 37•19 years ago
|
||
*** Bug 261678 has been marked as a duplicate of this bug. ***
Comment 38•19 years ago
|
||
*** Bug 268591 has been marked as a duplicate of this bug. ***
Comment 39•19 years ago
|
||
Is this bug going to be fixed in the upcoming 1.1 release? I believe its quite well described now and its embarrassing when talking about web applications that use dialog windows to display important information.
Comment 40•19 years ago
|
||
Also seeing this with Lotus Notes Webmail 6.5.
Comment 41•19 years ago
|
||
*** Bug 302563 has been marked as a duplicate of this bug. ***
Comment 42•19 years ago
|
||
*** Bug 275670 has been marked as a duplicate of this bug. ***
Comment 43•19 years ago
|
||
*** Bug 310692 has been marked as a duplicate of this bug. ***
Comment 44•19 years ago
|
||
from bug 260264 comment 3: This is a limit imposed in nsGlobalWindow, and therefore doesn't really have any clue about any whitelists. http://lxr.mozilla.org/mozilla/source/dom/src/base/nsGlobalWindow.cpp#3162
Comment 45•19 years ago
|
||
*** Bug 297105 has been marked as a duplicate of this bug. ***
Comment 46•19 years ago
|
||
(In reply to comment #44) Correct is "from Bug 297105 comment 3". Sorry.
Comment 47•19 years ago
|
||
This isn't quite correct. The limit is enforced in nsGlobalWindow::CheckForAbusePoint() and it ignores the whitelist by design. It sets the state to openOverridden whereas openAbused would be required to block the popup respecting the whitelist and popup blocker deactivation. The reasoning behind this was explained in some other bug (closed as INVALID as far as I remember). The original point of this bug was a very different one. According to the report gOpenPopupSpamCount isn't always reset correctly, causing popups to be disallowed even if there are no open popups at all. Unfortunately it seems that almost nobody can reproduce this problem.
Updated•19 years ago
|
Assignee: firefox → nobody
QA Contact: mconnor → preferences
Version: 1.0 Branch → Trunk
Comment 48•18 years ago
|
||
Using the testcase provided in comment #11, popups will always open. You've whitelisted the site and are performing a user initiated action that opens a popup on the allowed site. This is expected. Try http://www.popuptest.com/popuptest4.html to do the mouseover technique described in comment #10, this works as expected with Bon Echo from 2006073104 On Win XP. Once you hit the the dom.popup_maximum pref limit of 20, the sneaky mouse over popup is blocked. WFM
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WORKSFORME
Comment 49•16 years ago
|
||
First set the field dom.popup_maximum (in about:config) to 3 for example, open the test.html file. A popup will open, here click the link 'open popup'... close the popup and retry it 3 times... It's important that the test.html opens the test.html as popup from the 'onload' event once. In the test.html is an Object called 'btnClass', here you can see that teh 'onclick' calls a function that returns 'false'. And the 'onmouseup' calls the callback (that opens a popup). In the test2.html is a simple 'a'-tag and an 'init' function called at document's 'onload'. If i do not call the init function at the test2.html or use the 'onclick' methode in the btnClass-object to call the callback, then it works.
Comment 50•16 years ago
|
||
This is still happening and is a duplicate of Bug 320031 as well as others. I would recommend that firefox be shipped with default value in dom.popup_maximum of -1 to disable it. It should not block popups ever if the site is in the allowed list.
Comment 51•16 years ago
|
||
This bug is still there, especially affecting pandora.com 's song/artist information pop-ups. If the limit is not going to be disabled, would it be possible to at least fix the pop-up blocked message? That is, when the limit is reached, show the pop-up blocked message, but have an option to tell Firefox that the pop-ups are benign, or to reset the counter at least? Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5
Comment 53•15 years ago
|
||
RESOLVED WORKSFORME is not appropriate! On autoscout24 I'm loading the search results, each car article is the loaded manually into tabs. If I want to view the images of that particular car, they are forcefully opened in a popup window through window.open which initiates the counter. Now if I close that popup window and the parent tab, shouldn't the counter be reset? If I try to open another 20 car articles in separate tabs, as was done previously, the counter keeps its state and the popup blocker stops the window.open action to perform as expected. If I allow the popup from the status bar icon, the counter resets and the operation can continue for another 20/19 times. This behavior doesn't work as intended as the counter should be applied for each document separately, since it's marked as a per document setting (dom.) and not as a global setting. As a side note, my dom.popup_allowed_events are set to dblclick reset to prevent rogue on click events to be executed (ex: uploading.com download button). Adding click to the list overcomes the limit but leaves the browser open to other unwanted behaviors during this event. Workaround: dom.popup_maximum = -1 (Thanks Don!)
Comment 55•15 years ago
|
||
REOPENED due to comment 53
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment 56•15 years ago
|
||
I have come up against this same limitation (dom.popup_maximum = 20) on a daily basis while working with the Oracle PeopleSoft Enterprise software at my workplace. If I set the limit to say, 23, I am then able to open up 3 more popup windows and no more. I have my corporate domain (.arrisi.com) in the whitelist, and have even tried it with the popup blocker entirely disabled... Yet after clicking repeatedly on interface widgets that cause popup windows to appear, there is no ability to get them to open again short of restarting the browser, or setting dom.popup_maximum=-1 as suggested in comment 53. I don't enjoy restarting the browser as it usually breaks logged-in sessions, however due to the memory management bugs still plaguing FF it's not like I have much of a choice but that's another story (@ 645MB Real+650MB Virt right now...) I'm honestly flabbergasted that this is still an issue in FireFox 3.5.2... Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2
Comment 57•15 years ago
|
||
So the deal here is that after 20 popup windows are created, user initiated or otherwise, we block any subsequent popup, user initiated or otherwise? Does that persist across sessions?
Comment 58•15 years ago
|
||
From skimming the code, this might not be trivial to fix, since it seems we treat user-action-initiated popups as "bad" here. There's also the non-trivial aspect of not having a way to treat popups as user-initiated when opened by plugins. This is a core DOM bug, since that's where this logic is implemented, but what seems like a complete solution is: a) The primary maximum should be per-event (and probably lower, since > 3 popups per click is almost certainly an outlier) b) User-initiated actions should never count as abuse for this count. c) Ideally we'd make it possible for plugins to play nice with user-initiation. d) For page-initiated events that are not user-initiated, we should simply let the abuse count reset over time so long-running apps don't hit this when they're not being abusive. Alternatively, we could decrement the abuse count when windows are closed (look for the opener onclose).
Component: Preferences → DOM: Events
Flags: blocking-firefox3.6?
Product: Firefox → Core
QA Contact: preferences → events
Updated•15 years ago
|
Flags: blocking1.9.2?
Comment 59•15 years ago
|
||
Marking this a blocker for now, but if this turns out to be very scary I could be convinced we don't need it for 1.9.2.
Assignee: nobody → bnewman
Flags: blocking1.9.2? → blocking1.9.2+
Assignee | ||
Comment 60•15 years ago
|
||
One big flaw in the implementation of the current model (limiting the maximum number of open popups) is that gOpenPopupSpamCount is decremented inside nsGlobalWindow::Cleanup, which is called from the nsGlobalWindow destructor. Once upon a time, the window might have been destroyed immediately after it was closed, but nowadays the reference graph is more complicated, and nsGlobalWindows tend to be destroyed during cycle collection. The non-deterministic delay before cycle collection contributes to the appearance that the gOpenPopupSpamCount never resets. At a bare minimum, gOpenPopupSpamCount needs to be tied more directly to the closing of windows.
Assignee | ||
Comment 61•15 years ago
|
||
The other flaw in the current implementation is that we do not honor all dom.popup_allowed_events. The onmouseup event, for instance, still does not count as a user-initiated event, even after I append " mouseup" to the popup_allowed_events pref.
Assignee | ||
Comment 62•15 years ago
|
||
Setting aside the question of whether the popup maximum should be global or per-event, here's some pseudocode that I propose for determining if a popup should count against the max: def countsAgainstPopupMaximum(event): assert "click" in dom.popup_allowed_events if popup blocker enabled: if host whitelisted: return event not in dom.popup_allowed_events else: return event is not "click" else: if host whitelisted: return false else: return event not in dom.popup_allowed_events Implementation is complicated by nsGlobalWindow not knowing what the event actually was, but enough information is provided by gPopupControlState.
Assignee | ||
Comment 63•15 years ago
|
||
mconnor, I believe this patch satisfies (b) and (d) from your list. Specifically, whitelisted hosts are never restricted from opening popups in response to events listed in dom.popup_allowed_events, but the dom.popup_maximum limit applies for events not listed in dom.popup_allowed_events. Non-whitelisted hosts can open popups from "click" events without restriction, but other events in dom.popup_allowed_events are subject to the dom.popup_maximum limit. Events not listed in dom.popup_allowed_events are prevented altogether from opening popups. When popup blocking is turned off, all hosts are effectively whitelisted (i.e., the dom.popup_maximum limit applies only to events not in dom.popup_allowed_events). (d) is satisfied just because of the delay before cycle collection: non-user-initiated events count against the dom.popup_maximum limit (assuming the page is whitelisted), but at some indeterminate time after a popup window closes, the penalty is erased by window destruction. If the popups are infrequent, they will never be blocked in practice. Yes, I admit I'm doing an about-face since posting comment 60, as I'm now claiming the non-determinism as a benefit ;) For example, if your bank pops up a "Click here to avoid auto-logout!" window, and you click the button, and you go back to your banking, then by the time another such popup appears, the penalty will certainly have been forgotten. The delay is on the order of 10-30 seconds. Given the trickiness of implementing (a), I'd like to file a followup bug. I'm tempted to do the same for (c). After whitelisting pandora.com, I've found their buttons perfectly usable. I'm curious what use cases remain for plugins to override the default behavior.
Attachment #401353 -
Flags: ui-review?
Attachment #401353 -
Flags: review?(jst)
Assignee | ||
Comment 64•15 years ago
|
||
Comment on attachment 401353 [details] [diff] [review] Simplification of PopupControlState logic in nsGlobalWindow. I should probably also explain that the crux of the new logic is in the switch statement: + switch (abuse) { + case openControlled: + case openAbused: + case openOverridden: + if (PopupWhitelisted()) + abuse = PopupControlState(abuse - 1); + case openAllowed: break; + default: + NS_WARNING("Strange PopupControlState!"); + } Whitelisting counts for a single reduction in the abuse level (abuse - 1), instead of just granting a free pass (openAllowed). On a case-by-case basis, this seems to give the desired behavior. An initial value of abuse >= openAbused corresponds to events not in dom.popup_allowed_events, >= openControlled to events in dom.popup_allowed_events other than "click," and >= openAllowed to "click" events. I wish there were a more elegant way of motivating this heuristic, but if you play around with http://people.mozilla.org/~bnewman/bugs/260264/winopen.html the behavior should speak for itself. The rest of the patch has to do with removing CheckOpenAllow and the OpenAllowValue enum, which turned out to be unnecessary.
Attachment #401353 -
Flags: ui-review? → ui-review?(mconnor)
Assignee | ||
Updated•15 years ago
|
Assignee | ||
Comment 65•15 years ago
|
||
(In reply to comment #64) > An initial value of abuse >= > openAbused corresponds to events not in dom.popup_allowed_events, >= > openControlled to events in dom.popup_allowed_events other than "click," and >= > openAllowed to "click" events. The second two of those three >='s should have been =='s. My bad. Very sorry for the flurry of bugmail.
Assignee | ||
Comment 66•15 years ago
|
||
Mac and Windows tryserver builds available here: https://build.mozilla.org/tryserver-builds/bnewman@mozilla.com-try-2ba191dacee2/try-2ba191dacee2-macosx.dmg https://build.mozilla.org/tryserver-builds/bnewman@mozilla.com-try-2ba191dacee2/try-2ba191dacee2-win32.zip Linux failed to build for no good reason. My apologies. Can those of you who have been frustrated in the past try out these builds and report your level of satisfaction? The URL given in comment 64 above may help test various kinds of events: http://people.mozilla.org/~bnewman/bugs/260264/winopen.html In particular, though, I'm interested to hear whether actual applications see improved behavior with the latest patch.
Updated•15 years ago
|
Priority: -- → P2
Target Milestone: --- → mozilla1.9.2
Assignee | ||
Comment 67•15 years ago
|
||
I may have gotten a little carried away but these tests cover almost everything. I say "almost" only because they do not currently toggle the preference that turns popup blocking on and off (the first checkbox under the content tab of the preferences dialog). I'm assuming the popup blocker will be turned on by default. The crux of the test logic is here: +/** + * Please be patient; run_tests opens/closes a LOT of windows. + */ +function run_tests() { + hold(popupEvents, "click mouseup", function() { + // Note: pm.UNKNOWN_ACTION is the same as pm.DENY_ACTION. + hold(popupPrivilege, pm.DENY_ACTION, function() { + testIntentional("click"); + testProbablyIntentional("mouseup"); + testProbablyUnintentional("mouseover"); + }); + hold(popupPrivilege, pm.ALLOW_ACTION, function() { + testIntentional("click"); + testIntentional("mouseup"); + testProbablyIntentional("mouseover"); + }); + }); + + hold(popupEvents, "click", function() { + // Note: pm.UNKNOWN_ACTION is the same as pm.DENY_ACTION. + hold(popupPrivilege, pm.DENY_ACTION, function() { + testIntentional("click"); + testProbablyUnintentional("mouseup"); + testProbablyUnintentional("mouseover"); + }); + hold(popupPrivilege, pm.ALLOW_ACTION, function() { + testIntentional("click"); + testProbablyIntentional("mouseup"); + testProbablyIntentional("mouseover"); + }); + }); + + window.open.close(); // just in case +} The |hold| function assigns a value (second parameter) to a preference/privilege (first parameter) for just as long as it takes to execute the callback function (third parameter). No matter what happens during the callback, the preference/privilege gets reset to its original value. This seemed like a good idea because these tests do a lot of preference manipulation to exercise all the popup blocker cases, and I wanted to be sure everything got undone in the end.
Attachment #401353 -
Attachment is obsolete: true
Attachment #405645 -
Flags: ui-review?
Attachment #405645 -
Flags: review?(jst)
Attachment #401353 -
Flags: ui-review?(mconnor)
Attachment #401353 -
Flags: review?(jst)
Assignee | ||
Comment 68•15 years ago
|
||
(In reply to comment #67) > I may have gotten a little carried away but these tests cover almost > everything. I say "almost" only because they do not currently toggle the > preference that turns popup blocking on and off (the first checkbox under the > content tab of the preferences dialog). I'm assuming the popup blocker will be > turned on by default. No need to assume this: the general popup blocking preference is just tied to dom.disable_open_during_load, which is easy to force to be true: +<!DOCTYPE HTML> +<html> +<!-- @@ -423,6 +423,7 @@ new file mode 100644 + */ +var popupMax = makePrefAccessor("dom.popup_maximum"), + popupEvents = makePrefAccessor("dom.popup_allowed_events"), ++ blockPopups = makePrefAccessor("dom.disable_open_during_load"), + popupPrivilege = function(permission) { + var old = pm.testPermission(ownUri, "popup"); + if (arguments.length) { @@ -528,7 +529,7 @@ new file mode 100644 +setTimeout(function() { + netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect"); + check_sanity(); -+ run_tests(); ++ hold(blockPopups, true, run_tests); + SimpleTest.finish(); +}, 200); +
Assignee | ||
Comment 69•15 years ago
|
||
Applying the changes from comment 68.
Attachment #405645 -
Attachment is obsolete: true
Attachment #405885 -
Flags: ui-review?
Attachment #405885 -
Flags: review?(jst)
Attachment #405645 -
Flags: ui-review?
Attachment #405645 -
Flags: review?(jst)
Comment 70•15 years ago
|
||
Comment on attachment 405885 [details] [diff] [review] Making sure the popup blocker is enabled - In nsGlobalWindow::PopupWhitelisted(): + nsCOMPtr<nsIDOMWindow> topWindow; + GetTop(getter_AddRefs(topWindow)); + + nsCOMPtr<nsPIDOMWindow> topPIWin(do_QueryInterface(topWindow)); + + return topPIWin && (!IsPopupBlocked(topPIWin->GetExtantDocument()) || + !IsPopupBlocked(mDocument)); This isn't new in this patch, but since we're changing this code it seems worth making this change that the window in question either doesn't have popups blocked or that any of its parents don't, rather than just checking the top window. Assuming that tests out well, that is. r=jst with that looked into.
Attachment #405885 -
Flags: ui-review?(mconnor)
Attachment #405885 -
Flags: ui-review?
Attachment #405885 -
Flags: review?(jst)
Attachment #405885 -
Flags: review+
Assignee | ||
Comment 71•15 years ago
|
||
The simplest way to re-implement nsGlobalWindow::PopupWhitelisted was to use recursion. I figure that's ok because we'll probably hit other frame-nesting limits before we overflow the stack checking for popup permissions.
Attachment #405885 -
Attachment is obsolete: true
Attachment #405885 -
Flags: ui-review?(mconnor)
Assignee | ||
Comment 72•15 years ago
|
||
Attachment #407648 -
Attachment is obsolete: true
Assignee | ||
Comment 73•15 years ago
|
||
Using the window.open(null, win.name) trick to communicate between grandchild and grandparent iframes. It seems as though gOpenPopupSpamCount is incremented at some point in the preceding mochitests without being decremented, so I can't open dom.popup_maximum windows before being blocked. The desired behavior can still be tested pretty well, though: in this patch I'm verifying that opening more than dom.popup_maximum windows results in a blocked popup, and that closing windows after a block reclaims the gOpenPopupSpamCount allowance.
Attachment #407677 -
Attachment is obsolete: true
Attachment #408099 -
Flags: review+
Is this ready to land?
Comment 75•15 years ago
|
||
Comment on attachment 408099 [details] [diff] [review] Simpler and more defensive mochitests. sr=jst as well. Let's get this in!
Attachment #408099 -
Flags: superreview+
Assignee | ||
Comment 76•15 years ago
|
||
(In reply to comment #75) > (From update of attachment 408099 [details] [diff] [review]) > sr=jst as well. Let's get this in! After running it past the tryserver one last time, with no problems: http://hg.mozilla.org/mozilla-central/rev/95c4abb606ee
Status: REOPENED → RESOLVED
Closed: 18 years ago → 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 77•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a7268aab8bf5
status1.9.2:
--- → final-fixed
Comment 78•10 years ago
|
||
I have a similar problem with FF 32.0.1 (Windows) Popup Blocker is disabled, but after opening the 21st popup, I get a "Firefox prevented this site from opening a popup window" sometimes everything seems to work OK (all popups are opened as expected), but after a while it starts failing again. For now it seems to start failing after I used "Clear Recent History", Time range Everything I use Selenium IDE 2.8.0 with the attached open_test.html as test case to reproduce the problem currently, my workaround is to set dom.popup_maximum to a large value
Comment 79•9 years ago
|
||
I see that this bug is fixed but I noticed that this is happening in FF 38.0.1. Changing dom.popup_maximum works. Is this a new bug or regression of old one?
Comment 80•9 years ago
|
||
This is an ancient bug. Please file a new one.
Comment 81•6 years ago
|
||
(In reply to Ilya from comment #79) > I see that this bug is fixed but I noticed that this is happening in FF > 38.0.1. Changing dom.popup_maximum works. Is this a new bug or regression of > old one? (In reply to Olli Pettay [:smaug] from comment #80) > This is an ancient bug. Please file a new one. Did a new one ever get filed?
You need to log in
before you can comment on or make changes to this bug.
Description
•