247017 - popup window blocked at allowed site if disable_open_click_delay pref is set

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Dan M]

From Boris Piwinger in bug 210664:
The (pseudosecret) dom.disable_open_click_delay pref prevents windows from being
opened even if they're from a site allowed in the popup whitelist, if the
attempt to open the window occurs more than a user-configurable time delay after
the last mouseclick in the window. This is being triggered under inappropriate
circumstances. Pretty much always, in fact.

1) Set the pref to any non-zero value using about:config
2) Activate popup blocking
3) Add the site given in the URL field of this report to the whitelist
4) Navigate to that same website, which attempts to open a window
   in the load handler

Note some reports say to skip step 3. Then after the popup is (correctly)
refused, add the site to your whitelist by clicking or double-clicking
(depending on your client) on the blocked popup indicator in the statusbar, then
reload the page. Don't click in the page! Or if you do, wait long enough to
trigger the click_delay pref set in step (1). However this shouldn't matter and
doesn't matter in my testing.

With that pref non-zero, the popup cannot open. This should be true of any site
that attempts to open a window in the load handler.

Comment 1

[Dan M]

More notes:

Though the disable_open_click_delay pref is scheduled to be backed out (bug
160670), it has a popular following in some circles and may never be. We may as
well fix it while we're deciding.

The fix would seem to be to check the click_delay condition only if the document
has finished loading, and therefore after checking whether the document has been
loaded. But I recall there was some issue with doing things in that order, so
that wants to be investigated first.

Also consider bug 143423.

Comment 2

[Dan M]

Attachment: document load condition trumps click-delay condition

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 151033 [details] [diff] [review]
document load condition trumps click-delay condition

sr=jst

Comment 4

[Dan M]

Request blocking 1.7 because this bug makes the popup blocker whitelist largely
ineffective for people who use the disable_open_click_delay pref.

Comment 5

[Dan M]

...leading to spurious bug reports usually closed WFM because of
miscommunication and whatnot. Oh well.

Comment 6

[Dan M]

Attachment: local testcase with instructions

Comment 7

[Brian Ryner (not reading)]

Comment on attachment 151033 [details] [diff] [review]
document load condition trumps click-delay condition

>Index: dom/src/base/nsGlobalWindow.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v
>retrieving revision 1.673
>diff -p -b -u -6 -r1.673 nsGlobalWindow.cpp
>--- dom/src/base/nsGlobalWindow.cpp	11 Jun 2004 00:47:59 -0000	1.673
>+++ dom/src/base/nsGlobalWindow.cpp	17 Jun 2004 15:49:57 -0000
>@@ -2955,35 +3020,41 @@ GlobalWindowImpl::CheckForAbusePoint()
>     item->GetItemType(&type);
>     if (type != nsIDocShellTreeItem::typeContent)
>       return openAllowed;
>   }
> 
>   PRUint32 abuse = openAllowed; // level of abuse we've detected
>+  PRInt32  intPref;
>+
>+  // is the document being loaded or unloaded?
>+  if (!mIsDocumentLoaded)
>+    abuse = openAbused;
> 
>-  // disallow windows after a user-defined click delay
>-  PRInt32 intPref =
>-    nsContentUtils::GetIntPref("dom.disable_open_click_delay");
>+  /* Disallow windows after a user-defined click delay.
>+     This is a consideration secondary to document load because
>+     of its stronger response (openOverridden). See bug 247017. */
>+  if (abuse == openAllowed) {
>+    intPref = nsContentUtils::GetIntPref("dom.disable_open_click_delay");

Please move intPref's declaration inside this |if| (and use another one down at
the end of this function inside another |if|).

r=me with that change.

Comment 8

[Dan M]

Comment on attachment 151033 [details] [diff] [review]
document load condition trumps click-delay condition

Requesting 1.7.1. This patch fixes a problem seen by people who use the
dom.disable_open_click_delay pref. I've seen several reports that no popup
window can ever be opened, even with the popup blocker disabled. It's confusing
and unexpected and tends to be dismissed because no one thinks to report all
the circumstances, so no one can reproduce it. I'm pretty sure that everyone
who sees such behaviour is seeing this bug.

Comment 9

[Mike Kaply [:mkaply]]

Comment on attachment 151033 [details] [diff] [review]
document load condition trumps click-delay condition

a=mkaply for 1.7.1

Comment 10

[Dan M]

Checked in to trunk for 1.8a2, 1.7 branch for 1.7.1, and Aviary 1.0 branch for
0.9.1+.

Comment 11

[Boris 'pi' Piwinger]

Using Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.2) Gecko/20040803 still no
popup shows at the site given in the URL field. The blocked symbol shows up.

pi

Comment 12

[Dan M]

I don't know why I claimed it was fixed in 1.7.1. It's fixed on the trunk in
1.8a2, on the Mozilla 1.7 branch, and on the Aviary branch, but not on the
little security fix minibranches.