Adds a virtual nsINode::GetBaseURI() and a non-virtual nsINode::GetBaseURI(nsAString&). The latter is not inline, though maybe it should be? (have to pull in nsIURI if so) Also renamed non-addrefing nsIDocument::GetBaseURI to nsIDocument::GetDocumentBaseURI.

Comment on attachment 440784 [details] [diff] [review] Part 2 (move code) v1 Just moving code around, should have no functional changes.

Comment on attachment 440786 [details] [diff] [review] Part 4 (nsGenericTextNode) v1 Add a nsGenericTextNode between nsGenericDOMDataNode and nsTextNode and nsXMLCDATASection for implementing nsIDOM3Text (quickstubs will be able to cast to nsGenericTextNode instead of QI'ing).

Comment on attachment 440794 [details] [diff] [review] Part 12 (change 'canFail' default and allow missing 'code') v1 This inverts the default value for canFail from False to True, so that if we by accident forget to add it we'll get a compile error (|rv = foo();| with foo returning void won't compile) or just check a return value that'll always be NS_OK. Currently, if we forget to add canFail it'll compile, but ignore the nsresult return value. The patch also allows us to not add any custom code even if a thisType is specified, so that we can call functions with the same signature as the one we're stubbing but on a different interface/class, without having to write custom code.

Comment on attachment 440784 [details] [diff] [review] Part 2 (move code) v1 - In nsContentUtils.cpp: +struct ClassMatchingInfo { + nsCOMArray<nsIAtom> mClasses; + nsCaseTreatment mCaseTreatment; +}; + +// static +PRBool +nsDocument::MatchClassNames(nsIContent* aContent, + PRInt32 aNamespaceID, + nsIAtom* aAtom, void* aData) I don't see why this is important... I get why you move ClassMatchingInfo, but could we not put that in a header file and leave the nsDocument method implementations in nsDocument.cpp rather than separating the implementations of some class methods from others? Same goes for sticking nsContentUtils methods in nsGenericElement.cpp.

Comment on attachment 442383 [details] [diff] [review] Part 1 (nsINode::GetBaseURI) v1.1 Uber Nit: Feel free to name the function GetDocBaseURI as that's shorter and we want people to use it. Totally up to you. >diff --git a/content/base/src/nsGenericDOMDataNode.cpp b/content/base/src/nsGenericDOMDataNode.cpp > nsGenericDOMDataNode::GetBaseURI() const > { > // DOM Data Node inherits the base from its parent element/document > nsIContent *parent = GetParent(); > if (parent) { > return parent->GetBaseURI(); > } > >- nsIURI *uri; > nsIDocument *doc = GetOwnerDoc(); >- if (doc) { >- NS_IF_ADDREF(uri = doc->GetBaseURI()); >- } >- else { >- uri = nsnull; >- } > >- return uri; >+ return doc ? doc->GetBaseURI() : nsnull; Nit: You could use GetDocumentBaseURI here, right? r=me

Comment on attachment 442409 [details] [diff] [review] Part 7 (nsINode::IsEqualNode/CompareDocumentPosition/IsSameNode) v1.1 This will conflict with bz's changes in bug 562688, but should be fairly straight forward to update.

(In reply to comment #24) > I don't see why this is important... Ok, I'll leave GetElementsByClassName on nsDocument. But later patches actually move those nsNodeUtils and nsContentUtils methods to nsINode (see for example attachment 442409 [details] [diff] [review] for s/nsContentUtils::ComparePosition/nsINode::CompareDocumentPosition/). Are you objecting to that too? (In reply to comment #25) > > nsGenericDOMDataNode::GetBaseURI() const > >+ return doc ? doc->GetBaseURI() : nsnull; > > Nit: You could use GetDocumentBaseURI here, right? Well, GetDocumentBaseURI returns a non-addrefed pointer but GetBaseURI returns an already_AddRefed so I don't think that would work as is, I'd need to NS_IF_ADDREF.

Comment on attachment 442412 [details] [diff] [review] Part 9 (nsNode3Tearoff for all nodes) v1 NS_INTERFACE_TABLE_HEAD(nsDocument) NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY NS_DOCUMENT_INTERFACE_TABLE_BEGIN(nsDocument) - NS_INTERFACE_TABLE_ENTRY(nsDocument, nsINode) This is either wrong, or I'm really confused. r=jst with that sorted out or explained.

I'll file a bug about making nsDocumentFragment not inherit from nsGenericElement and mention the number in the comment in nsDOMQS.h.

(In reply to comment #27) > (In reply to comment #24) > > I don't see why this is important... > > Ok, I'll leave GetElementsByClassName on nsDocument. I started doing that, but it turns out it's a bit of a hassle to inline GetElementsByClassName in nsGenericElement then, so I ended up keeping the attached patch. (In reply to comment #28) > (From update of attachment 442412 [details] [diff] [review]) > NS_INTERFACE_TABLE_HEAD(nsDocument) > NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY > NS_DOCUMENT_INTERFACE_TABLE_BEGIN(nsDocument) > - NS_INTERFACE_TABLE_ENTRY(nsDocument, nsINode) > > This is either wrong, or I'm really confused. Ah, sorry, forgot to mention that when asking for review. Turns out that these were redundant, NS_NODE_OFFSET_AND_INTERFACE_TABLE_BEGIN adds nsINode already (NS_DOCUMENT_INTERFACE_TABLE_BEGIN calls NS_NODE_OFFSET_AND_INTERFACE_TABLE_BEGIN, same thing for NS_NODE_INTERFACE_TABLE* in nsDOMAttribute.cpp).

http://hg.mozilla.org/mozilla-central/rev/a5e772392c05 http://hg.mozilla.org/mozilla-central/rev/d3b20261c4ef http://hg.mozilla.org/mozilla-central/rev/ef29b5596b77 http://hg.mozilla.org/mozilla-central/rev/d1a0437dff3e http://hg.mozilla.org/mozilla-central/rev/3abca4e8001c http://hg.mozilla.org/mozilla-central/rev/5813224215db http://hg.mozilla.org/mozilla-central/rev/4434bc7cd016 http://hg.mozilla.org/mozilla-central/rev/0786d96d6d21 http://hg.mozilla.org/mozilla-central/rev/c635db22672e http://hg.mozilla.org/mozilla-central/rev/7b8a7ae82c41 http://hg.mozilla.org/mozilla-central/rev/08a17f1b0c14 http://hg.mozilla.org/mozilla-central/rev/74e306877161 http://hg.mozilla.org/mozilla-central/rev/00955067e4b5