Closed
Bug 324601
Opened 19 years ago
Closed 19 years ago
[FIX]Documents created via DOMImplementation should have better principal
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
sicking
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
Right now they get a principal based on the URI of the document the DOMImplementation came from. They should probably just get the principal that document has at createDocument call time.
Assignee | ||
Comment 1•19 years ago
|
||
Note to self: should store principal, not URI, in global window too (the mOpenerScriptURL stuff).
Assignee | ||
Comment 2•19 years ago
|
||
Note to self -- fix NS_NewDOMDocument to take a principal or something.
Assignee | ||
Comment 3•19 years ago
|
||
Allan, does XForms actually depend on the fact that the document created via DOMImplementation has the baseURI and documentURI set to the documentURI of the document the DOMImplementation came from?
I'm thinking that we should change that, basically (and use about:blank for the URI instead).
Assignee | ||
Comment 4•19 years ago
|
||
I filed bug 325816 on the mOpenerScriptURL, since that depends on bug 323810 unless I want icky conflicts.
Assignee | ||
Comment 5•19 years ago
|
||
This changes the URI on blank documents to be about:blank and the principal to be the one of the document the DOMImplementation came from...
Attachment #210661 -
Flags: superreview?(jst)
Attachment #210661 -
Flags: review?(bugmail)
Assignee | ||
Comment 6•19 years ago
|
||
Assignee: general → bzbarsky
Attachment #210661 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #210676 -
Flags: superreview?(jst)
Attachment #210676 -
Flags: review?(bugmail)
Attachment #210661 -
Flags: superreview?(jst)
Attachment #210661 -
Flags: review?(bugmail)
Assignee | ||
Updated•19 years ago
|
Summary: Documents created via DOMImplementation should have better principal → [FIX]Documents created via DOMImplementation should have better principal
Target Milestone: --- → mozilla1.9alpha
Comment 7•19 years ago
|
||
(In reply to comment #3)
> Allan, does XForms actually depend on the fact that the document created via
> DOMImplementation has the baseURI and documentURI set to the documentURI of the
> document the DOMImplementation came from?
No, we should not need that.
Comment 8•19 years ago
|
||
(In reply to comment #6)
> Created an attachment (id=210676) [edit]
> Actually compiling
And actually fixing the problem for XForms :)
Assignee | ||
Comment 9•19 years ago
|
||
Attachment #210676 -
Attachment is obsolete: true
Attachment #211428 -
Flags: superreview?(jst)
Attachment #211428 -
Flags: review?(bugmail)
Attachment #210676 -
Flags: superreview?(jst)
Attachment #210676 -
Flags: review?(bugmail)
Assignee | ||
Comment 10•19 years ago
|
||
Attachment #211428 -
Attachment is obsolete: true
Attachment #211430 -
Flags: superreview?(jst)
Attachment #211430 -
Flags: review?(bugmail)
Attachment #211428 -
Flags: superreview?(jst)
Attachment #211428 -
Flags: review?(bugmail)
Attachment #211430 -
Flags: review?(bugmail) → review+
Comment on attachment 211430 [details] [diff] [review]
Fix that buglet for real
actually, i'm not sure if this is good. Shouldn't the principal be set along with the baseuri of the XMLHttpRequest.
If we can't get to the principal but we can get to the baseuri maybe we should create a new principal based on the baseuri assuming the baseuri is actually a DocumentURI() or similar.
Attachment #211430 -
Flags: review+ → review?(bugmail)
Though really, this seems like a case for a non-principal. For now maybe use an about:blank principal since it should be reset once loading starts, right?
Assignee | ||
Comment 13•19 years ago
|
||
> Shouldn't the principal be set along with the baseuri of the XMLHttpRequest.
Please read nsXMLHttpRequest::GetBaseURI?
> Though really, this seems like a case for a non-principal.
Possibly, but I'd like to fix this bug, since it's actually breaking functionality... I don't know when, or whether, I'll have time to work on non-principal, but I can file a followup bug on it.
Frankly, using the about:blank principal would just be more code than this, for little purpose. ;)
Comment on attachment 211430 [details] [diff] [review]
Fix that buglet for real
>Index: extensions/xmlextras/base/src/nsXMLHttpRequest.cpp
>===================================================================
>RCS file: /home/bzbarsky/mozilla/cvs-mirror/mozilla/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp,v
>retrieving revision 1.143
>diff -u -p -d -r1.143 nsXMLHttpRequest.cpp
>--- extensions/xmlextras/base/src/nsXMLHttpRequest.cpp 20 Jan 2006 21:33:24 -0000 1.143
>+++ extensions/xmlextras/base/src/nsXMLHttpRequest.cpp 10 Feb 2006 22:27:23 -0000
>@@ -1233,7 +1233,15 @@ nsXMLHttpRequest::OnStartRequest(nsIRequ
> nsCOMPtr<nsIPrivateDOMImplementation> privImpl =
> do_QueryInterface(implementation);
> if (privImpl) {
>- privImpl->Init(GetBaseURI());
>+ // XXXbz this is probably all wrong when not called from JS... and possibly
>+ // even then! Fixing that requires giving XMLHttpRequest some principals
>+ // when inited. Until then, cases when we don't actually parse the
>+ // document will give our mDocument he wrong principal. I'm just not sure
>+ // how wrong it can get... Shouldn't be too bad as long as mScriptContext
>+ // is sane, I guess.
>+ nsCOMPtr<nsIDocument> doc = GetDocumentFromScriptContext(mScriptContext);
>+ nsIURI* uri = GetBaseURI();
>+ privImpl->Init(uri, uri, doc->GetNodePrincipal());
> }
Should you call GetBaseURI() before using mScriptContext to make sure that it is set? Also, you should probably handle doc being null.
Attachment #211430 -
Flags: review?(bugmail) → review+
Assignee | ||
Comment 15•19 years ago
|
||
> Should you call GetBaseURI() before using mScriptContext to make sure that it
> is set?
"Not sure". I really don't trust this "set mScriptContext lazily whenever we happen to" business, hence the comments. I'd prefer to fail over with a null principal if it's not already set when we get here.
> Also, you should probably handle doc being null.
That I do. Will fix.
Comment 16•19 years ago
|
||
Comment on attachment 211430 [details] [diff] [review]
Fix that buglet for real
r+sr=jst
Attachment #211430 -
Flags: superreview?(jst) → superreview+
Assignee | ||
Comment 17•19 years ago
|
||
Fixed on trunk. I addressed the comment about doc being null from comment 14 in a separate checkin. :(
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
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
•