User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:22.214.171.124) Gecko/2008092417 Firefox/3.0.3 (.NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:126.96.36.199) Gecko/2008092417 Firefox/3.0.3 (.NET CLR 3.5.30729) When (save) dialog is open in pop up blocker does not work at all and this is reproducible every time. Reproducible: Always Steps to Reproduce: 1. Turn on the pop-up blocker 2. Go to a site that has pop-ups that are done via JS over a period of time (that means they have to re-try, and hence not only on page load. 3. Pick a graphic or whatever to save from the site and leave the 'save dialog' open and wait for the JS to launch the popup. 4. The pop-up will be launch every time. Actual Results: The pop-up's appear even though the blockers is enabled Expected Results: no pop-ups
This is a mass search for bugs which are in the Firefox General component, are UNCO, have not been changed for 500 days and have an unspecified version. Reporter, can you please update to Firefox 3.6.10 or later, create a fresh profile, http://support.mozilla.com/en-US/kb/managing+profiles, and test again. If you still see the issue, please update this bug. If the issue is gone, please set the status to RESOLVED > WORKSFORME.
No reply from reporter, INCOMPLETE. Please retest with Firefox 3.6.12 or later and a new profile (http://support.mozilla.com/kb/Managing+profiles). If you continue to see this issue with the newest firefox and a new profile, then please comment on this bug.
The reason seems to be because the save and open dialogs are open through a XUL command event and those events are considered safe so the popup blocker level is reduced to "openControlled" thus making all popup to pass. So it doesn't happen when using an <input type='file'> which makes this bug less critical... Though, I wonder why we need to check if the event is a XUL command and reduce the popup blocker in that case. Do we really want to have a popup blocker for chrome? I mean can't we just by-pass the popup-blocker check when we have chrome privileges?
So, I'm going to fix that in two parts: 1. Make sure popup blocker is by-passed when the caller has chrome privileges (bug 662519); 2. Don't reduce the abuse level when the command event is used. I could probably do 2 without 1 but the only reason we could be reducing the abuse level is that the popup blocker was sometimes interfering with the chrome/xul pages. 1 will make sure that this can't happen. In addition, there is no reason to use the popup blocker in the chrome context. At least, I don't see any...
Created attachment 537808 [details] [diff] [review] Patch v1
Actually, my test is using "ctrlKey: true" but I wonder how would that behave on a Mac. Should I use metaKey in that case?
Jst, any update regarding the review?
Comment on attachment 537808 [details] [diff] [review] Patch v1 Sorry for the delay, I thought I had already reviewed this... :( r=jst
Ehsan, the test in this patch seems to fail because of other tests using mockObject.js. It seems that calling my test followed by toolkit/content/tests/browser/browser_save_resend_postdata.js or the latter followed by my test always creates a time out. Here is the patch I pushed to try (the test has changed a bit): https://hg.mozilla.org/try/rev/c56b80fa9d76 And the error: http://tinderbox.mozilla.org/showlog.cgi?log=Try/1312340808.1312345039.7982.gz#err0 By any chance, do you have any idea of what could be happening?
Created attachment 551933 [details] [diff] [review] Patch with updated tests
So, I'm trying to understand what's happening with this test... I've tried to improve mockObjects.js by unregistering the current factory and re-register it correctly at the end and that helped to narrow down the issue. This code fails with an exception thrown when .createInstance() is called because no factory is found: (I believe the variable names are self-explanatory) componentRegistrar.unregisterFactory(this._originalCID, this._originalFactory); componentRegistrar.registerFactory(this._cid, "", this._contractID, this._mockFactory); Components.classes[this._contractID].createInstance(Components.interfaces.nsIFilePicker); It seems that nsComponentManagerImpl::RegisterFactory is correctly called and works so a factory is actually registered but when nsComponentManagerImpl::CreateInstance is called, there is indeed no factory but these methods don't have the same CID in parameter. ::RegisterFactory has this._cid, as expected while ::CreateInstance is using the CID from the previous factory. I really don't understand why the previous CID is still around except if Components.classes[this._contractID] doesn't return the correct object and nsJSCID::mDetails hasn't been updated...
Bug 678166 comes with the required fix to get the tests passing... Finally able to push! :)
I backed this out of mozilla-beta / Firefox8 due to bug 679961: http://hg.mozilla.org/releases/mozilla-beta/rev/f8b0cc0c644a We need to see if we can alter this a bit in bug 679961 (or get product to make an explicit decision)
Verified that this fix is backed out from Firefox 8 Beta 6 (build 2) using the test case attached in Comment 4. Tested on Windows XP, Windows 7, Mac OS X 10.6 and Ubuntu 11.10 - pop-ups appear when opening the save dialog. Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0 Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0) Gecko/20100101 Firefox/8.0 Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0 Should this bug be reopened?
(In reply to Simona B [QA] from comment #17) > Should this bug be reopened? The patch has been backed out from Aurora but is still applied on mozilla-central so the bug remains fixed.
[Triage Comment] Due to 679961, please back this bug out on aurora and beta, and also m-c if we don't think 679961 will be fixed in the short term.
Reopening, as it seems (bug 679961 comment 15) this has now been backed out everywhere.
I can no longer reproduce this using nightly on Linux x86_64 as of today.