Closed Bug 198941 Opened 17 years ago Closed 17 years ago

Popup blocker bustage

Categories

(Firefox :: General, defect, major)

x86
All
defect
Not set
major

Tracking

()

VERIFIED FIXED
Firebird0.6

People

(Reporter: seb, Assigned: bugzilla)

References

()

Details

(Keywords: regression)

Attachments

(2 files)

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4a) Gecko/20030323 Phoenix/0.5

The popup blocker is busted:
- When a popup is blocked, the "Unblock site" button does nothing (ie. nothing
is added to the White list)
- In Options/Privacy/Popups, the "Add Site"/"Remove Site"/"Remove all sites"
buttons do nothing. Mor precisely, when Add Site is clicked, a dialog box
prompts the user for an URI, but clicking OK adds nothing to the White list. The
Remove Site and Remove all sites buttons seem functional, but after having
closed the Options panel, nothing is saved (that is to say: no site is removed
from the white list).

This bustage might be related to the context menu bustage that happened in the
same build: bug 198929
*** Bug 198944 has been marked as a duplicate of this bug. ***
On a side-note, since this build, the popup blocker doesn't have the same
behavior to the sites that are already white-listed:
- Before, adding "domain.com" in White-list authorized popups from domain.com
and every sub-domain (www.domain.com, whatever.domain.com, ...) to open.
- Now, it just block popups from domain.com, no more from the sub-domains.
Could it be caused by bug 191380 ?
This is most likely a fallout from bug 191380, but might be fixed by bug 198829.
Please try a 03/24 or 25 build.

> - Before, adding "domain.com" in White-list authorized popups from domain.com
> and every sub-domain (www.domain.com, whatever.domain.com, ...) to open.
Are you sure? I never saw the code for that...
Ok, i think it is not fixed. Are there errors on the javascript console?
>> - Before, adding "domain.com" in White-list authorized popups from domain.com
>> and every sub-domain (www.domain.com, whatever.domain.com, ...) to open.
> Are you sure? I never saw the code for that...

Yup, I'm sure, I tested it with the 20030320.

The bustage is still there in 20030324.
No error in the javascript console for me.
Depends on: 191380
Severity: normal → major
Target Milestone: --- → Phoenix0.6
Attached patch fixSplinter Review
All due to permission patches. This is to make phoenix happy about it.
Fix checked in to Phoenix.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
*** Bug 198929 has been marked as a duplicate of this bug. ***
verified with 20030326.
Status: RESOLVED → VERIFIED
Reopening, I forgot to check one part of the bug.

Michiel, the "unblock site" button that appears when a popup is blocked and you
click the (i) button in the lower-left corner still does nothing.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Target Milestone: Phoenix0.6 → Phoenix0.7
Michiel, to make unblocking popups working again, this file should be updated as
well:
http://lxr.mozilla.org/mozilla/source/browser/base/content/pageReport.js#77

I tried porting to it your patch for pref-features.js, but it just does nothing
here.
What I dont understand:
// This is perverse and backwards.  We have subverted Mozilla's blacklist
implementation
// and made it into a whitelist.  So we want to add this to the list of
"blocked' popups.

Mozilla has a whitelist for quite some time. Why is that still here?
This part of the file was checked in on Oct 8 by David Hyatt, according to
Bonsai. I'm not sure that Mozilla already had a whitelist at this time.
Besides, the file hasn't been updated in the last two months, at least.
That means that it has been broken for a few months now? The functions add the
site to the blacklist instead of whitelist...
Works fine in the build from the 16th March (and the 20th, i think) so it's only
been broken a short while.
I just tested, and it still worked w/ Mozilla/5.0 (Windows; U; Windows NT 5.0;
en-US; rv:1.4a) Gecko/20030321 Phoenix/0.5
There was no Phoenix build on 20030322, and the build on 20030323 had the bug.
It looks like the comment is just wrong... So this should be not too hard to
fix. Except that I don't build phoenix...
More worrying that this has been bumped to 0.7 - Do you really want to release
0.6 with a bug like this?
bringing back into 0.6
Target Milestone: Phoenix0.7 → Phoenix0.6
I think Sébastien is right in comment 13 that we need to have pageReport.js
updated to the new permission code. I get this error in the JS console when
trying to unblock site. 

Warning: reference to undefined property popupmanager.add
Source File: chrome://browser/content/pageReport.js
Line: 57
Error: popupmanager.add is not a function
Source File: chrome://browser/content/pageReport.js
Line: 57

Michiel van Leeuwen, if you don't build Phoenix, and if this is just a js file,
can you patch a Phoenix binary to test changes to that file? If not, could you 
make a build? Instructions are available at the bottom of this page
http://lxr.mozilla.org/mozilla/source/browser/README.html  

We're not far from shipping a Phoenix 0.6 release and this would be a major
regression in our functionality from those recent Mozilla changes. 
Attached patch followup v1Splinter Review
Update pageReport.js to the new api.

One thing I am wondering about is this: While l=using lxt to search for the
text "popupmanager" I came across
http://lxr.mozilla.org/mozilla/source/browser/components/prefwindow/content/pref-advanced.xul

But I could not find that file in the binary distribution. From chromelist.txt
it appears that instead
xpfe/components/prefwindow/resources/content/pref-advanced.xul is used (so from
xpfe). Anyone can give me more info on that?
Michiel, the patch works perfectly here (Mozilla/5.0 (Windows; U; Windows NT
5.0; en-US; rv:1.4a) Gecko/20030411 Phoenix/0.5+). I was able to unblock a site,
and no error was displayed in the JS console.

For the pref-advanced.xul file, you should ask Ben Goodger (I'm cc'ing him).
new fix checked in.

close as necessary
Fixed.
Thanks Michiel and Michael.
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
*** Bug 202269 has been marked as a duplicate of this bug. ***
verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.