Bug 1026008 Incorrect XBL behaviour with multiple insertion points r=mrbkap
--- a/content/base/src/ChildIterator.cpp
+++ b/content/base/src/ChildIterator.cpp
@@ -20,29 +20,29 @@ public:
: mIsContentElement(true), mContentElement(aInsertionPoint) {}
MatchedNodes(XBLChildrenElement* aInsertionPoint)
: mIsContentElement(false), mChildrenElement(aInsertionPoint) {}
uint32_t Length() const
{
return mIsContentElement ? mContentElement->MatchedNodes().Length()
- : mChildrenElement->mInsertedChildren.Length();
+ : mChildrenElement->InsertedChildrenLength();
}
nsIContent* operator[](int32_t aIndex) const
{
return mIsContentElement ? mContentElement->MatchedNodes()[aIndex]
- : mChildrenElement->mInsertedChildren[aIndex];
+ : mChildrenElement->InsertedChild(aIndex);
}
bool IsEmpty() const
{
return mIsContentElement ? mContentElement->MatchedNodes().IsEmpty()
- : mChildrenElement->mInsertedChildren.IsEmpty();
+ : !mChildrenElement->HasInsertedChildren();
}
protected:
bool mIsContentElement;
union {
HTMLContentElement* mContentElement;
XBLChildrenElement* mChildrenElement;
};
};
--- a/dom/xbl/XBLChildrenElement.cpp
+++ b/dom/xbl/XBLChildrenElement.cpp
@@ -98,18 +98,18 @@ nsAnonymousContentList::GetLength(uint32
}
uint32_t count = 0;
for (nsIContent* child = mParent->GetFirstChild();
child;
child = child->GetNextSibling()) {
if (child->NodeInfo()->Equals(nsGkAtoms::children, kNameSpaceID_XBL)) {
XBLChildrenElement* point = static_cast<XBLChildrenElement*>(child);
- if (!point->mInsertedChildren.IsEmpty()) {
- count += point->mInsertedChildren.Length();
+ if (point->HasInsertedChildren()) {
+ count += point->InsertedChildrenLength();
}
else {
count += point->GetChildCount();
}
}
else {
++count;
}
@@ -139,21 +139,21 @@ nsAnonymousContentList::Item(uint32_t aI
}
uint32_t remIndex = aIndex;
for (nsIContent* child = mParent->GetFirstChild();
child;
child = child->GetNextSibling()) {
if (child->NodeInfo()->Equals(nsGkAtoms::children, kNameSpaceID_XBL)) {
XBLChildrenElement* point = static_cast<XBLChildrenElement*>(child);
- if (!point->mInsertedChildren.IsEmpty()) {
- if (remIndex < point->mInsertedChildren.Length()) {
- return point->mInsertedChildren[remIndex];
+ if (point->HasInsertedChildren()) {
+ if (remIndex < point->InsertedChildrenLength()) {
+ return point->InsertedChild(remIndex);
}
- remIndex -= point->mInsertedChildren.Length();
+ remIndex -= point->InsertedChildrenLength();
}
else {
if (remIndex < point->GetChildCount()) {
return point->GetChildAt(remIndex);
}
remIndex -= point->GetChildCount();
}
}
@@ -174,33 +174,33 @@ nsAnonymousContentList::IndexOf(nsIConte
NS_ASSERTION(!aContent->NodeInfo()->Equals(nsGkAtoms::children,
kNameSpaceID_XBL),
"Looking for insertion point");
if (!mParent) {
return -1;
}
- size_t index = 0;
+ int32_t index = 0;
for (nsIContent* child = mParent->GetFirstChild();
child;
child = child->GetNextSibling()) {
if (child->NodeInfo()->Equals(nsGkAtoms::children, kNameSpaceID_XBL)) {
XBLChildrenElement* point = static_cast<XBLChildrenElement*>(child);
- if (!point->mInsertedChildren.IsEmpty()) {
- size_t insIndex = point->mInsertedChildren.IndexOf(aContent);
- if (insIndex != point->mInsertedChildren.NoIndex) {
+ if (point->HasInsertedChildren()) {
+ int32_t insIndex = point->IndexOfInsertedChild(aContent);
+ if (insIndex != -1) {
return index + insIndex;
}
- index += point->mInsertedChildren.Length();
+ index += point->InsertedChildrenLength();
}
else {
int32_t insIndex = point->IndexOf(aContent);
if (insIndex != -1) {
- return index + (size_t)insIndex;
+ return index + insIndex;
}
index += point->GetChildCount();
}
}
else {
if (child == aContent) {
return index;
}
--- a/dom/xbl/XBLChildrenElement.h
+++ b/dom/xbl/XBLChildrenElement.h
@@ -17,18 +17,16 @@ class nsAnonymousContentList;
namespace mozilla {
namespace dom {
class ExplicitChildIterator;
class XBLChildrenElement : public nsXMLElement
{
public:
- friend class mozilla::dom::ExplicitChildIterator;
- friend class nsAnonymousContentList;
XBLChildrenElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo)
: nsXMLElement(aNodeInfo)
{
}
XBLChildrenElement(already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
: nsXMLElement(aNodeInfo)
{
}
@@ -110,39 +108,40 @@ public:
return mInsertedChildren.Length();
}
bool HasInsertedChildren()
{
return !mInsertedChildren.IsEmpty();
}
- enum {
- NoIndex = uint32_t(-1)
- };
- uint32_t IndexOfInsertedChild(nsIContent* aChild)
+ int32_t IndexOfInsertedChild(nsIContent* aChild)
{
return mInsertedChildren.IndexOf(aChild);
}
bool Includes(nsIContent* aChild)
{
NS_ASSERTION(!mIncludes.IsEmpty(),
"Shouldn't check for includes on default insertion point");
return mIncludes.Contains(aChild->Tag());
}
bool IsDefaultInsertion()
{
return mIncludes.IsEmpty();
}
- nsTArray<nsIContent*> mInsertedChildren;
+ nsIContent* InsertedChild(uint32_t aIndex)
+ {
+ return mInsertedChildren[aIndex];
+ }
private:
+ nsTArray<nsIContent*> mInsertedChildren; // WEAK
nsTArray<nsCOMPtr<nsIAtom> > mIncludes;
};
} // namespace dom
} // namespace mozilla
class nsAnonymousContentList : public nsINodeList
{
--- a/dom/xbl/nsBindingManager.cpp
+++ b/dom/xbl/nsBindingManager.cpp
@@ -829,30 +829,30 @@ nsBindingManager::AppendAllSheets(nsTArr
mBoundContentSet->EnumerateEntries(EnumAppendAllSheets, &aArray);
}
}
static void
InsertAppendedContent(XBLChildrenElement* aPoint,
nsIContent* aFirstNewContent)
{
- size_t insertionIndex;
+ int32_t insertionIndex;
if (nsIContent* prevSibling = aFirstNewContent->GetPreviousSibling()) {
// If we have a previous sibling, then it must already be in aPoint. Find
// it and insert after it.
insertionIndex = aPoint->IndexOfInsertedChild(prevSibling);
- MOZ_ASSERT(insertionIndex != aPoint->NoIndex);
+ MOZ_ASSERT(insertionIndex != -1);
// Our insertion index is one after our previous sibling's index.
++insertionIndex;
} else {
// Otherwise, we append.
// TODO This is wrong for nested insertion points. In that case, we need to
// keep track of the right index to insert into.
- insertionIndex = aPoint->mInsertedChildren.Length();
+ insertionIndex = aPoint->InsertedChildrenLength();
}
// Do the inserting.
for (nsIContent* currentChild = aFirstNewContent;
currentChild;
currentChild = currentChild->GetNextSibling()) {
aPoint->InsertInsertedChildAt(currentChild, insertionIndex++);
}
@@ -1072,25 +1072,25 @@ nsBindingManager::HandleChildInsertion(n
if (!point) {
break;
}
// Insert the child into the proper insertion point.
// TODO If there were multiple insertion points, this approximation can be
// wrong. We need to re-run the distribution algorithm. In the meantime,
// this should work well enough.
- size_t index = aAppend ? point->mInsertedChildren.Length() : 0;
+ uint32_t index = aAppend ? point->InsertedChildrenLength() : 0;
for (nsIContent* currentSibling = aChild->GetPreviousSibling();
currentSibling;
currentSibling = currentSibling->GetPreviousSibling()) {
// If we find one of our previous siblings in the insertion point, the
// index following it is the correct insertion point. Otherwise, we guess
// based on whether we're appending or inserting.
- size_t pointIndex = point->IndexOfInsertedChild(currentSibling);
- if (pointIndex != point->NoIndex) {
+ int32_t pointIndex = point->IndexOfInsertedChild(currentSibling);
+ if (pointIndex != -1) {
index = pointIndex + 1;
break;
}
}
point->InsertInsertedChildAt(aChild, index);
nsIContent* newParent = point->GetParent();
--- a/dom/xbl/nsXBLBinding.cpp
+++ b/dom/xbl/nsXBLBinding.cpp
@@ -697,17 +697,17 @@ static void
UpdateInsertionParent(XBLChildrenElement* aPoint,
nsIContent* aOldBoundElement)
{
if (aPoint->IsDefaultInsertion()) {
return;
}
for (size_t i = 0; i < aPoint->InsertedChildrenLength(); ++i) {
- nsIContent* child = aPoint->mInsertedChildren[i];
+ nsIContent* child = aPoint->InsertedChild(i);
MOZ_ASSERT(child->GetParentNode());
// Here, we're iterating children that we inserted. There are two cases:
// either |child| is an explicit child of |aOldBoundElement| and is no
// longer inserted anywhere or it's a child of a <children> element
// parented to |aOldBoundElement|. In the former case, the child is no
// longer inserted anywhere, so we set its insertion parent to null. In the