popup window blocked at allowed site if disable_open_click_delay pref is set

RESOLVED FIXED in mozilla1.8alpha2

Status

SeaMonkey
UI Design
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: Dan M, Assigned: Dan M)

Tracking

({fixed-aviary1.0, fixed1.7})

Trunk
mozilla1.8alpha2
fixed-aviary1.0, fixed1.7
Bug Flags:
blocking1.7 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Assignee)

Description

14 years ago
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.
(Assignee)

Comment 1

14 years ago
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.
(Assignee)

Comment 2

14 years ago
Created attachment 151033 [details] [diff] [review]
document load condition trumps click-delay condition
(Assignee)

Updated

14 years ago
Attachment #151033 - Flags: superreview?(jst)
Attachment #151033 - Flags: review?(bryner)
Comment on attachment 151033 [details] [diff] [review]
document load condition trumps click-delay condition

sr=jst
Attachment #151033 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 4

14 years ago
Request blocking 1.7 because this bug makes the popup blocker whitelist largely
ineffective for people who use the disable_open_click_delay pref.
Flags: blocking1.7?

Updated

14 years ago
Flags: blocking1.7? → blocking1.7-
(Assignee)

Comment 5

14 years ago
...leading to spurious bug reports usually closed WFM because of
miscommunication and whatnot. Oh well.
(Assignee)

Comment 6

14 years ago
Created attachment 151245 [details]
local testcase with instructions
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.
Attachment #151033 - Flags: review?(bryner) → review+
(Assignee)

Comment 8

14 years ago
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.
Attachment #151033 - Flags: approval1.7.1?

Comment 9

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

a=mkaply for 1.7.1
Attachment #151033 - Flags: approval1.7.1? → approval1.7.1+
(Assignee)

Comment 10

14 years ago
Checked in to trunk for 1.8a2, 1.7 branch for 1.7.1, and Aviary 1.0 branch for
0.9.1+.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8alpha2

Comment 11

14 years ago
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 12

14 years ago
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.
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago14 years ago
Keywords: fixed-aviary1.0, fixed1.7
Resolution: --- → FIXED
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.