DOM/ContentIterator Issues
Content Iterator, we've discovered, has a design flaw in it responsible for several bugs in Mozilla. In essence, nsContentIterator and its friends assume (incorrectly) a nsIContent interface is available for every nsIDOMNode object they receive.
The truth of the matter is, there are four non-content DOM node types (possibly five, if you count XPathResult):
- Attr
- Document
-
DocumentFragment(nsDocumentFragment QI's to nsIContent) - EntityReference
According to DOM Level 3 Core, all four of these node types can have child nodes. These child nodes do implement nsIContent (except for EntityReference, which can itself be a child node of any of these).
Contents
Current Design
nsContentIterator has four key variables determined by its Init() methods:
- nsCOMPtr<nsIContent> mCurNode;
- nsCOMPtr<nsIContent> mFirst;
- nsCOMPtr<nsIContent> mLast;
- nsCOMPtr<nsIContent> mCommonParent;
With DOM ranges, mFirst and mLast are calculable: we go to the first content child of the range's start boundary point, and the last descendant of the range's end boundary point, respectively. mCurNode typically becomes mFirst. mCommonParent is the Achilles heel.
If the commonAncestorContainer of the range is a node of the four types mentioned earlier, then mCommonParent == null. With that condition, the rest of the iterator simply falls apart. RebuildIndexStack, for example, will eventually return NS_ERROR_FAILURE when a GetParent() call returns null, even though it's probably doing the right thing.
So how do we fix this?
Potential Solutions
Re-initialize iterator for each child node
This might mean creating a special iterator which the parent iterator or caller refers to, using the child nodes of the common ancestor as appropriate. Or it might mean the current iterator gets its key parameters reset. When the iterator runs out of nodes for one child node of the common ancestor, move to the next child node of the common ancestor.
We have to be a little careful in this approach, in that we can't cache the child nodes of the common ancestor ahead of time. DOM node trees are live, and can be updated at any time.
Special-case handling of null mCommonParent values
We know the node types which are likely to give us trouble. For instance, we could create a mCommonDocument value to hold a reference to a document, if that were the common ancestor. This may lead to memory bloat, however (mCommonDocument, mCommonAttribute, mCommonEntityRef, mCommonDocumentFragment, and who knows what else we'll need pointers for, most of which will be null). It's also not very scalable.
Replace usage of nsIContent with nsINode
mrbkap nsINode WeirdAl mrbkap: dump nsIContent for nsINode in that case? mrbkap Yeah. WeirdAl mrbkap: I'll need to think about that. I like the idea -- DOM stuff sometimes relies a little too much on content, I've sensed Pike which is a perf decision
