User-Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 1.1.4322) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9) Gecko/2008052906 Firefox/3.0 When generating frame content, a media source does not open in the generated frame. I tested this with QuickTime (*.mov media) and WMP (*.rmi media). I believe the problem affects all media embedded locally. Reproducible: Always Steps to Reproduce: 1. Open local example file: midifram.htm 2. In top frame click: Generate 3. The lower frame generates, but plugin isn't showing up Actual Results: Html code loads, but the plugin appears not to be loaded. Expected Results: The plugin should play the midi file. For me it becomes serious with offline pages as I currently have them, so I'll label the severity as major.

I wondered if this was a local security restriction and you could somehow turn off the setting, but I think it's bug.

I just noticed (testing it again) that the behavior isn't always reproduced. Sometimes the plugin plays. Sometimes it doesn't. Why? Here is an earlier example I had which always breaks the plugin embedding. You open both frames in a new window first. In the new window the generated plugin never plays. Steps: 1. Open file new.htm in folder root 2. In top frame click: Generate 3. plugin isn't loaded

I only hear music in Opera 9.5.

The object tag is mozilla compatible, in this case, and you require the Windows Media Player Firefox Plugin (npnul32.dll), or the older version (npdsplay.dll) to play pre1bach.mid. I omitted the internet explorer alternate object tag from the generated code, because it isn't needed here. You should not just hear music, you should see the plugin on the page. But neither works. I've also tested with QuickTime and it does not embed the plugin using the attachment (id=329316) example. http:// protocol or FF2 works without a hitch, as it should. Older Opera is not very good with plugins from what I hear, newer Opera I think it is compatible with the WMP plugin.

FYI, npnul32.dll isn't at all related to WMP...

np-mswmp.dll Oops.

The issue seems to be related to file origin. Perhaps if the media file is in another folder the plugin can't open? I know in beta releases there were problems with local scripting across frames in different folders. Those could be resolved by setting pref("security.fileuri.strict_origin_policy", true); in grepref/all.js to false, reverting to the traditional same-origin policy mode. That doesn't fix this problem.

Hmm... I don't have a Windows setup yet (working on it). If someone who does can reproduce, can you find a regression range?

I can only narrow it down to after Gecko/2008012604 Minefield/3.0b3pre and before Gecko/2008032620 Firefox/3.0b5 (so far - working on it too.) Where I can download builds in between?

Tom, thanks for working on this! You should be able to get nightly builds in that range at <http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2008/>. Then pick the month (01, 02, 03 in this case), then a day.

It's the least I could do. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032106 Minefield/3.0b5pre Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032204 Minefield/3.0b5pre (and before) test negative. --------------------------- Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032304 Minefield/3.0b5pre Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032405 Minefield/3.0b5pre (and later) test positive. ---------------------------

Arg. This is in fact a regression from bug 402983, and sorta my fault. In bug 402983 comment 10 I said: Could we perhaps do something where a subframe that has a file:// principal also stores the principal of its parent frame, or better yet of whoever loaded it. If the latter is also a file:// principal, it's used for same-origin checks. In other words, the principals for a frame and its parent test equal, but still have different URIs (so that base URI stuff works right). but what we ended up implementing (and I should have caught this in review) just uses the parent frame principal. Which means that base URIs do NOT work right anymore, as in this case. In other words, since we set the base URI of the JS-generated frame based on the principal of the JS, and since we now inherit file principals, we end up with the wrong base URI (that of the page that opened the frameset, not of the frame in question). We really need to fix this.

Looks like bug 442244 also talks about this issue. Setting dependency.

Not blocking since there's no patch and such. Boris, should you own this?

Boris, marking this as a P3 blocker for you, meaning it doesn't need a beta to bake in.

Why are base URIs affected by principal? They should be orthogonal, no?

Because we're generating a page via document.write, so we set its base URI to the principal URI of the caller.

Couldn't we set it to the baseURI of the caller? That seems more correct.

We don't have that. We just have the principal, gotten off the JS stack.

However we fix this it sounds scary enough that it needs beta baking, so setting to P2. Couldn't we grab the calling context instead of the calling principal? And get the document out of that?

Fundamentally, we use the principal URI as base URI in various places, not just document.write. So we just need to make sure it's correct...

I guess we can't change nsIPrincipal on 1.9.0, right? So I need to work around that somehow?

> Couldn't we grab the calling context instead of the calling principal? Oh, and as far as I undertand, no. If code in window A calls code in window B that does a document.write in window C, then as far as I can see all the JS is running on the JSContext of window A, and that's all we can get as far as contexts go... Maybe I'm wrong, of course.

OK, so I was wrong in comment 21. The only place where we use the principal for the base URI is right here, in OpenCommon. So then I did a bit of digging into the behavior here in other browsers and the history of our behavior. The basic testcase is two windows, A and B, with different base URIs. Window B has functions that can either set location or document.write into subframes of window B. It either calls those itself, or A calls them. I'll roll such a test into the mochitests I write for this bug. IE 7's behavior seems to be the following in a simple case with two static documents (so no document.write or javascript: magic): B sets location.href directly: document URI of B is used to resolve relative URI B calls document.write directly: target gets B's document URI as URI and base URI A sets location.href via B: document URI of A is used to resolve relative URI A calls document.write via B: target gets document URI of A as URI and base URI Opera 9.6 on Mac behavior is slightly different (saner, too): B sets location.href directly: base URI of B is used to resolve relative URI B calls document.write directly: target gets B's document URI as URI and B's base URI as base URI. A sets location.href via B: base URI of A is used to resolve relative URI A calls document.write via B: target gets document URI of A as URI and base URI of A as base URI Safari 3.1.2 and a Webkit nightly match Opera except in the "A calls document.write via B" case, in which case the base URI and document URI of B are used, not those of A. Gecko matches Safari for document URIs as long as the principal URI for a document's NodePrincipal() is the document URI. In Gecko 1.8 this is the case for file:// while in Gecko 1.9 it's not. In Gecko 1.9 for file://, if A loaded B we end up using the document URI of A for everything. In Gecko, the base URI of a document.open-created document is always equal to its document URI. Revision history looks something like this. 1) We used to use GetDocumentFromCaller() and then use that document's documentURI for the URI of the newly-opened document. This broke cases where the caller was a javascript: document, since we don't give those the URI of whatever created them. In those cases the newly-opened document ended up not being same-origin with the caller, which was bad. 2) In bug 57600 we started giving the newly-opened document its caller document's principal and using the URI from the principal (or about:blank for system principals). 3) This caused security issues, because GetDocumentFromCaller() didn't really work that well. Or more precisely it didn't give us the principal we want. So in bug 296830 we switched to using GetSubjectPrincipal() to get the principal. We continued to use GetDocumentFromCaller() to get the securityInfo, though. 4) We fixed this in bug 298064 by switching GetDocumentFromCaller() to get the document via the JS callstack instead of just getting it via the JSContext/nsIScriptContext. 5) This broke the securityInfo handling in some cases, so in bug 312363 we effectively backed out bug 298064 as far as document.open is concerned (at least if I read this correctly; blake, can you confirm?). We needed the fixed GetDocumentFromCaller() in other places, so we switched to GetDocumentFromContext() (which did what GetDocumentFromCaller() used to do before bug 298064). A lot of this would have been simpler if we'd simply had tests people actually ran as we did all this. :(

I think we pretty much want Opera's behavior here, in general. It matches IE's when there are no base URIs around, and is much saner when there are. One way to do that would be to get the URI and base URI for the document.opened document from the document we get the security info from (the one returned by GetDocumentFromContext()). We presumably want to keep using the subject principal for the principal, so as not to regress bug 296830, though I'd like to double-check that. Unfortunately, this would almost certainly cause issues for documents created via javascript: which then call document.write(), because I'm pretty sure that we don't give those the same document URI or base URI that other browsers do. See bug 49312 and its dependencies, and especially bug 210862 comment 15.

Oh, for additional fun, we do weird stuff with location sets from documents loaded using javascript: (said weird stuff being added in bug 162128). Perhaps we should simply do the same for document.write as a quick fix if I can't think of a better way to really fix this.

(In reply to comment #24) > 5) This broke the securityInfo handling in some cases, so in bug 312363 we > effectively backed out bug 298064 as far as document.open is concerned (at > least if I read this correctly; blake, can you confirm?). We needed the > fixed GetDocumentFromCaller() in other places, so we switched to > GetDocumentFromContext() (which did what GetDocumentFromCaller() used to do > before bug 298064). Yeah, that's what happened there.

There are a few things going on here. I decided to not mess with the principals after all, and just fix document.write, based on comment 24. That's the nsHTMLDocument change, and it fixes this bug. The other changes in this bug are: 1) Tests for this bug, bug 298064, bug 57600. 2) Changes to javascript: to do base URIs better. In particular, newURI now stashes away the base URI passed to the newURI call, and we stick that in a property on the nsJSChannel. nsDocument::Reset reads the property. There's a bunch of code here because URIs have to support all sorts of interfaces, sadly, and I had to implement a new URI class for javascript:. 3) A minor change to nsDocumentViewer to fire onload if a cached image is loaded directly in the window. Right now we fire onload if the image is not cached, but not if it is, which screwed up the tests. I have manually verified that bug 296830 and bug 312363 did not regress with this change, but didn't add a test, because I can't reproduce those bugs even backing out the patches that fixed them. I do think we may want to switch from the subject principal to the GetDocumentFromContext() document principal, now that we enforce same-origin on document.write. But I'd rather do that in a separate bug; not sure I want to make changes like that on branch. ;) This patch incidentally fixes bug 49312 and its various dependencies. Christian, I'd love it if you take a look at the URI/channel stuff (which is sadly the bulk of the non-test changes).

Comment on attachment 347446 [details] [diff] [review] Proposed fix - In DocumentViewerImpl::LoadComplete(): + // that one as a success code; otherwise whether we file onload for the image s/file/fire/ r+sr=jst

Comment on attachment 347446 [details] [diff] [review] Proposed fix >+NS_IMETHODIMP >+nsJSURI::Read(nsIObjectInputStream* aStream) >+{ >+ nsresult rv; >+ >+ rv = aStream->ReadObject(PR_TRUE, getter_AddRefs(mSimpleURI)); >+ if (NS_FAILED(rv)) return rv; >+ >+ PRBool haveBase; >+ rv = aStream->ReadBoolean(&haveBase); >+ if (NS_FAILED(rv)) return rv; >+ >+ if (haveBase) { >+ rv = aStream->ReadObject(PR_TRUE, getter_AddRefs(mBaseURI)); >+ if (NS_FAILED(rv)) return rv; >+ } >+ >+ return NS_OK; >+} Shouldn't this be setting up mMutable too? >+// Use an extra base object to avoid having to manually retype all the >+// nsIURI methods. I wish we could just inherit from nsSimpleURI instead. >+class nsJSURI_base : public nsIURI, >+ public nsIMutable >+{ >+public: >+ nsJSURI_base(nsIURI* aSimpleURI) : >+ mSimpleURI(aSimpleURI) >+ { >+ mMutable = do_QueryInterface(mSimpleURI); >+ NS_ASSERTION(aSimpleURI && mMutable, "This isn't going to work out"); >+ } >+class nsJSURI : public nsJSURI_base, >+ public nsISerializable, >+ public nsIClassInfo >+{ >+ // For use only from deserialization >+ nsJSURI() : nsJSURI_base() {} Won't this trigger the assertion?

> Shouldn't this be setting up mMutable too? Yes. Will fix. > Won't this trigger the assertion? That's calling the no-arg base constructor, which doesn't assert.

Dave, see comment 31.

Comment on attachment 347446 [details] [diff] [review] Proposed fix a191b2=beltzner

landed test followup as http://hg.mozilla.org/mozilla-central/rev/cbd14041ff7d

Landed the main patch as http://hg.mozilla.org/mozilla-central/rev/b2fbc259ee2b Fixed, yay!

So as followups, I'd like to: 1) Remove nsLocation::FindUsableBaseURI 2) Change the principal stuff per comment 28. I'll try to get bugs on that (and tests for bug 49312 plus dependencies) dealt with tomorrow.

Filed bug 465804 on removing FindUsableBaseURI and bug 465806 on the document.open principal thing.

This includes both of the diffs in this bug, as well as the one in bug 465752. When checking in, I need to make sure to land the files diffed here plus the four binary ones (the images)....

I checked in the 4 images into CVS now, so the roll-up patch now accurately reflects what needs to land in 1.9.0.

The work you put into a 1.9.0 patch is much appreciated, but how much risk are we taking on and is a common-enough use case broken to be worth it?

Hmm. I think the risk is fairly low since we're mostly aligning with IE, and in cases where current interop is poor amongst browsers anyway. This basically only affects cases when document.write() is called in window A from window B and the two windows have different directories, plus the file:// case this was originally filed on. Not sure how common it is to use document.write() in file:// URIs, but any such use in a directory other than the one originally loaded in the browser is broken right now.

Comment on attachment 349196 [details] [diff] [review] roll-up 1.9 patch Boris: do you think it's worth taking this late in the 3.0.x cycle? This feels like a really weird edgecase that's fixed on trunk for those people actually hitting it, which likely isn't many...

Yes, I think it is. We've had several duplicates of this bug, and this bug breaks at least one HTML tutorial/reference application (see bug 471252). I should also note that it'll always be "late in the cycle" if a month goes between approval request and anyone looking at the bug. :( I'm happy to land this at the beginning of a branch cycle if the approvals actually... y'know... happen.

Comment on attachment 349196 [details] [diff] [review] roll-up 1.9 patch Re-setting approval request so I'll actually get mail if it happens.

Comment on attachment 349196 [details] [diff] [review] roll-up 1.9 patch Yeah, ok. Let's do this... Approved for 1.9.0.7. a=ss. Please land in CVS when the tree opens for 1.9.0.7 (I'll even send you an email reminder!).

Bug 473003 describes another help system that was broken by this bug, for what it's worth.

Fixed on 1.9.0 branch.

Verified in 1.9.0.7 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.7pre) Gecko/2009021704 GranParadiso/3.0.7pre. Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090213 Shiretoko/3.1b3pre.

I raised a bug 442244 which was marked as a duplicate of this one. Which is resolved. I have just observed my original issue 442244 in Firefox 3.0.10 on Fedora. It might be worth someone verifying that this issue is currently present in Firefox for Linux, and reactivating this one or creating a new issue if it is confirmed. I am not certain of the correct procedure - my excuse for putting this in a comment.

> I am not certain of the correct procedure - my excuse for putting this in a > comment. Fraser, correct procedure if this bug is not fixed as far as you can tell is to comment in this bug. ;) That said, I just tried Firefox 3.0.10 (installed via yum, so this is the distro build) on my FC9 system, and your testcase from bug 442244 works fine for me. If it also does for you but there's another testcase that shows the problem, could you please file a new bug with the new testcase and cc me? Alternately, if you're using some other Firefox 3.0.10 than the one yum installs, let me know which one it is so I can make sure to test the same thing that you're testing?

Boris - apologies. I messed up the install of Firefox 3.0.10. I re-installed through yum and the test case works correctly. I unfortunately don't use Linux often - so am a little clumsy with it. Thanks :)