174266 - links opening blank new window but content showing in current window if browser.block.target_new_window

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jeremy M. Dolan]

http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/gnapster/gnapster/ChangeLog

Click the link at the top, "1.7"

http://themes.freshmeat.net/browse/920/?topic_id=920

Click the first screenshot.

Both cases, a blank new window is opened, but the screenshot or changelog is
loaded in the old window.

Linux 2002101305 but this is a very old bug. See it at many different sites.

May be affected by these in my user.js:

user_pref("browser.block.target_new_window", true);
user_pref("dom.disable_open_click_delay", 1000);
user_pref("capability.policy.default.Window.onunload", "noAccess");

Comment 1

[Boris Zbarsky [:bzbarsky]]

> user_pref("browser.block.target_new_window", true);

Does removing that affect the behavior?  Because that link on the sourceforge
page does:

<a href="/cgi-bin/viewcvs.cgi/*checkout*/gnapster/gnapster/ChangeLog?rev=1.7"
target="cvs_checkout"
onClick="window.open('about:blank', 'cvs_checkout',
'resizeable=1,scrollbars=1')"
>

We execute the JS but we disallow the retargeting because of that pref, I bet...
(and that is such a _stupid_ way to write a link!)

Comment 2

[Jeremy M. Dolan]

Another major site where this is happening. Survey at:

http://www.popsci.com/popsci/aviation/article/0,12543,365576-4,00.html

Rather different code though:

<form method="post" action="http://www.pathfinder.com/gdml.dyn" target="_blank"
onsubmit="openTarget (this, 'width=350,height=600,resizable=1,scrollbars=1');
return true;">

Comment 3

[Jeremy M. Dolan]

Netscape webmail used to do this all the time too when you clicked on
Preferances, but it seems OK now. Should this bug be moved to evang?

Comment 4

[Bertrand Denoix]

Same problem on laposte.net (French Post Office webmail) using last night's build.

This happens even with everything allowed in "Scripts and Plugins".

As far as I can tell the bug appeared with 1.2a or 1.2b (but I skipped
everything between 1.0rc2 and 1.2a).

Comment 5

[Bertrand Denoix]

Gone fishing some previous versions... for me, bug is absent in 1.1 (Aug 26),
and shows up in 1.2a (Sept 11), if this can help.

Comment 6

[Torben]

Related/dependes on bug 64560? Since this seems to be caused by window.open I am
sending this to DOM Level 0 (where bug 64560 also lives). This happens on win
too (2002110708), changing OS to All.

Comment 7

[Torben]

...and on macs, HW->All

Comment 8

[haferfrost]

I've encountered this bug only with user_pref("browser.block.target_new_window",
true);
With this pref set to false, the bug disappears, i.e. instead of opening an
empty window it opens a window that contains the new page.
I have neither user_pref("dom.disable_open_click_delay", 1000);
nor
user_pref("capability.policy.default.Window.onunload", "noAccess");
(about:config doesn't even list them. Do they exist in 1.3?)

Comment 9

[Johnny Stenback (:jst)]

Mass-reassigning bugs to [e-mail]

Comment 10

[Torben]

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

Comment 11

[logan]

Attachment: simple testcase

this uses freshmeat's target=screenshot, window.open('', 'screenshot', ...)
bit, that should open mozilla.org in the popup.

Comment 12

[Gérard Talbot]

I tried the testcase: clicking on the link loads mozilla.org into the popup.
WORKSFORME
Mozilla 1.8a2 build 2004061308 and Firefox 0.9 build 20040614 rv:1.7 under XP
Pro SP1a here.

Comment 13

[Gérard Talbot]

I also tried the testcase at
http://www.kaosol.net/~ziz/formtarget.html (see bug 206831)
and it WORKSFORME with Moz 1.8a2.

Comment 14

[Michael Gabriel]

WFM 1.7 final win2k

Comment 15

[logan]

Nothing has changed here... My testcase and the test in comment 13 still open
the content in the main browser window rather than the popup.

Changing the browser.block.target_new_window pref (back to false) in
about:config did nothing for me, I had to restart for things to work as they should.

Comment 16

[Gérard Talbot]

Resolving as WORKSFORME then

Comment 17

[Jeremy M. Dolan]

It's either invalid, or evang, but not WFM. Behavior is still the same.

This bug still makes surfing with browser.block.target_new_window rather
unpleasant, which is the opposite of its intended effect. :(

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 152987 [details] [diff] [review]
Proposed patch

Danm, could you review?  The short of it is that the second arg to
FindItemWithName is used internally by that method to prevent infinite
recursion.  If it's set, that call to the method will NOT ask the window
watcher for targets, hence will not find targets in different windows.	So
toplevel entry points into FindItemWithName should really pass in null for the
second arg...

Comment 20

[Dan M]

Comment on attachment 152987 [details] [diff] [review]
Proposed patch

As you said, the point of this argument is to prevent a recursive search from
re-entering places it's already been. Beginning a fresh search with anything
but null for that parameter looks like an error to me.

Now about the hackish nature of this whole section of code...

Comment 21

[Boris Zbarsky [:bzbarsky]]

Yeah, we really need to combine this with the windowwatcher OpenWindowJS code
better...

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 152987 [details] [diff] [review]
Proposed patch

jst, would you sr?

Comment 23

[Johnny Stenback (:jst)]

Comment on attachment 152987 [details] [diff] [review]
Proposed patch

sr=jst

Comment 24

[logan]

I don't know about anyone else, but I'd sure like to have this in Firefox 1.0
(hint, hint)... ;)

Thanks for all your work guys.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 152987 [details] [diff] [review]
Proposed patch

I feel that this simple fix is worth taking on branch (and once again I'll need
someone for the aviary checkin....)

Comment 27

[Mike Kaply [:mkaply]]

Comment on attachment 152987 [details] [diff] [review]
Proposed patch

a=mkaply for 1.7.2

Comment 28

[Boris Zbarsky [:bzbarsky]]

Fixed on branch.

Comment 29

[Jeremy M. Dolan]

This still needs aviary landing.

Comment 30

[Boris Zbarsky [:bzbarsky]]

No, it needs aviary _approval_.  Then someone with an aviary tree can land it.

Comment 31

[Asa Dotzler [:asa]]

Comment on attachment 152987 [details] [diff] [review]
Proposed patch

a=asa (on behalf of aviary drivers) for checkin to the aviary branch.

Comment 32

[Boris Zbarsky [:bzbarsky]]

Checked in on aviary by rjkeller.

Comment 33

[Jeremy M. Dolan]

V w/ Gecko/20040804 Firefox/0.9.1+. Nice fix! Nothing beats receiving e-mails
with the strings "X-Bugzilla-Reason: Reporter" and "Keywords Added fixed-aviary1.0".