Closed
Bug 132105
Opened 22 years ago
Closed 22 years ago
xlink with actuate="onLoad" and show="new" will open a new window with the "Open unrequested windows" checkbox cleared
Categories
(Core :: XML, defect, P2)
Core
XML
Tracking
()
VERIFIED
FIXED
mozilla1.1alpha
People
(Reporter: gary, Assigned: hjtoi-bugzilla)
References
()
Details
Attachments
(1 file, 1 obsolete file)
7.08 KB,
patch
|
harishd
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•22 years ago
|
||
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
Comment 2•22 years ago
|
||
Yeah... The pref should apply to all methods of opening a window onload.
OS: Windows NT → All
Hardware: PC → All
Assignee | ||
Comment 3•22 years ago
|
||
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
Assignee | ||
Comment 4•22 years ago
|
||
Reporter | ||
Comment 5•22 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•22 years ago
|
QA Contact: petersen → rakeshmishra
Assignee | ||
Comment 6•22 years ago
|
||
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 7•22 years ago
|
||
Comment on attachment 85843 [details] [diff] [review] check pref before opening links in new windows sr=jst
Attachment #85843 -
Flags: superreview+
Comment on attachment 85843 [details] [diff] [review] check pref before opening links in new windows r=harishd
Attachment #85843 -
Flags: review+
Reporter | ||
Comment 9•22 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.
Assignee | ||
Comment 10•22 years ago
|
||
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 11•22 years ago
|
||
Comment on attachment 86246 [details] [diff] [review] better fix sr=jst
Attachment #86246 -
Flags: superreview+
Comment 12•22 years ago
|
||
Comment on attachment 86246 [details] [diff] [review] better fix r=harishd
Attachment #86246 -
Flags: review+
Assignee | ||
Comment 13•22 years ago
|
||
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
QA Contact: rakeshmishra → petersen
Resolution: --- → FIXED
Target Milestone: mozilla1.0.1 → mozilla1.1alpha
Updated•22 years ago
|
QA Contact: petersen → rakeshmishra
Comment 14•22 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.
Description
•