Popup blocker isn't fast enough or good enough for this page

RESOLVED FIXED in mozilla1.4final

Status

SeaMonkey
UI Design
RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: Brian 'netdragon' Bober, Assigned: Dan M)

Tracking

Trunk
mozilla1.4final
Bug Flags:
blocking1.4 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

15 years ago
2003050714 WinXP

Steps to reproduce:
1. Go to http://www.catholicninjas.org/superfuntime/ with popup blocking 
disabled.
2. Close the window, and some popups come up giving you the "Popup manager" 
dialog box, and try to click yes.

Expected results:
You don't get any popups after clicking yes

Actual results:
Preferences window comes up and soon you have like 20 or more popup windows 
after clicking Yes, with more appearing constantly. Maybe all the popups is 
causing the browser to become unresponsive to you clicking Yes?
(Assignee)

Comment 1

15 years ago
Neat site. Catchy tune.
Assignee: jaggernaut → danm
Target Milestone: --- → mozilla1.5alpha
(Assignee)

Comment 2

15 years ago
Created attachment 125342 [details] [diff] [review]
don't name windows using reserved target names

This unlikely patch fixes the problem. catholicninjas isn't doing anything
tricky; it just opens windows in the unload handler. We were allowing them to
slip through because named windows that will open into an extant window aren't
blocked, and because the popup windows were named "_blank", and because there
was indeed an open window named "_blank". That last one is the bug.

By the way I was able to reproduce this bug only using a new profile, or at
least one that hasn't yet seen a popup window. Disabling popups after the popup
cascade has already begun seems a necessary condition. Otherwise, they were
blocked as expected.
(Assignee)

Updated

15 years ago
Attachment #125342 - Flags: superreview?(jaggernaut)
Attachment #125342 - Flags: review?(mscott)
(Assignee)

Comment 3

15 years ago
Comment on attachment 125342 [details] [diff] [review]
don't name windows using reserved target names

hang on. sounds like this patch will create an incompatibility with IE. better
one's in the works.
Attachment #125342 - Flags: superreview?(jaggernaut)
Attachment #125342 - Flags: review?(mscott)
(Assignee)

Comment 4

15 years ago
Created attachment 125355 [details] [diff] [review]
don't name windows using reserved target names

Like the previous patch except this one makes us behave like IE -- sigh. We
won't name a window _blank (that's the important part), _self retains the
original name, and _foo is as valid a name as any.
Attachment #125342 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #125355 - Flags: superreview?(mscott)
Attachment #125355 - Flags: review?(jst)

Comment 5

15 years ago
Comment on attachment 125355 [details] [diff] [review]
don't name windows using reserved target names

Looks gppd to me, take r= or sr= as needed
Comment on attachment 125355 [details] [diff] [review]
don't name windows using reserved target names

r/sr=jst
Attachment #125355 - Flags: review?(jst) → review+
Putting on the 1.4 blocking list
Flags: blocking1.4+
Comment on attachment 125355 [details] [diff] [review]
don't name windows using reserved target names

I'd still like mscott to sr= this, but I can stamp sr=brendan@mozilla.org in
case that helps get it landed in the branch for tomorrow's builds.

Danm, you around?

/be
Attachment #125355 - Flags: superreview?(mscott) → superreview+
Comment on attachment 125355 [details] [diff] [review]
don't name windows using reserved target names

a=dveditz for drivers
Attachment #125355 - Flags: superreview?
Attachment #125355 - Flags: superreview+
Attachment #125355 - Flags: approval1.4+
(Assignee)

Comment 10

15 years ago
checked in on the 1.4 branch, but not yet on the trunk
Status: NEW → ASSIGNED
Target Milestone: mozilla1.5alpha → mozilla1.4final
(Assignee)

Comment 11

15 years ago
Created attachment 125376 [details]
window naming testcase

Not what this bug was originally about, but the reason it got fasttracked into
the branch. Here's the testcase I've been using; I figure it'll save someone a
minute's effort. Unpatched builds do disappointing things.

Comment 12

15 years ago
Updating qa contact to sarah

Adding keyword fixed1.4, since this has landed on the branch
Keywords: fixed1.4
QA Contact: paw → sairuh
(Assignee)

Comment 13

15 years ago
Comment on attachment 125355 [details] [diff] [review]
don't name windows using reserved target names

Restoring the sr request specifically for docshell sr mscott. The bug already
has reviews from three sr people but we wanted him as well.
Attachment #125355 - Flags: superreview? → superreview?(mscott)
vrfy'd fixed on the branch (tested on win2k and linux rh8.0, 2003.06.11-1.4
commercial) --i can set the pref to block the popups.
Keywords: fixed1.4 → verified1.4
OS: Windows XP → All
Hardware: PC → All

Comment 15

15 years ago
Comment on attachment 125355 [details] [diff] [review]
don't name windows using reserved target names

so we want to only set the name on the docshell if the window is new. That
seems ok to me.
Attachment #125355 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 16

15 years ago
Thanks all. Patch is checked in on the trunk and the 1.4 branch. I believe the
bug is fixed, though there's some question since I had to modify the original
steps to reproduce.

There's still a more general and similar case which is not fixed. For that see
bug 209134.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 17

15 years ago
This change caused regression bug 209082.

It seems to cause that unnamed new windows are no longer called "_blank", which
breaks the assumption of some existing code.
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.