I'd like to remove CheckLoadURI() and now that bug 324464 is fixed, you can get to principals in chrome JS. List of callers: /toolkit/content/contentAreaUtils.js, line 138 /toolkit/content/widgets/tabbrowser.xml, line 834 /toolkit/content/widgets/text.xml, line 321 /browser/base/content/browser.js, line 2447 /browser/base/content/browser.js, line 2885 /browser/base/content/browser.js, line 4765 /browser/base/content/browser.js, line 4813 /browser/base/content/browser.js, line 4827 /browser/components/bookmarks/src/nsBookmarksFeedHandler.cpp, line 910 /browser/components/places/src/nsBookmarksFeedHandler.cpp, line 841

Does this make bug 335334 branch-only?

Er... Consumers that have a string should use CheckLoadURIStr; those that have a URI object should use CheckLoadURIWithPrincipal(). At least until we can pass URI objects to webnav from chrome or something...

Do we need to have an additional bug raised to cover: /xpfe/global/resources/content/bindings/tabbrowser.xml or can that be included in here?

Either way. I got tired of filing bugs sometime last night, because bugzilla was so busted, so I stopped.

*** Bug 342484 has been marked as a duplicate of this bug. ***

-> Firefox:General, since this bug is filed mostly against firefox-specific code.

* I'm not sure what would be the replacement for the caller in text.xml. * bookmarks/places are not yet fixed.

Comment on attachment 246867 [details] [diff] [review] patch v1 >Index: browser/base/content/browser.js >@@ -2860,20 +2861,19 @@ var goButtonObserver = { >+ urlSecurityCheck(url, >+ gBrowser.contentDocument.nodePrincipal, >+ Ci.DISALLOW_INHERIT_PRINCIPAL); ---------------------------^ oops > function handleLinkClick(event, href, linkNode) > { >- var docURL = event.target.ownerDocument.location.href; I'd keep this and make it "doc" to avoid the longer "event.target.ownerDocument" elsewhere in the function. >Index: browser/base/content/nsContextMenu.js >- // Remember the URL of the document containing the node >- // for referrer header and for security checks. >- this.docURL = aNode.ownerDocument.location.href; Similarly, could keep this as just this.doc. >- urlSecurityCheck(frameURL, this.browser.currentURI.spec, >+ urlSecurityCheck(frameURL, this.browser.contentDocument.nodePrincipal, Hmm, this pattern is repeated a lot, I wonder if we should add a <browser>.currentPrincipal? >Index: browser/base/content/utilityOverlay.js >+/** >+ * openNewTabWith: opens a new tab with the given URL. >+ * >+ * @param aURL >+ * The URL to open (as a string). >+ * @param aDoucment typo >+ * @param aEvent >+ * The triggering event (for the purpose of determining whether to open >+ * in the background), or null nit: missing period >+ * @param aAllowThirdPartyFixup >+ * if true, then we allow the URL text to be sent to third party services nit: capitalize "if" r=me with those fixed/considered.

(In reply to comment #7) > * I'm not sure what would be the replacement for the caller in text.xml. <bz_away> so can you describe the behavior you want? <gavin> I want text links to refuse to load "unsafe" URIs using that method <bz_away> Do you basically want to only allow loading "public" protocols no matter where the binding lives? <bz_away> ok <bz_away> in that case <bz_away> var safePrincipal = Components.classes["@mozilla.org/nullprincipal;1"].createInstance(Components.interfaces.nsIPrincipal); <bz_away> secMan.checkLoadURIWithPrincipal(safePrincipal, uri, nsISSM.DISALLOW_INHERIT_PRINCIPAL);

Comment on attachment 255169 [details] [diff] [review] v1.1 >Index: toolkit/content/widgets/text.xml >+ Components.classes["mozilla.org/nullprincipal;1"] r=me if you add the forgotten "@" at the beginning here :) >+ .createInstance(Components.interfaces.nsIPrincipal);

And a |Components.utils.reportError(ex);| in the out try/catch wouldn't be a bad idea either, I had no idea why this was failing until I added it.

I assume the milestone change to final is just because there's no a3 milestone?

Yeah, thus the temporary whiteboard note.

mozilla/toolkit/content/Makefile.in 1.6 mozilla/toolkit/content/contentAreaUtils.js 1.86 mozilla/toolkit/content/tests/unit/test_contentAreaUtils.js 1.1 mozilla/toolkit/content/widgets/browser.xml 1.102 mozilla/toolkit/content/widgets/tabbrowser.xml 1.222 mozilla/toolkit/content/widgets/text.xml 1.34 mozilla/browser/base/content/browser.js 1.763 mozilla/browser/base/content/nsContextMenu.js 1.10 mozilla/browser/base/content/utilityOverlay.js 1.49

mozilla/toolkit/content/tests/Makefile.in 1.1

(In reply to comment #7) > Created an attachment (id=246867) [details] > patch v1 > * bookmarks/places are not yet fixed. Should there be another bug filed for those?

That's bug 342484.