Closed
Bug 130337
Opened 22 years ago
Closed 22 years ago
browser.block.target_new_window messes up Get New Themes link
Categories
(Core :: Layout: Images, Video, and HTML Frames, defect)
Core
Layout: Images, Video, and HTML Frames
Tracking
()
RESOLVED
FIXED
mozilla1.2alpha
People
(Reporter: jonasj, Assigned: akkzilla)
References
Details
(Whiteboard: [Hixie-P0])
Attachments
(1 file)
1.45 KB,
patch
|
mozeditor
:
review+
kinmoz
:
superreview+
|
Details | Diff | Splinter Review |
Trunk build 2002-03-11-03, Win2000: If the browser.block.target_new_window pref is turned on, the Get New Themes link in Preferences|Appearance|Themes will open in the Preferences window instead of in a new window. To reproduce: * Open Preferences and go to Advanced|Scripts & Windows. * Disable "Open a link in a new window". * Click OK. * Open Preferences again, this time go to Appearance|Themes. * Click the "Get New Themes" link. Actual results: * A page from XULPlanet.com is loaded inside the Preferences dialog. Expected results: * The page is loaded in a new window. I guess the way to fix this will be to always allow target="anything" for chrome:// URLs. Boris: I hope you don't mind me assigning this to you -- you did such a nice, quick job in bug 126003, so it seems like a very natural thing to do. :) But feel free to give this away, of course.
Comment 1•22 years ago
|
||
Punting to Akkana. I have pretty much no time to work on Mozilla stuff till after 1.0....
Assignee: bzbarsky → akkana
Assignee | ||
Comment 2•22 years ago
|
||
I'd like to revisit this code anyway.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.1alpha
Updated•22 years ago
|
Whiteboard: [Hixie-P0]
Comment 3•22 years ago
|
||
*** Bug 149503 has been marked as a duplicate of this bug. ***
Comment 4•22 years ago
|
||
Blah! missed 1.1. This is /really/ screwy.
Target Milestone: mozilla1.1alpha → mozilla1.2alpha
Assignee | ||
Comment 5•22 years ago
|
||
Re-adding bz and adam as possible reviewers, and for their opinions on which schemes we should be checking.
Assignee | ||
Comment 6•22 years ago
|
||
Here's a fix. In addition to checking whether the new url will be chrome or resource, we also check whether the URI of the current docshell is chrome. Should we also check for resource: ? Can a docshell's URI be resource:, and for what sort of window would that occur? In the chrome case, we just ignore mDisallowPopupWindows and go ahead and pop up a new window. Personally I'd rather have it open the url in the last-accessed browser window, but I'm not sure how to get that (get it somehow from the nsIDocShellTreeItem weak ref mParent?) or whether the prefs dialog modality might cause that window to be unusable on nonunix platforms. So I took the easy way out. The normal case (click on a target=new link in a browser) still works -- the docshell's uri is the uri of the page, not of the browser chrome (whew). The patch also includes an initializer-ordering warning fix. After this, nsDocShell.cpp still generates one warning, but it looks like that might be gcc being overly picky as usual: nsDocShell.cpp: In method `nsresult nsDocShell::InternalLoad (nsIURI *, nsIURI *, nsISupports *, int, const PRUnichar *, nsIInputStream *, nsIInputStream *, unsigned int, nsISHEntry *, int, nsIDocShell **, nsIRequest **)': nsDocShell.cpp:4671: warning: enumeral mismatch in conditional expression: `nsIContentPolicy::{anonymous enum}' vs `nsIContentPolicy::{anonymous enum}'
Comment 7•22 years ago
|
||
Comment on attachment 95651 [details] [diff] [review] A fix r=jfrancis
Attachment #95651 -
Flags: review+
Assignee | ||
Comment 8•22 years ago
|
||
Kin, could I get an sr for this small fix? (About 5 lines.)
Comment on attachment 95651 [details] [diff] [review] A fix sr=kin@netscape.com
Attachment #95651 -
Flags: superreview+
Assignee | ||
Comment 10•22 years ago
|
||
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
Component: Layout: HTML Frames → Layout: Images
Product: Core Graveyard → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•