See attached test case Works in all other browsers.

-> mstolz

Confirmed on Mozilla trunk builds linux 102708 RedHat 6.2 win32 102704 NT 4 mac 102708 Mac OS9

Changed description slightly, added dataloss keyword because a page is not loading!!!. While I do accept that I cannot judge the priority of this, I feel that it is a serious issue when I also look at http://bugzilla.mozilla.org/show_bug.cgi?id=57636 document.write() is a fundamental tool that is often merely used to code browser bug workarounds. With a bug like this, a web developer loses a basic tool in cases that are difficult enough already. This fundamental method works in all other current browsers that one can think of today and older versions, too. If one browser (and even only the first minor release) does not support this, then web authors are stuffed for the next few years because one of their basic tools is broken. The times are over where one could afford JavaScript error messages when experimenting with frames. Browsers since version 3 are sophisticated enough to support reliable event driven applications. Mozilla/Netscape must not start at a reliability level that is below Internet Explorer version 3. Mozilla is an awsome browser but this case shows an extreme disparity.

Mass changing QA to ckritzer.

Mass changing milestones to Moz0.9.1. Many of these bugs are dependent on the XPConnected DOM and its associated security UI changes.

This seems to work now, marking Fixed.

Marking VERIFIED FIXED on: -MacOS91 2001-05-21-15-trunk -Win98SE 2001-05-22-06-trunk -LinRH62 2001-05-22-05-trunk

It's back on: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.1) Gecko/20040707 It's a very bad bug.

This was broken in 1.4 as well: "Permission denied to get property HTMLDocument.write"

jst, any hope we can unregress this? /be

Not sure when or how this broke, but with todays code we don't seem to ever set the principal of the document that's as a result of document.open() + write() to anything useful, and in this case where the caller is a child frame whose src is a javascript: URI we end up with a principal generated from the javascript: URI, which is never right. This patch makes us set the principal of the callee to the principal of the caller, as this should work AFAICT.

Go caillon, I'll sr and approve the patch as soon as it gets your r+. /be

Comment on attachment 159941 [details] [diff] [review] Carry over the caller's principal in document.open(). >@@ -2106,18 +2106,26 @@ nsHTMLDocument::OpenCommon(nsIURI* aSour >+ // Grab a reference to the callers principal as it may not be >+ // reachable past the call to Reset(). >+ nsCOMPtr<nsIPrincipal> callingPrincipal; >+ >+ if (callingDoc) { >+ callingPrincipal = callingDoc->GetPrincipal(); >+ } >+ Why here? Isn't this essentially what the code starting on line 1902 is doing with securityInfo? Seems like both should happen together just for conceptual clarity, though I don't know if here or ~1900 is better. Otherwise looks like the right thing to do r=dveditz, but a double-check from caillon wouldn't hurt

Comment on attachment 159941 [details] [diff] [review] Carry over the caller's principal in document.open(). r=caillon if you move this into where the securityInfo is set earlier in the method. Also why do we restore conditionally? Shouldn't we always do that, like we do for the securityInfo?

This changes this code such that when calling document.open() we make the URI of the callee be that of the codebase in the callers principal, and not the direct URI of the callee. The two should be the same in all cases except when the caller's URI is a javascript: or data: URI, and in those cases we want the URI of the callee to be the callers codebase, not the javascript: or data: URI. Basically the same thing as my previous patch, except that it doesn't share the principal instances across documents, just creates new principals based on the principal of the caller in stead of the document URI of the caller.

Comment on attachment 160008 [details] [diff] [review] More consistent approach. jst: what if the principal has a cert (signed script)? We should copy that too. Maybe we need a Clone operation. If we fixed the mAnnotations bloat problem, why wouldn't we share a reference to the caller's principal here, though? If principals are immutable (as far as identity goes), that wins by saving memory and cycles. /be

I'd propose to take attachment 160008 [details] [diff] [review] for the branches and make us *always* share principals on the trunk. The reason being that the patch for always sharing potentially has a greater impact than if we don't share, since we currently never do in this case. Chris and Brendan, pelase review attachment 160008 [details] [diff] [review] and we'll get that in on the branches and I'll fix this for real on the trunk...

Comment on attachment 160008 [details] [diff] [review] More consistent approach. Fair enough. r=me, however please make GetSourceCodebaseURI() a void method since the only caller really doesn't care about an rv, just whether it gets an nsIURI, and it already makes that check. You'll need to move the NS_ENSURE_TRUE inside the null check block.

Comment on attachment 160008 [details] [diff] [review] More consistent approach. sr=brendan@mozilla.org. /be

(In reply to comment #20) > (From update of attachment 160008 [details] [diff] [review]) > Fair enough. r=me, however please make GetSourceCodebaseURI() a void method > since the only caller really doesn't care about an rv, just whether it gets an > nsIURI, and it already makes that check. You'll need to move the > NS_ENSURE_TRUE inside the null check block. Yeah, I agree, but given that this is a temporary branch-only change I'd rather keep the change small for the branches, and do it right for the trunk later on. Thanks for the reviews!

Comment on attachment 160008 [details] [diff] [review] More consistent approach. a=asa for branches checkin.

Fixed on the aviary and 1.7 brances.

Please don't forget the trunk patch. Or perhaps the blocking1.8a6? flag should be set.

Comment on attachment 161587 [details] [diff] [review] Trunk patch. r=me, sorry for letting this linger for so long. Looks good to me, let's get this in and tested on trunk.

Should the original window properties be accessible in new window object? See http://forums.mozillazine.org/viewtopic.php?p=1275061 Where it was brought up that IE can access the 'globals' after a document.write if you specify the global variable without the window reference. I attached a simple test case.

Will, that's bug 114461 -- it has nothing to do with this bug. /be

Comment on attachment 161587 [details] [diff] [review] Trunk patch. sr=me. /be

Fix checked in.

Added a test for this in bug 445004.