104219 - we need a nsIContent lookalike for attributes

Status: RESOLVED WONTFIX

(Core :: XML, defect)

Description

[Axel Hecht]

There is currently no way to get to the nodeinfo of a attribute. This is sad,
and gives us more reason to sob in XSLT.
There are (AFAICT) two implementations of attributes,
content/base/src/nsDOMAttribute.h and
content/xul/content/src/nsXULAttributes.h.
They don't share an interface but the string oriented nsIDOMAttr (slow).
nsDOMAttribute has an nsIDOMAttributePrivate, but I can't find that this is used.

We would need to get the local name atom and the nsid from an attribute, so some
nsIAttr should have at least

   NS_IMETHOD GetDocument(nsIDocument*& aResult) const = 0;
   NS_IMETHOD GetNameSpaceID(PRInt32& aResult) const = 0;
   NS_IMETHOD GetTag(nsIAtom*& aResult) const = 0;

We could use a
  NS_IMETHOD IndexOf(nsIAttr* aPossibleChild, PRInt32& aIndex) const = 0;

Comments?

Axel

Comment 1

[Chris Waterson]

Please try to do this in a way that doesn't require increasing the storage size
of attributes.

Does XSLT currently use nsIContent?

Comment 2

[Axel Hecht]

see bug 76070 for the first strike in our current battle against string
compares. And XSLT has the (in terms of mozilla) very sad attitude of thinking
of attributes as full fledged nodes. So we have to deal with attribute nodes
efficiently, and get rid of as many string compares as possible.

So the answer is no, not yet, but we should get as close as we can.

Comment 3

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

In my earliset iteration to fix bug 41983 I had added GetAttrAt(PRInt32 aIndex,
nsINodeInfo **aNodeInfo) to nsIContent, but jst did not want me adding new
methods to that interface.

That bug, once I check it in (I have some crasher problems), will make it so
that HTML content stores attributes as node info's like XML. Then it is just a
matter of deciding what kind of methods we provide to get them.

But are you sure you need node infos directly, because there is GetAttrNameAt()
which gives you name, prefix and namespace ID, with which you can get the
corresponding node info from the node manager?

Comment 4

[Axel Hecht]

Heikki,
we don't need the nodeinfo, but we need localname atom and nsid.
The trick (or treat) is, we start with an attribute node. So we don't know the
position of that node, and GetAttrNameAt doesn't help.
I'm relieved to hear good news from the html attr front though.

Axel

Comment 5

[Fabian Guisset]

Is this part of a larger revamp of the attribute node support? Given that the
current implementation isn't that good (lots of bugs about attr nodes), I really
hope it is part of a larger plan. Hacking stuff here and there is imho not the
solution in this case. (note that without investigating the problem, nsIContent
for attributes looks like a very good step)

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I hope to do a new implementation of attributes soon, which should do what you 
request and much much more :-)

However this will probobly take a few weeks to design and implement so i don't 
wanna hold off landing the xslt code until that is done.

What the xslt code would ideally like to have is an

nsIContent::IndexOf(nsIAttr* aPossibleChild, PRInt32& aIndex)

and

nsIAttr::IndexOf(nsIContent* aPossibleChild, PRInt32& aIndex)

but of course we *could* live without either of them...

Comment 7

[Axel Hecht]

I guess we have two topics here, one is attributes, that is handling attributes
from their ownerElement, and one is attribute nodes.
I was talking about attribute nodes here. Though the current distinction between
attributes and attribute nodes really causes some pain. For example the multiple
attribute nodes for a single attribute.
(But then again, if getting the specifying data for an attribute node would make
that alot less hurtfull.)
I´m afraid we don´t say exactly enough when we are talking about which.

Comment 8

[Johnny Stenback (:jst)]

I don't like adding nsIContent::IndexOf(nsIAttr *), I could be talked into
adding a nsIContent::IndexOfAttr(nsINodeInfo *) tho...

Comment 9

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

I don't have cycles for this in the near future, so moving to Future milestone.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

XSLT does not need this any more, and I don't think anyone else does either.
Suggesting WONTFIX

Comment 11

[Johnny Stenback (:jst)]

WONTFIX it is then. Reopen if you disagree.