1361975 - window.location is not nullable (but is null in Gecko when the browsing context is discarded)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Anne (:annevk)]

Test PR: https://github.com/w3c/web-platform-tests/pull/5778.

HTML: https://github.com/whatwg/html/issues/2545.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: location.patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

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.

Comment 3

[Andrea Marchesini [:baku]]

Attachment: location.patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

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...

Comment 5

[Pulsebot]

Pushed by [e-mail]:
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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/79890abc6ff5
https://hg.mozilla.org/mozilla-central/rev/48282ae47cf7
https://hg.mozilla.org/mozilla-central/rev/dc4b235f7946