From df9d617a7ae578799fcb6aaa3e3788a605538cd0 Mon Sep 17 00:00:00 2001 From: "waterson%netscape.com" Date: Fri, 11 Jan 2002 03:00:53 +0000 Subject: [PATCH] Bug 118863. Fix several incorrect uses of IsElementContainedBy when IsElementInBuilder is required. Fixes problems with nested templates. r=rjc, sr=hyatt. git-svn-id: svn://10.0.0.236/trunk@111924 18797224-902f-48f8-a5cc-f745e15eee43 --- .../xul/templates/src/nsContentTestNode.cpp | 114 +++++++++++------ .../xul/templates/src/nsContentTestNode.h | 8 +- .../xul/templates/src/nsXULContentBuilder.cpp | 118 ++++++++---------- .../templates/src/nsXULTemplateBuilder.cpp | 43 ++----- 4 files changed, 140 insertions(+), 143 deletions(-) diff --git a/mozilla/content/xul/templates/src/nsContentTestNode.cpp b/mozilla/content/xul/templates/src/nsContentTestNode.cpp index f62c61e2d98..2dd4f1a8f2e 100644 --- a/mozilla/content/xul/templates/src/nsContentTestNode.cpp +++ b/mozilla/content/xul/templates/src/nsContentTestNode.cpp @@ -50,45 +50,20 @@ extern PRLogModuleInfo* gXULTemplateLog; #endif -PRBool -IsElementContainedBy(nsIContent* aElement, nsIContent* aContainer) -{ - // Make sure that we're actually creating content for the tree - // content model that we've been assigned to deal with. - - // Walk up the parent chain from us to the root and - // see what we find. - if (aElement == aContainer) - return PR_TRUE; - - // walk up the tree until you find rootAtom - nsCOMPtr element(do_QueryInterface(aElement)); - nsCOMPtr parent; - element->GetParent(*getter_AddRefs(parent)); - element = parent; - - while (element) { - if (element.get() == aContainer) - return PR_TRUE; - - element->GetParent(*getter_AddRefs(parent)); - element = parent; - } - - return PR_FALSE; -} +extern PRBool +IsElementInBuilder(nsIContent *aContent, nsIXULTemplateBuilder *aBuilder); nsContentTestNode::nsContentTestNode(InnerNode* aParent, nsConflictSet& aConflictSet, nsIXULDocument* aDocument, - nsIContent* aRoot, + nsIXULTemplateBuilder* aBuilder, PRInt32 aContentVariable, PRInt32 aIdVariable, nsIAtom* aTag) : TestNode(aParent), mConflictSet(aConflictSet), mDocument(aDocument), - mRoot(aRoot), + mBuilder(aBuilder), mContentVariable(aContentVariable), mIdVariable(aIdVariable), mTag(aTag) @@ -107,6 +82,21 @@ nsContentTestNode::nsContentTestNode(InnerNode* aParent, #endif } +#ifdef PR_LOGGING +static void +ElementToString(nsIContent *aContent, nsString &aResult) +{ + aResult.Truncate(); + + nsCOMPtr tag; + aContent->GetTag(*getter_AddRefs(tag)); + tag->ToString(aResult); + + aResult.Append(PRUnichar('@')); + aResult.AppendInt(PRInt32(aContent), 16); +} +#endif + nsresult nsContentTestNode::FilterInstantiations(InstantiationSet& aInstantiations, void* aClosure) const { @@ -124,6 +114,22 @@ nsContentTestNode::FilterInstantiations(InstantiationSet& aInstantiations, void* Value idValue; PRBool hasIdBinding = inst->mAssignments.GetAssignmentFor(mIdVariable, &idValue); +#ifdef PR_LOGGING + if (PR_LOG_TEST(gXULTemplateLog, PR_LOG_DEBUG)) { + nsAutoString content(NS_LITERAL_STRING("(unbound)")); + if (hasContentBinding) + ElementToString(VALUE_TO_ICONTENT(contentValue), content); + + const char *id = "(unbound)"; + if (hasIdBinding) + VALUE_TO_IRDFRESOURCE(idValue)->GetValueConst(&id); + + PR_LOG(gXULTemplateLog, PR_LOG_DEBUG, + ("nsContentTestNode[%p]: FilterInstantiations() content=%s id=%s", + this, NS_LossyConvertUCS2toASCII(content).get(), id)); + } +#endif + if (hasContentBinding && hasIdBinding) { // both are bound, consistency check PRBool consistent = PR_TRUE; @@ -147,6 +153,9 @@ nsContentTestNode::FilterInstantiations(InstantiationSet& aInstantiations, void* consistent = PR_FALSE; } + PR_LOG(gXULTemplateLog, PR_LOG_DEBUG, + (" consistency check => %s", consistent ? "passed" : "failed")); + if (consistent) { Element* element = nsContentTestNode::Element::Create(mConflictSet.GetPool(), @@ -172,8 +181,18 @@ nsContentTestNode::FilterInstantiations(InstantiationSet& aInstantiations, void* nsCOMPtr tag; content->GetTag(*getter_AddRefs(tag)); - if (tag != mTag) + if (tag != mTag) { consistent = PR_FALSE; + + const PRUnichar *expected, *actual; + mTag->GetUnicode(&expected); + tag->GetUnicode(&actual); + + PR_LOG(gXULTemplateLog, PR_LOG_DEBUG, + (" => tag mismatch; expected %s, actual %s", + NS_LossyConvertUCS2toASCII(expected).get(), + NS_LossyConvertUCS2toASCII(actual).get())); + } } if (consistent) { @@ -181,6 +200,15 @@ nsContentTestNode::FilterInstantiations(InstantiationSet& aInstantiations, void* nsXULContentUtils::GetElementRefResource(content, getter_AddRefs(resource)); if (resource) { +#ifdef PR_LOGGING + if (PR_LOG_TEST(gXULTemplateLog, PR_LOG_DEBUG)) { + const char *str; + resource->GetValueConst(&str); + PR_LOG(gXULTemplateLog, PR_LOG_DEBUG, + (" => [%s]", str)); + } +#endif + Instantiation newinst = *inst; newinst.AddAssignment(mIdVariable, Value(resource.get())); @@ -194,6 +222,10 @@ nsContentTestNode::FilterInstantiations(InstantiationSet& aInstantiations, void* aInstantiations.Insert(inst, newinst); } + else { + PR_LOG(gXULTemplateLog, PR_LOG_DEBUG, + (" => element has no resource")); + } } aInstantiations.Erase(inst--); @@ -201,21 +233,19 @@ nsContentTestNode::FilterInstantiations(InstantiationSet& aInstantiations, void* else if (hasIdBinding) { // the 'id' is bound, find elements in the content tree that match const char* uri; - rv = VALUE_TO_IRDFRESOURCE(idValue)->GetValueConst(&uri); - if (NS_FAILED(rv)) return rv; + VALUE_TO_IRDFRESOURCE(idValue)->GetValueConst(&uri); + + mDocument->GetElementsForID(NS_ConvertUTF8toUCS2(uri), elements); - rv = mDocument->GetElementsForID(NS_ConvertUTF8toUCS2(uri), elements); - if (NS_FAILED(rv)) return rv; PRUint32 count; - rv = elements->Count(&count); - if (NS_FAILED(rv)) return rv; + elements->Count(&count); for (PRInt32 j = PRInt32(count) - 1; j >= 0; --j) { nsISupports* isupports = elements->ElementAt(j); nsCOMPtr content = do_QueryInterface(isupports); NS_IF_RELEASE(isupports); - if (IsElementContainedBy(content, mRoot)) { + if (IsElementInBuilder(content, mBuilder)) { if (mTag) { // If we've got a tag, check it to ensure // we're consistent. @@ -226,6 +256,16 @@ nsContentTestNode::FilterInstantiations(InstantiationSet& aInstantiations, void* continue; } +#ifdef PR_LOGGING + if (PR_LOG_TEST(gXULTemplateLog, PR_LOG_DEBUG)) { + nsAutoString str; + ElementToString(content, str); + + PR_LOG(gXULTemplateLog, PR_LOG_DEBUG, + (" => %s", NS_LossyConvertUCS2toASCII(str).get())); + } +#endif + Instantiation newinst = *inst; newinst.AddAssignment(mContentVariable, Value(content.get())); diff --git a/mozilla/content/xul/templates/src/nsContentTestNode.h b/mozilla/content/xul/templates/src/nsContentTestNode.h index 1c4b14bcbc8..58b62c09768 100644 --- a/mozilla/content/xul/templates/src/nsContentTestNode.h +++ b/mozilla/content/xul/templates/src/nsContentTestNode.h @@ -45,6 +45,7 @@ #include "nsFixedSizeAllocator.h" #include "nsIAtom.h" +class nsIXULTemplateBuilder; class nsIXULDocument; class nsConflictSet; @@ -54,7 +55,7 @@ public: nsContentTestNode(InnerNode* aParent, nsConflictSet& aConflictSet, nsIXULDocument* aDocument, - nsIContent* aRoot, + nsIXULTemplateBuilder* aBuilder, PRInt32 aContentVariable, PRInt32 aIdVariable, nsIAtom* aTag); @@ -112,14 +113,11 @@ public: protected: nsConflictSet& mConflictSet; nsIXULDocument* mDocument; // [WEAK] because we know the document will outlive us - nsCOMPtr mRoot; + nsIXULTemplateBuilder *mBuilder; PRInt32 mContentVariable; PRInt32 mIdVariable; nsCOMPtr mTag; }; -extern PRBool -IsElementContainedBy(nsIContent* aElement, nsIContent* aContainer); - #endif // nsContentTestNode_h__ diff --git a/mozilla/content/xul/templates/src/nsXULContentBuilder.cpp b/mozilla/content/xul/templates/src/nsXULContentBuilder.cpp index 919d8b7a577..82870078f5a 100644 --- a/mozilla/content/xul/templates/src/nsXULContentBuilder.cpp +++ b/mozilla/content/xul/templates/src/nsXULContentBuilder.cpp @@ -70,6 +70,7 @@ #include "nsXULContentUtils.h" #include "nsXULElement.h" #include "nsXULTemplateBuilder.h" +#include "nsSupportsArray.h" #include "jsapi.h" #include "pldhash.h" @@ -82,6 +83,38 @@ static NS_DEFINE_CID(kTextNodeCID, NS_TEXTNODE_CID); static NS_DEFINE_CID(kXMLElementFactoryCID, NS_XML_ELEMENT_FACTORY_CID); static NS_DEFINE_CID(kXULSortServiceCID, NS_XULSORTSERVICE_CID); +PRBool +IsElementInBuilder(nsIContent *aContent, nsIXULTemplateBuilder *aBuilder) +{ + // Make sure that the element is contained within the heirarchy + // that we're supposed to be processing. + nsCOMPtr doc; + aContent->GetDocument(*getter_AddRefs(doc)); + + nsCOMPtr xuldoc = do_QueryInterface(doc); + if (! xuldoc) + return PR_FALSE; + + nsCOMPtr content = dont_QueryInterface(aContent); + do { + nsCOMPtr builder; + xuldoc->GetTemplateBuilderFor(content, getter_AddRefs(builder)); + if (builder) { + if (builder == aBuilder) + return PR_TRUE; // aBuilder is the builder for this element. + + // We found a builder, but it's not aBuilder. + break; + } + + nsCOMPtr parent; + content->GetParent(*getter_AddRefs(parent)); + content = parent; + } while (content); + + return PR_FALSE; +} + //---------------------------------------------------------------------- // // Return values for EnsureElementHasGenericChild() @@ -126,9 +159,6 @@ protected: nsresult Init(); // Implementation methods - PRBool - IsElementInBuilder(nsIContent *aElement); - nsresult OpenContainer(nsIContent* aElement); @@ -930,6 +960,9 @@ nsXULContentBuilder::SynchronizeUsingTemplate(nsIContent* aTemplateNode, rv = aTemplateNode->GetAttrCount(numAttribs); if (NS_FAILED(rv)) return rv; + // XXXwaterson. Ugh, we just checked the failure code, above. Why + // do it again? This method needs a scrub-down, and could stand + // to have some of this bogo-error checking removed. if (rv == NS_CONTENT_ATTR_HAS_VALUE) { for (PRInt32 aLoop=0; aLoopmAssignments.GetAssignmentFor(mContentVar, &parentValue); - - nsIContent* parent = VALUE_TO_ICONTENT(parentValue); - // Now that we've got the resource of the member variable, we // should be able to update its kids appropriately - nsresult rv; - nsCOMPtr elements; - rv = NS_NewISupportsArray(getter_AddRefs(elements)); - NS_ASSERTION(NS_SUCCEEDED(rv), "unable to create new ISupportsArray"); - if (NS_FAILED(rv)) return rv; + nsSupportsArray elements; + GetElementsForResource(resource, &elements); - rv = GetElementsForResource(resource, elements); - NS_ASSERTION(NS_SUCCEEDED(rv), "unable to retrieve elements from resource"); - if (NS_FAILED(rv)) return rv; - - PRUint32 cnt; - rv = elements->Count(&cnt); - if (NS_FAILED(rv)) return rv; + PRUint32 cnt = 0; + elements.Count(&cnt); #ifdef PR_LOGGING if (PR_LOG_TEST(gXULTemplateLog, PR_LOG_DEBUG) && cnt == 0) { @@ -1939,24 +1958,19 @@ nsXULContentBuilder::SynchronizeMatch(nsTemplateMatch* match, const VariableSet& #endif for (PRInt32 i = PRInt32(cnt) - 1; i >= 0; --i) { - nsISupports* isupports = elements->ElementAt(i); - nsCOMPtr element( do_QueryInterface(isupports) ); - NS_IF_RELEASE(isupports); - - // If the element is contained by the parent of the rule - // that we've just matched, then it's the wrong element. - if (! IsElementContainedBy(element, parent)) + nsCOMPtr element = do_QueryElementAt(&elements, i); + if (! IsElementInBuilder(element, this)) continue; nsCOMPtr templateNode; mTemplateMap.GetTemplateFor(element, getter_AddRefs(templateNode)); + NS_ASSERTION(templateNode, "couldn't find template node for element"); if (! templateNode) - return NS_ERROR_UNEXPECTED; + continue; // this node was created by a XUL template, so update it accordingly - rv = SynchronizeUsingTemplate(templateNode, element, *match, modified); - if (NS_FAILED(rv)) return rv; + SynchronizeUsingTemplate(templateNode, element, *match, modified); } #ifdef PR_LOGGING @@ -1977,43 +1991,11 @@ nsXULContentBuilder::SynchronizeMatch(nsTemplateMatch* match, const VariableSet& // Implementation methods // -PRBool -nsXULContentBuilder::IsElementInBuilder(nsIContent *aContent) -{ - // Make sure that the element is contained within the heirarchy - // that we're supposed to be processing. - nsCOMPtr doc; - aContent->GetDocument(*getter_AddRefs(doc)); - - nsCOMPtr xuldoc = do_QueryInterface(doc); - if (! xuldoc) - return PR_FALSE; - - nsCOMPtr content = dont_QueryInterface(aContent); - do { - nsCOMPtr builder; - xuldoc->GetTemplateBuilderFor(content, getter_AddRefs(builder)); - if (builder) { - if (builder == NS_STATIC_CAST(nsIXULTemplateBuilder *, this)) - return PR_TRUE; // We're the builder for this element. - - // We found a builder, but it's not us. - break; - } - - nsCOMPtr parent; - content->GetParent(*getter_AddRefs(parent)); - content = parent; - } while (content); - - return PR_FALSE; -} - nsresult nsXULContentBuilder::OpenContainer(nsIContent* aElement) { // See if we're responsible for this element - if (! IsElementInBuilder(aElement)) + if (! IsElementInBuilder(aElement, this)) return NS_OK; nsCOMPtr resource; @@ -2055,7 +2037,7 @@ nsresult nsXULContentBuilder::CloseContainer(nsIContent* aElement) { // See if we're responsible for this element - if (! IsElementInBuilder(aElement)) + if (! IsElementInBuilder(aElement, this)) return NS_OK; nsCOMPtr tag; @@ -2143,7 +2125,7 @@ nsXULContentBuilder::InitializeRuleNetworkForSimpleRules(InnerNode** aChildNode) new nsContentTestNode(mRules.GetRoot(), mConflictSet, xuldoc, - mRoot, + this, mContentVar, mContainerVar, nsnull); @@ -2263,7 +2245,7 @@ nsXULContentBuilder::CompileContentCondition(nsTemplateRule* aRule, new nsContentTestNode(aParentNode, mConflictSet, xuldoc, - mRoot, + this, mContentVar, // XXX see above urivar, tag); diff --git a/mozilla/content/xul/templates/src/nsXULTemplateBuilder.cpp b/mozilla/content/xul/templates/src/nsXULTemplateBuilder.cpp index 3fb8adc199e..2ce4b764138 100644 --- a/mozilla/content/xul/templates/src/nsXULTemplateBuilder.cpp +++ b/mozilla/content/xul/templates/src/nsXULTemplateBuilder.cpp @@ -469,9 +469,8 @@ nsXULTemplateBuilder::Propagate(nsIRDFResource* aSource, nsRDFTestNode* rdftestnode = NS_STATIC_CAST(nsRDFTestNode*, *i); Instantiation seed; - if (rdftestnode->CanPropagate(aSource, aProperty, aTarget, seed)) { + if (rdftestnode->CanPropagate(aSource, aProperty, aTarget, seed)) livenodes.Add(rdftestnode); - } } } @@ -573,23 +572,15 @@ nsXULTemplateBuilder::OnAssert(nsIRDFDataSource* aDataSource, if (IsActivated(aSource)) return NS_OK; - nsresult rv; - if (mCache) mCache->Assert(aSource, aProperty, aTarget, PR_TRUE /* XXX should be value passed in */); LOG("onassert", aSource, aProperty, aTarget); nsClusterKeySet newkeys; - rv = Propagate(aSource, aProperty, aTarget, newkeys); - if (NS_FAILED(rv)) return rv; - - rv = FireNewlyMatchedRules(newkeys); - if (NS_FAILED(rv)) return rv; - - rv = SynchronizeAll(aSource, aProperty, nsnull, aTarget); - if (NS_FAILED(rv)) return rv; - + Propagate(aSource, aProperty, aTarget, newkeys); + FireNewlyMatchedRules(newkeys); + SynchronizeAll(aSource, aProperty, nsnull, aTarget); return NS_OK; } @@ -646,19 +637,13 @@ nsXULTemplateBuilder::OnUnassert(nsIRDFDataSource* aDataSource, if (IsActivated(aSource)) return NS_OK; - nsresult rv; - if (mCache) mCache->Unassert(aSource, aProperty, aTarget); LOG("onunassert", aSource, aProperty, aTarget); - rv = Retract(aSource, aProperty, aTarget); - if (NS_FAILED(rv)) return rv; - - rv = SynchronizeAll(aSource, aProperty, aTarget, nsnull); - if (NS_FAILED(rv)) return rv; - + Retract(aSource, aProperty, aTarget); + SynchronizeAll(aSource, aProperty, aTarget, nsnull); return NS_OK; } @@ -688,30 +673,22 @@ nsXULTemplateBuilder::OnChange(nsIRDFDataSource* aDataSource, mCache->Assert(aSource, aProperty, aNewTarget, PR_TRUE); } - nsresult rv; - LOG("onchange", aSource, aProperty, aNewTarget); if (aOldTarget) { // Pull any old rules that were relying on aOldTarget - rv = Retract(aSource, aProperty, aOldTarget); - if (NS_FAILED(rv)) return rv; + Retract(aSource, aProperty, aOldTarget); } if (aNewTarget) { // Fire any new rules that are activated by aNewTarget nsClusterKeySet newkeys; - rv = Propagate(aSource, aProperty, aNewTarget, newkeys); - if (NS_FAILED(rv)) return rv; - - rv = FireNewlyMatchedRules(newkeys); - if (NS_FAILED(rv)) return rv; + Propagate(aSource, aProperty, aNewTarget, newkeys); + FireNewlyMatchedRules(newkeys); } // Synchronize any of the content model that may have changed. - rv = SynchronizeAll(aSource, aProperty, aOldTarget, aNewTarget); - if (NS_FAILED(rv)) return rv; - + SynchronizeAll(aSource, aProperty, aOldTarget, aNewTarget); return NS_OK; }