Closed
Bug 819475
Opened 12 years ago
Closed 7 years ago
Should we stop returning null from document.domain?
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: qdot)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file)
Spec seems to have this not nullable. We return null for various non-hierarchical things... What do other UAs do?
Comment 1•12 years ago
|
||
FWIW, data:text/html,<script>alert(document.domain=="")</script> gives me true in Opera/Chrome/Safari. I remember it was the same for XMLHttpRequest.responseXML.domain when I tested that.
![]() |
Reporter | |
Comment 2•12 years ago
|
||
Hm. So in Gecko, at this point, a document with a null principal ends up with a nsNullPrincipalURI for the domainURI, which claims to have a host of "".
If I could produce a principal whose URI is an nsSimpleURI, we could get a null here. That means a non-hierarchical URI that we render but which does not inherit the principal from parent.
Looks like something like about:buildconfig or about:neterror (try putting in "http://255.255.255.255/" in the url bar) and such are the main cases where we get null.
I suppose it's not a problem to return "" in the currently-null cases...
Comment 3•11 years ago
|
||
See also bug 417021.
Comment 4•10 years ago
|
||
The test from comment 2 returns true. Not sure if it always did, the code seems to suggest that it did...
So what remains here, changing IDL?
![]() |
Reporter | |
Comment 5•10 years ago
|
||
> The test from comment 2 returns true. Not sure if it always did
It certainly has for a while.
Again, the main cases in which null is returned are not really web-accessible.
We could change the idl (and our impl).
Assignee | ||
Comment 6•7 years ago
|
||
Was looking at fixing this up as part of the document.domain work I'm doing about bug 1459671. It doesn't look like changing from DOMString? to DOMString should be too hard, but it also looks like we have domain in HTMLDocument instead of Document, which means we're failing a WPT (testing/web-platform/tests/html/browsers/origin/relaxing-the-same-origin-restriction/document_domain.html).
I could get that moved as part of this and kill 2 bugs with one stone, but as this is fairly core functionality for a bunch of things, are there any cavaets for this move, or is it just one of those "needed someone to do it" things? The domain handling code is fairly self contained and easy to move, but getting up into how nsIDocument spreads through other document types gets a bit confusing, so I want to make sure I'm not missing anything.
Assignee: nobody → kyle
Flags: needinfo?(bzbarsky)
![]() |
Reporter | |
Comment 7•7 years ago
|
||
There are a bunch of things we still need to move from HTMLDocument to Document. See bug 897815 and the things it tracks.
I would rather we kept the two behavior changes here (not returning null and moving to Document) separate. That said, I don't think there's a problem with it other than "no one has done it yet" and I would be happy to review a fix to that effect.
Flags: needinfo?(bzbarsky)
Comment hidden (mozreview-request) |
![]() |
Reporter | |
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8984317 [details]
Bug 819475 - Make document.domain non-nullable;
https://reviewboard.mozilla.org/r/250128/#review256460
::: dom/html/nsHTMLDocument.cpp:893
(Diff revision 1)
> nsHTMLDocument::GetDomain(nsAString& aDomain)
> {
> nsCOMPtr<nsIURI> uri = GetDomainURI();
>
> if (!uri) {
> - SetDOMStringToNull(aDomain);
> + aDomain.SetLength(0);
.Truncate(), please.
::: dom/html/nsHTMLDocument.cpp:903
(Diff revision 1)
> nsresult rv = nsContentUtils::GetHostOrIPv6WithBrackets(uri, hostName);
> if (NS_SUCCEEDED(rv)) {
> CopyUTF8toUTF16(hostName, aDomain);
> } else {
> // If we can't get the host from the URI (e.g. about:, javascript:,
> - // etc), just return an null string.
> + // etc), just set length to zero.
"just return an empty string"
::: dom/html/nsHTMLDocument.cpp:904
(Diff revision 1)
> if (NS_SUCCEEDED(rv)) {
> CopyUTF8toUTF16(hostName, aDomain);
> } else {
> // If we can't get the host from the URI (e.g. about:, javascript:,
> - // etc), just return an null string.
> - SetDOMStringToNull(aDomain);
> + // etc), just set length to zero.
> + aDomain.SetLength(0);
Again, .Truncate()
Attachment #8984317 -
Flags: review?(bzbarsky) → review+
Comment hidden (mozreview-request) |
Comment 11•7 years ago
|
||
Pushed by kmachulis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5edda305eaf0
Make document.domain non-nullable; r=bz
Comment 12•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•7 years ago
|
Keywords: dev-doc-needed
Comment 13•7 years ago
|
||
Documentation added.
Updated description at ref page:
https://developer.mozilla.org/en-US/docs/Web/API/Document/domain
Updated browser compat data with a note:
https://github.com/mdn/browser-compat-data/pull/2506
Add note to Fx62 rel notes:
https://developer.mozilla.org/en-US/Firefox/Releases/62#DOM
Let me know if this looks sufficient. Many thanks!
Flags: needinfo?(kyle)
Keywords: dev-doc-needed → dev-doc-complete
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•