263960 - popup blocker fails to stop synthetic click popup in single window mode

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Dan M]

A synthetic click on an anchor element that opens a new window will succeed in
opening the new window if Firefox is in Single Window Mode (Tools -> Options ->
Advanced -> Force links that open new windows to open in a new tab).

Normally nsDocShell::FindTarget fails to find a target (likely its name =
"_blank") and calls nsGlobalWindow::Open. Open stops the popup window. In Single
Window mode FindTarget fails to find the target and bypasses
nsGlobalWindow::Open to open a new tab directly.

Comment 1

[Dan M]

Attachment: testcase

Note this technique is also used by obnoxious Flash objects.

Comment 2

[Johnny Stenback (:jst)]

Looks like that's because when opening a link into a new tab the dom code that
blocks popups is entirely bypassed. We need the docshell, or the tabbrowser code
to check if it's ok to open new windows in this case too
(nsPIDOMWindow::GetPopupControlState() will give you the current popup control
state, and so on).

Comment 3

[Dan M]

Attachment: abort tab opening if window is a disallowed popup

I need a little more than the popup state: I also need to know whether the site
is whitelisted etc. This patch exposes in nsPIDOMWindow a new method to
determine whether the window should be stopped, and uses it in
nsDocShell::FindTarget.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 161999 [details] [diff] [review]
abort tab opening if window is a disallowed popup

r=jst

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 161999 [details] [diff] [review]
abort tab opening if window is a disallowed popup

sr=jst too!

Comment 6

[Brendan Eich [:brendan]]

Gotta fix regressions.

/be

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 161999 [details] [diff] [review]
abort tab opening if window is a disallowed popup

a=[e-mail] for branches -- this will make it into the 1.7 branch
too, right?

/be

Comment 8

[Dan M]

*** Bug 264587 has been marked as a duplicate of this bug. ***

Comment 9

[Omar Khan]

Attachment: Testcase, also uses synthetic events, but differently

Not sure if this is the right place,
but rather than create a duplicate bug report
here goes, also not sure if the proposed
patch fixes these issues

Jump to Tab 1 - type number, goes to tab number
uses control + number
NewWindow - create new window
uses shift + click
NewTabForeground - create new tab
uses control + shift + click
NewTabBackground - create new tab
uses control + click
CaretBrowsing - ON - prompts user to turn on caret browsing
F7

Comment 10

[Johnny Stenback (:jst)]

No, this patch doesn't fix any of those issues, but as the testcase is written,
only the last link shows a problem, the first ones are supposed to work since
they're all triggerd from a click on the link, which enables popups, per design.

Try the same testcase but do all that from the onload handler w/o user
interaction, and if any of those cases break, report back.

Please file a separate bug on the caret browsing problem (and cc me, please).

Comment 11

[Omar Khan]

Attachment: Testcase - uses onload event handler

Open and watch.

Comment 12

[Johnny Stenback (:jst)]

(In reply to comment #11)
> Created an attachment (id=163065)
> Testcase - uses onload event handler
> 
> Open and watch.

I opened, and saw nothing (today's nightly). Did I miss something?

Comment 13

[Omar Khan]

Re Comment #12
In my old build
Mozilla/5.0 (Windows; U; Win 9x 4.90; rv:1.7.3) Gecko/20041001 Firefox/0.10.1
it open a windows and two tabs onload,
I guess they patched it up,
have a good day.

Comment 14

[Omar Khan]

(In reply to comment #12)
> I opened, and saw nothing (today's nightly). Did I miss something?

I just got the latest nightly and I did not
see anything.  So my test case is 
invalid.

Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.7.3) Gecko/20041024 Firefox/1.0