###!!! ASSERTION: Non-chrome URI?: 'isChrome', file chrome/src/nsChromeRegistry.cpp, line 155

SetProtocol should probably be creating a new nsIURI or something instead of blindly dong SetScheme. SetScheme is a major footgun... URL seems to be the only code that exposes a URI that's been messed with this way to script. It needs to stop doing that. For example, it could SetScheme and then GetHref and NS_NewURI that string or something (that's what setting .protocol on <a> does, say).

Per the specification it should set the scheme (but not change the relative flag of the URL). But then in the specification there's not a per-scheme handler as there is in Gecko. It's not entirely clear to me what the correct solution is here.

This patch "fixes" this issue but it doesn't allow the changing of the scheme from data: to chrome:.

Comment on attachment 8432449 [details] [diff] [review] urlchrome.patch Forgot to qref?

Comment on attachment 8432479 [details] [diff] [review] urlchrome.patch Why do we need all those changes to non-URL code? And why are we doing hand-parsing of URLs like this anyway? What's wrong with just making URL do what <a> does right now: setting protocol does the SetScheme, but then round-trips through a string?

This patch throws if the protocol cannot be set but we can do no-op as many other methods of URL API. bz, what do you suggest?

Comment on attachment 8434205 [details] [diff] [review] urlchrome.patch I think you should talk to Anne about what behavior the spec has here. The spec does do scheme-specific parsing, so it should at least think about what happens when protocol is switched to/from the list of things that are parsed specially.

I have no strong opinions on what is best here. Currently you can just change scheme and then things will fail once you do a fetch or some such.

Comment on attachment 8434205 [details] [diff] [review] urlchrome.patch I think we should no-op on failure, as elsewhere.

Comment on attachment 8435564 [details] [diff] [review] urlchrome.patch >+ // if we change it to not reserialize we need to do something smarter in s/it/this code/ >+ nsresult rv = clone->SetScheme(NS_ConvertUTF16toUTF8(Substring(start, iter))); Please null-check clone first. Also, add a comment here explaining why we do the serialize/reparse, please. >+ nsCOMPtr<nsIIOService> ioService(do_GetService(NS_IOSERVICE_CONTRACTID, &rv)); NS_NewURI is your friend. r=me with those fixed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec32b9724221 https://tbpl.mozilla.org/?tree=Try&rev=3236ebc37b71

Reproduced in Nightly 2014-05-30-mozilla-central-debug. Verified fixed FF 33.0a1 2014-06-13-mozilla-central-debug Win 7 x64.