Created attachment 8864458 [details] [diff] [review] location.patch
Comment on attachment 8864458 [details] [diff] [review] location.patch Won't this just crash if the first .location get happens after GetDocShell starts returning null? What we should probably do is just have the "location" getter pass a null docshell to the Location constructor... >+ mLocation->SetDocShell(nullptr); Why is this needed, both places? Seems to me like we should instead just remove Location::SetDocShell altogether. It's dead code before this patch.
Created attachment 8864813 [details] [diff] [review] location.patch
Comment on attachment 8864813 [details] [diff] [review] location.patch > Location::GetURI(nsIURI** aURI, bool aGetInnermostURI) So the behavior change here is to start returning NS_OK instead of an error result if Location has no docshell. Why is that a desirable behavior change? Might be good to explain that in the commit message. Yes, the commit message says the change is being made, but not why. My main problem with it is that it creates a thing that looks like an XPCOM getter but can in fact return null and NS_OK. This needs to at least be clearly documented in the header, for both GetURI and GetWritableURI. I did check that all callers are OK with it...
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/79890abc6ff5 window.location is not nullable, r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/48282ae47cf7 Fixing tests in order to support window.location not nullable, r=me
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/dc4b235f7946 Fixing tests in order to support window.location not nullable: fix eslint complaint. r=eslint-fix
https://hg.mozilla.org/mozilla-central/rev/79890abc6ff5 https://hg.mozilla.org/mozilla-central/rev/48282ae47cf7 https://hg.mozilla.org/mozilla-central/rev/dc4b235f7946