Popup suppressed silently

RESOLVED FIXED in Camino1.5

Status

Camino Graveyard
Annoyance Blocking
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Benjamin Tucker, Assigned: Stuart Morgan)

Tracking

({fixed1.8.1.1})

unspecified
Camino1.5
PowerPC
Mac OS X
fixed1.8.1.1

Details

(URL)

Attachments

(1 attachment)

4.89 KB, patch
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1) Gecko/20061006 Camino/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1) Gecko/20061006 Camino/1.0+

In this popup test, camino blocks the popup, but does not display notification of such

Reproducible: Always

Steps to Reproduce:
1. request http://www.popuptest.com/popuptest3.html
2. wait 5 seconds

Actual Results:  
Nothing

Expected Results:  
Notification of popup being blocked

This worked as expected in Firefox 1.5.0.7

Comment 1

11 years ago
For reference, that page launches timed popups every 5 seconds.  I see this too.  Is it possible that this is just bug 333284 in another guise?
I think this is different from bug 333284, since here we never see the notification at all.

This works on the 1.8.0 branch (and in Fx 2 as well).

Håkan, what all have we changed that would have affected this?  It seems like fairly simple JS throwing the popup.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Target Milestone: --- → Camino1.1

Comment 3

11 years ago
Does it work in Camino-1.8? Does it work in Firefox trunk? It might be a gecko change.

If someone wants to debug it, set a breakpoint in the method in the CHBrowser code, where we handle the nsIDOMPopupBlockedEvent, and see if it's even fired for this page... If it is, it's our bug, if it's not, it's not our bug. :)

Comment 4

11 years ago
- Works on today's Minefield
- Broken on Camino on both 1.8 branch and trunk
- Worked with the old popup blocker (ie on Camino 1.0.3)
- Yes, that method gets called

ie, it's definitely our bug.

Comment 5

11 years ago
This sounds like the bug I was just having. Happens at http://www.worthplaying.com/article.php?sid=37960&mode=thread&order=0

The javascript:popUp is being suppressed by Camino's pop-up blocker, but no notification is showing up. Only happens in Trunk for me. 1.8.0 and 1.8 don't block the popups at all, so I'm assuming changes in the trunk were made to include this type of popup.

Popup also blocked in Minefield, with a notification.

Comment 6

11 years ago
This is working on trunk (2006101622) for me now, as is davedit's testcase in comment 5.

cl
(Assignee)

Comment 7

11 years ago
The issue here is that we always defer the blocker display until the pageload finishes, which doesn't work so well for popups that are triggered after the page finishes loading.  We'll need to defer only while the page is loading, not after.

Taking.
Assignee: nobody → stuart.morgan
*** Bug 357642 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 9

11 years ago
Created attachment 243286 [details] [diff] [review]
removes delayed display

Per our discussion, removes the delay entirely to prevent potential issues with very slow-loading pages.  We'll keep our eyes peeled for issues that would make bringing back the delay for popups thrown during load necessary.

The bulk of this change is just moving code unmodified, so it's actually a very small change. As an added bonus, it stops leaking all the blocker notification views.
Attachment #243286 - Flags: superreview?(mikepinkerton)
+    [mBlockedPopupView release]; // retain count of 1 from nib

why do we need this? are we leaking it? shouldn't it go away when the nib goes away?
(Assignee)

Comment 11

11 years ago
(In reply to comment #10)
> +    [mBlockedPopupView release]; // retain count of 1 from nib
> 
> why do we need this? are we leaking it?

Per IRC discussion, yes.
Status: NEW → ASSIGNED
Comment on attachment 243286 [details] [diff] [review]
removes delayed display

sr=pink
Attachment #243286 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 13

11 years ago
Checked in on trunk and MOZILLA_1_8_BRANCH
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: regression → fixed1.8.1.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.