User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; ru; rv:1.9pre) Gecko/2008041213 Firefox/3.0pre Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; ru; rv:1.9pre) Gecko/2008041213 Firefox/3.0pre "View MathML source" menu item from the formula's context menu opens a blank window. Reproducible: Always Steps to Reproduce: 1. Open http://www.mozilla.org/projects/mathml/demo/basics.xhtml 2. Right click any formula and choose "View MathML source" from the menu Actual Results: A blank window opens, no MathML source there. Expected Results: A window with source should open. It works fine in http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2008/01/2008-01-25-04-trunk/firefox-3.0b3pre.en-US.linux-i686.tar.bz2 It shows a blank window starting from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2008/01/2008-01-26-04-trunk/firefox-3.0b3pre.en-US.linux-i686.tar.bz2 and up to the latest trunk.

The same happens on Firefox 3 Beta 5 on Windows XP, too.

Error: uncaught exception: [Exception... "Security error" code: "1000" nsresult: "0x805303e8 (NS_ERROR_DOM_SECURITY_ERR)" location: "<unknown>"] I don't know how the MathML source stuff works, but given that it's the same error message as the complaints about regressions starting in bug 397791 comment 37, and the same day, I guess it's tripping over the same thing. Drivers: yeah, I realize MathML viewsource isn't a blocker in and of itself, but shipping useless-UI like this probably is, and if it's not fixable at this point we should at least remove the menuitem until it works.

I was wavering around trying to decide when would be the right time to deploy the nuclear option, when I realized that if we get a patch to fix it, backing out the comment-out is simple, so there's no reason not to take out the useless-UI now, before it slips through the cracks.

This seems to fix it. Not sure why it's required.

Looking at the first hunk of attachment 298836 [details] [diff] [review], I can see why that patch broke this - it required the caller to have the same principal as the document when calling document.open(), which isn't the case here. I don't understand why my patch fixes it, though, since as far as I can tell there are still two different principals involved (the "content" document and the chrome code). I don't understand why the wrapper has an effect. jst/sicking can you explain?

Comment on attachment 315344 [details] [diff] [review] The nuclear option Let's hold off on this since we might be able to fix it.

Comment on attachment 315344 [details] [diff] [review] The nuclear option view source is a pretty important part of the web, so I don't think I like the nuclear option

jst/jonas: this is blocking now, can you help gavin out with his question in comment 5?

Yeah, view source is pretty important, and it still works. View the DOM created by source is pretty important, too, and it still works - view selection source works fine, whether what's selected is MathML or not. What doesn't work is viewing the DOM created by MathML source by right-clicking anywhere within the rendered MathML without having to select it. But, I'm all for the humor of saying that even though for quite a while in bug 324857 we weren't willing to block 1.9 on having MathML even render, we're now willing to block Firefox 3 even if the only remaining issue is that MathML authors trying to figure out what we're doing with their MathML have to select a formula in order to see the DOM source.

jst or blake really is the authority here, but here's how I understand it: When you use the XPCNativeWrapper (which is what you'll have before the patch) we never change what principal we make the call with, we simply make sure that you never call any content set functions but instead go directly to the browser implementation. When you call using the SafeJSObjectWrapper we will use the principal of the object to actually make the call, which is likely why it succeeds here. This was done to make sure that content-overridden functions got called with content level security contexts. I guess an interesting side effect is that even native functions will execute with content level access. Unfortunately I don't know if relying on this behavior is a good idea or not...

Yes, Jonas' explanation is right here. Whether we should rely on this or not is a good question. I don't see this ever changing, but if we want this code to be independent of whether that does change or not, this code could use evalInSandbox() to evaluate the code that does the document.open etc here in the context of the webpage. I don't know if I care strongly either way, but a mochitest that makes sure we don't break this again would be golden!

Yeah, I agree with jst, if we can get a test for this I think it's fine to check in the patch as is.

Comment on attachment 316280 [details] [diff] [review] patch with test r=mano

Comment on attachment 316280 [details] [diff] [review] patch with test a1.9=beltzner

mozilla/toolkit/components/viewsource/Makefile.in 1.3 mozilla/toolkit/components/viewsource/content/viewPartialSource.js 1.15 mozilla/toolkit/components/viewsource/test/Makefile.in 1.1 mozilla/toolkit/components/viewsource/test/test_428653.xul 1.1

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050804 Minefield/3.0pre.

Ugh. So all of this code was actually relying on a bug in document.write, and it breaks if I fix that (bug 465806).

I guess that was discussed earlier in this bug too. It's a good thing that we had a test for this, indeed. ;) So why are we using document.write instead of a data: URL, say? And if we are, can we do it indirectly either via evalInSandbox as suggested above?

er, "either via evalInSandbox as suggested above or via setTimeout on the target window or something".

(In reply to comment #19) > So why are we using document.write instead of a data: URL, say? I don't see any reason why a data: URL wouldn't work.

OK. I switched this to a data: URL in bug 465806 and updated the test to test that document.write throws a security error.

The mochitest fails with my current iteration of parserless about:blank (attachment 541670 [details] [diff] [review] [review]) applied. How do I need to prepare the generated about:blank to make sure the .open() call on it throws a security exception as expected? See also bug 601803 comment 37.

> How do I need to prepare the generated about:blank to make sure the .open() > call on it throws a security exception as expected? You need to give it the correct principal. What principal are you giving your about:blank right now? Note that this discussion is really pertinent to bug 465806, which is what made the security exceptions start happening.

For posterity: bz figured this out in IRC. If the docshell isn't of type chrome and the principal the initial about:blank is about to get is the system principal, it needs to get null instead.