xlink with actuate="onLoad" and show="new" will open a new window with the "Open unrequested windows" checkbox cleared

VERIFIED FIXED in mozilla1.1alpha

Status

()

P2
normal
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: gary, Assigned: hjtoi-bugzilla)

Tracking

Trunk
mozilla1.1alpha
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

17 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.9+)
Gecko/20020319
BuildID:    2002031903

The "Open Unrequested Windows" setting in "Scripts & Windows" is supposed to
restrict new windows from being opened when a page is visited. But the
actuate="onLoad" feature of xlink doesn't appear to check the capability to open
new windows before opening them.

Reproducible: Always
Steps to Reproduce:
1. Turn off "Open unrequested windows" in the "Scripts & Windows" options.
2. Go to the reference URL.


Actual Results:  An extra new window will open up, which should be stopped by
checking the capability to open new unrequested windows.

Expected Results:  Mozilla should not open a new window in response to an onLoad
event.
It isn't strictly a script that is opening the window, but maybe checking that
pref would make sense.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Yeah... The pref should apply to all methods of opening a window onload.
OS: Windows NT → All
Hardware: PC → All
Hmm, is the issue a) opening links in new windows or b) automatically following
links? I have a patch that makes XLinks check "browser.block.target_new_window"
pref before opening in new window. Is that sufficient? I'll attach the patch.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.0.1
Created attachment 85843 [details] [diff] [review]
check pref before opening links in new windows
(Reporter)

Comment 5

17 years ago
My concern is mainly that websites could use the xlink feature to open popup and
popunder ad windows, despite having dom.disable_open_during_load set to true.

What would be closer to what I'd imagine to be the correct behaviour is to block
if either browser.block.target_new_window OR dom.disable_open_during_load is set
to PR_TRUE.

For the first pref, not blocking will cause the first page to be replaced by the
second, probably not the desired behaviour - since the second page is
automatically loaded and might usually be expected to be the adjunct, not
primary content?

And for the second pref, the code in question in your patch is concerned with
when xlink:actuate="onLoad", so it should not open any windows during onLoad.

So 
inline PRBool BlockNewWindow()
{
  PRBool blockNewWindow = PR_FALSE;
  PRBool blockNewWindowOnLoad = PR_FALSE;
  nsCOMPtr<nsIPref> prefs(do_GetService(kPrefServiceCID));
  if (prefs) {
    prefs->GetBoolPref("browser.block.target_new_window", &blockNewWindow);
    prefs->GetBoolPref("dom.disable_open_during_load", &blockNewWindowOnLoad);
  }
  return (blockNewWindow == PR_TRUE || blockNewWindowOnLoad == PR_TRUE);
}

I don't have a CVS tree and I haven't written C in ages, so that may be wrong,
but you get the idea :-)

Updated

16 years ago
QA Contact: petersen → rakeshmishra
I checked what happens if "browser.block.target_new_window" is true and you
click on regular HTML a link: we replace the content. Therefore I think we
should just replace.

Also, please compare automatically loading XLinks that replace the content to
HTML refresh mechanism with 0 timeout. You can do that with HTTP headers or meta
elements:

<meta HTTP-EQUIV="refresh" CONTENT="0;URL=<link here>">

The preference "dom.disable_open_during_load" does not affect that, so I don't
think automatic XLinks that replace content should be affected with that.
Comment on attachment 85843 [details] [diff] [review]
check pref before opening links in new windows

sr=jst
Attachment #85843 - Flags: superreview+

Comment 8

16 years ago
Comment on attachment 85843 [details] [diff] [review]
check pref before opening links in new windows

r=harishd
Attachment #85843 - Flags: review+
(Reporter)

Comment 9

16 years ago
Far be it from me to stop a patch going in which improves the situation (i.e.
please check it in), but I will just say that semantically, I feel that the pref
being used usually blocks new windows from opening during interactive webpage
use, while the other pref usually blocks new windows from opening at load time.

Obviously, this is not the real meaning of either - I personally feel that both
prefs should be amalgamated as a new pref with values of ('never', 'onload',
'always') for when windows should be opened, and all methods where a window
could be opened should refer to that, rather than having two separate prefs for
different methods.

But then this should be a different bug, or else discussed on n.p.m.general I guess.
Created attachment 86246 [details] [diff] [review]
better fix

Ok, I think this should satisfy everyone. If "dom.disable_open_during_load" is
true, we don't open links in new windows when loading the document (but they
replace the current content instead, see reasons for this in earlier comments).
If "browser.block.target_new_window" is true, we *never* open links in new
window; instead they replace the content (again, see discussion before for
reasons).
Attachment #85843 - Attachment is obsolete: true
Comment on attachment 86246 [details] [diff] [review]
better fix

sr=jst
Attachment #86246 - Flags: superreview+

Comment 12

16 years ago
Comment on attachment 86246 [details] [diff] [review]
better fix

r=harishd
Attachment #86246 - Flags: review+
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
QA Contact: rakeshmishra → petersen
Resolution: --- → FIXED
Target Milestone: mozilla1.0.1 → mozilla1.1alpha

Updated

16 years ago
QA Contact: petersen → rakeshmishra

Comment 14

16 years ago
verified fixed on mozilla trunk 2002-12-19-08-trunk on win2K with the test case
at the URL http://www.snee.com/xml/xlink/sxlinkdemo.xml
marking verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.