336020 - "Unblock pop-up" doesn't list or whitelist the right domain when page throwing popup is a frameset frame on a different domain

Status: VERIFIED WORKSFORME

(Camino Graveyard :: Annoyance Blocking, defect)

Description

[Marc Gelfo]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20060426 Camino/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20060426 Camino/1.0+

I go to the above URL in the 1.0 stable build, and find the pop-up has been blocked.  I add the url and the domain to the exceptions list, but the pop-up is still blocked.  

Then I upgraded to the nightly build.  The pop-up is still blocked.  I click the unblock button and nothing happens. Reload the page, and the pop-up is still blocked.  What's going on? I want to go to grad school!

Reproducible: Always

Steps to Reproduce:
1.  Goto https://www.indiana.edu/~grdforms/deptforms/onlineapp.htm
2.  Click "unblock"
3.  Reload page, expecting to view pop-up
4.  Goto 2.  
Actual Results:  
Human infinite loop.  (Well, what happens is I open Safari and apply to grad-school)

Expected Results:  
Should have allowed the page to generate the required pop-up.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

The pop-up is coming from https://app.applyyourself.com/common/testPopup.html, not from www.indiana.edu.  

https://www.indiana.edu/~grdforms/deptforms/onlineapp.htm is simply a frameset URL that wraps https://app.applyyourself.com/?id=IU-BLA (the actual content of the page).

I'm not positive about the way this is supposed to work with framesets which have contents from different domains (Firefox acts the same way as we do, so we're "working"), but I think this feature is probably working as designed; we'll check to see before disposing of this bug.

(Note also that bug 331194 will introduce an option to show the blocked popup rather than simply whitelist it, which would have helped a bit here.)

Comment 2

[Stuart Morgan]

(In reply to comment #1)
> I'm not positive about the way this is supposed to work with framesets which
> have contents from different domains (Firefox acts the same way as we do, so
> we're "working"), but I think this feature is probably working as designed;
> we'll check to see before disposing of this bug.

A feature that says it blocked a popup, has a button to unblock the popup, and doesn't unblock the popup when you press the button can't reasonably said to be working.  Firefox being equally broken doesn't mean it's correct behavior.

Comment 3

[Samuel Sidler (old account; do not CC)]

Right, I spoke with some Firefox guys and they agree that this is broken in Firefox as well. I haven't had time to search for bugs in their product, but we might do well to make sure this isn't a dupe then send it to core or something like that.

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

*** This bug has been marked as a duplicate of 280536 ***

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Reopening...bz decided this was a UI bug that was copied-and-pasted into all the apps:

[01:17am] bz: ss: I'll wager money that it's a UI bug that got copied and pasted
[01:18am] ss: Then I'll believe you fully. 
[01:18am] bz: verifies
[01:19am] bz: For example
[01:19am] bz: Firefox does
[01:19am] bz: 426  fillPopupList: function (aEvent)
[01:19am] bz: 433    var uri = gBrowser.selectedBrowser.webNavigation.currentURI;
[01:19am] bz: 448        // Offer an item to allow popups for this site
[01:19am] bz: 449        var allowString = bundle_browser.getFormattedString("popupAllow", [uri.host]);
[01:20am] bz: Note that it's not using the requesting URI from the popup event or anything
[01:20am] ss: I see...
[01:22am] bz: ss: I'd undup and try to get the camino UI fixed
[01:22am] ss: That's exactly what I think we'll do.
[01:22am] ss: looks at ardissone

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

We should follow the conversation in bug 280536; it might not be quite as easy as it first seemed.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

See also bug 280536 comment 6.

I'm not sure we should fix this until we can provide a way of showing you what site the popups are really associated with (and that's also why I think the defaults of whitelisting without showing in 1.0.x and whitelisting and showing on 18branch and trunk are bad).

Take for example cnn.com.  Loading it produces a number of blocked popup notifications (dismiss the bar and it comes back!) but whitelisting cnn.com does nothing; the next visit, the popups are blocked again, since the popups are coming from somewhere else.  In this case, that's a good thing, because the user likely doesn't really want to see these popups (they're evil ads), only real CNN content.

But when we fix this bug, we'd be whitelisting an ad-server that 1) the user doesn't really want whitelisted 2) the user doesn't know he's whitelisting 3) without having shown the user the popup first to allow the user to make an informed decision and 4) with no easy way for the user to discover the actual site spawning the popups so as to remove the site from the exceptions list when he discovers he's getting popups not only from this site but all over the web.

I'll go file a separate bug on changing the default, but we need to keep this and that in mind when thinking about how to fix the popup blocker UI(s).

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Bug 343772 should have "fixed" the CNN case (on the trunk atm, and perhaps later on the branch); see the discussion there for more implications to think about.

Comment 9

[Håkan Waara]

I think the fix for this bug is a trivial change. Anyone want to verify?

Index: BrowserWindowController.mm
===================================================================
RCS file: /cvsroot/mozilla/camino/src/browser/BrowserWindowController.mm,v
retrieving revision 1.279
diff -u -r1.279 BrowserWindowController.mm
--- BrowserWindowController.mm  26 Sep 2006 05:43:15 -0000      1.279
+++ BrowserWindowController.mm  30 Sep 2006 13:07:26 -0000
@@ -1748,7 +1749,7 @@
   popupWindowURI->GetSpec(uriStr);
   
   // whitelist the URL
-  [self whitelistURL:requestingWindowURI];
+  [self whitelistURL:popupWindowURI];
 
   // show the blocked popup
   nsCOMPtr<nsIDOMWindow> openedWindow;

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Hrm, something has changed here.  For starters, the url in this bug seems to no longer produce unrequested popups.  I also checked cnn.com:

Without the patch, we whitelist ads.cnn.com, and on the next visit show the popups automatically (the url of the popups is www.cnn.com/foo)

With the patch, we whitelist www.cnn.com, and on the next visit show the popups automatically.

Ah, the cnn.com case apparently is covered, and fixed, by bug 343772, on branch and trunk.  

Nevertheless, in bug 280536 (the Fx equiv of this bug), they claim that bug 343772 doesn't fix the frameset case, but on the http://www.r3tro.de/frameset.html testcase there, we whitelist the domain throwing the popup (www.coding.de, in one of the frameset's frames, which throws a popup with a heise.de url)--unpatched.  Reloading the page shows the popup automatically.

With the patch, we whitelist heise.de in that case, which is clearly wrong! (and reloading the page shows the blocker bar again, also wrong).

So I think that this bug is now WFM (the patch is clearly wrong, given the testcase from bug 280536).

I think all we need to do is make sure we're showing the right urls when we're constructing a menu listing the blocked popups (and/or sites to whitelist) for the status bar blocker option.

If I seem sane here, then please close this bug appropriately ;)

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Hrm, bug 343772 comment 7-9 indicate that the fix that landed is really not the "right" fix and that at some point they'll fix the trunk in a better way (which we'd need to adapt to, presumably, when it happens).