Closed
Bug 212218
Opened 18 years ago
Closed 10 years ago
Implement DOM Level 3 Document properties/methods
Categories
(Core :: DOM: Core & HTML, defect, P4)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: caillon, Assigned: caillon)
References
Details
(Keywords: meta)
Attachments
(2 obsolete files)
Assignee | ||
Comment 1•18 years ago
|
||
Still leaves a few things unimplemented for various reasons, either because they entail some more risky work (renameNode), because of potential security reasons, or just because. :-)
Assignee | ||
Updated•18 years ago
|
Attachment #127417 -
Flags: superreview?(bzbarsky)
Attachment #127417 -
Flags: review?(jst)
![]() |
||
Comment 2•18 years ago
|
||
Comment on attachment 127417 [details] [diff] [review] First pass patch >Index: dom/public/idl/core/Makefile.in > nsIDOM3Node.idl \ >+ nsIDOM3Document.idl \ > nsIDOMNSDocument.idl \ > nsIDOMXMLDocument.idl \ > nsIDOMUserDataHandler.idl \ >+ nsIDOMDOMConfiguration.idl \ What's with the weird spacing of the '\' at the end? Could you just line the up, please? >Index: dom/public/idl/core/nsIDOM3Document.idl >+interface nsIDOM3Document : nsISupports Should this be inheriting from nsIDOM3Node or something like that? I guess that's jst's call... Add a pointer to the relevant spec to the comments here, please. >Index: dom/public/idl/core/nsIDOMDOMConfiguration.idl Same here. >Index: content/base/src/nsDocument.cpp > nsDocument::Normalize() > if (node) { > return node->Normalize(); > } I'd think you want something more like: nsresult rv; if (node) { if (NS_FAILED(rv = node->Normalize())) { return rv; } } or something like that. >+nsDocument::GetXmlEncoding(nsAString& aXmlEncoding) >+{ >+ SetDOMStringToNull(aXmlEncoding); >+ >+ if (mXMLDeclarationBits & XML_DECLARATION_BITS_DECLARATION_EXISTS && >+ mXMLDeclarationBits & XML_DECLARATION_BITS_ENCODING_EXISTS) { >+ // XXX We don't store the encoding given in the xml declaration. >+ // For now, just output the actualEncoding which we do store. >+ GetActualEncoding(aXmlEncoding); >+ } How about calling SeDOMStringToNull in the else clause? >+nsDocument::GetXmlStandalone(PRBool *aXmlStandalone) >+{ >+ *aXmlStandalone = PR_FALSE; >+ >+ if (mXMLDeclarationBits & XML_DECLARATION_BITS_DECLARATION_EXISTS && >+ mXMLDeclarationBits & XML_DECLARATION_BITS_STANDALONE_EXISTS && >+ mXMLDeclarationBits & XML_DECLARATION_BITS_STANDALONE_YES) { >+ *aXmlStandalone = PR_TRUE; >+ } How about: *aXMLStandalone = mXMLDeclarationBits & XML_DECLARATION_BITS_DECLARATION_EXISTS && mXMLDeclarationBits & XML_DECLARATION_BITS_STANDALONE_EXISTS && mXMLDeclarationBits & XML_DECLARATION_BITS_STANDALONE_YES; return NS_OK; >+nsDocument::GetDocumentURI(nsAString& aDocumentURI) >+{ >+ SetDOMStringToNull(aDocumentURI); >+ >+ if (mDocumentURL) { >+ nsCAutoString uri; >+ mDocumentURL->GetSpec(uri); >+ CopyUTF8toUTF16(uri, aDocumentURI); >+ } Again, put the SetDOMStringToNull in the else clause of that if. >+nsDocument::AdoptNode(nsIDOMNode *source, nsIDOMNode **aReturn) >+{ >+ return NS_ERROR_NOT_IMPLEMENTED; >+} Please add a comment that there are security implications to adopting that would need to be considered, I think.... (and maybe a comment that this basically consists of giving the node a new nodeinfo).
![]() |
||
Comment 3•18 years ago
|
||
Comment on attachment 127417 [details] [diff] [review] First pass patch sr=me with those nits fixed.
Attachment #127417 -
Flags: superreview?(bzbarsky) → superreview+
Comment 4•18 years ago
|
||
Comment on attachment 127417 [details] [diff] [review] First pass patch In addition to what bz already said: - In nsDocument::SetXmlStandalone(): +{ + return NS_ERROR_NOT_IMPLEMENTED; +} Use two-space indentation here too, dude :-) - In nsDocument.h: @@ -227,16 +228,17 @@ class nsDocument : public nsIDocument, ... public nsIDOM3Node, + public nsIDOM3Document, I think, after considering this for some time, that we want to make nsIDOM3Document inherit nsIDOM3Node. Not that it matters for nsIDOM3Document, but it kinda does matter for nsIDOM3Element (once that comes along) where we'll hopefully be able to save a vtable slot on all DOM 3 tearoffs if nsIDOM3Element inherits nsIDOM3Node. So, for consistency's sake, nsIDOM3Document should inherit nsIDOM3Node... r/sr=jst
Attachment #127417 -
Flags: review?(jst) → review+
Assignee | ||
Comment 5•18 years ago
|
||
Attachment #127417 -
Attachment is obsolete: true
Assignee | ||
Comment 6•18 years ago
|
||
Comment on attachment 127581 [details] [diff] [review] Updated to comments Patch checked in. Leaving bug open for now to track other work here in the dependency list.
Attachment #127581 -
Attachment is obsolete: true
Updated•17 years ago
|
Priority: -- → P4
Comment 7•10 years ago
|
||
Marking fixed; everything we should implement from DOM3 Document, and rather much more we shouldn't have, has been implemented.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•