199399 - Eliminate nsIXMLDocument

Status: RESOLVED FIXED

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

Description

[Johnny Stenback (:jst)]

Once bug 111514 is fixed it looks like we can eliminate nsIXMLDocument from
Mozilla. After bug 111514 is fixed it'll only have one method in it and I don't
see anyone using that...

Comment 1

[Johnny Stenback (:jst)]

Attachment: Eliminate nsIXMLDocument

Comment 2

[Peter Van der Beken [:peterv]]

Comment on attachment 121242 [details] [diff] [review]
Eliminate nsIXMLDocument

>Index: content/base/src/nsGenericElement.cpp
>===================================================================

>+    nsCOMPtr<nsIContent> tmp(content);
>+    tmp->GetParent(*getter_AddRefs(content));

Small nit, you could make this:

    nsCOMPtr<nsIContent> tmp;
    content.swap(tmp);
    tmp->GetParent(*getter_AddRefs(content));

Avoids one addref/release pair.

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 121242 [details] [diff] [review]
Eliminate nsIXMLDocument

I am concerned that now non-XML documents would pay a perf penalty for baseURI
access. Could you do some other check in nsGenericElement to avoid entering the
while loop in nsGenericElement, maybe QI to nsIDOMXMLDocument or something? See
this link for where we use baseURI in chrome:
http://lxr.mozilla.org/seamonkey/search?string=%5C.baseURI

Add newline to to the end of content/xml/document/public/MANIFEST

Comment 4

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 121242 [details] [diff] [review]
Eliminate nsIXMLDocument

I am concerned that now non-XML documents would pay a perf penalty for baseURI
access. Could you do some other check in nsGenericElement to avoid entering the
while loop in nsGenericElement, maybe QI to nsIDOMXMLDocument or something? See
this link for where we use baseURI in chrome:
http://lxr.mozilla.org/seamonkey/search?string=%5C.baseURI

Add newline to to the end of content/xml/document/public/MANIFEST

Comment 5

[Johnny Stenback (:jst)]

I intentionally did *not* want to check what type of document we're dealing with
in nsGenericElement.cpp, since even if we're not dealing with an XML document
(i.e. it's an HTML or XHTML document) we still want to find XML base info if it
exists in the ancestor chain (i.e. think of an SVG element embedded in an XHTML
document). I'm not too worried about this walk up the ancestor chain from a
performance point of view, I don't see it being used in any place where
performance is all that critical.

PS. There is a new line at the end of the MANIFEST file, but there wasn't one on
the line I removed (that's what the diff tells you).

Comment 6

[Johnny Stenback (:jst)]

Fix checked in. FIXED.