From c1e558bc249ac0af71618bad9bdd1386a0f4ba70 Mon Sep 17 00:00:00 2001 From: "Olli.Pettay%helsinki.fi" Date: Tue, 26 Feb 2008 12:40:21 +0000 Subject: [PATCH] Bug 411054, Audit IsNativeAnonymous()/GetBindingParent() uses, r+sr=sicking --- caps/src/nsScriptSecurityManager.cpp | 2 +- content/base/public/nsContentUtils.h | 6 -- content/base/public/nsIContent.h | 15 ++++- content/base/src/nsContentUtils.cpp | 30 --------- content/base/src/nsDocument.cpp | 2 +- content/base/src/nsGenericElement.cpp | 65 ++++++++++++------- content/base/src/nsStyledElement.cpp | 4 +- content/events/src/nsEventListenerManager.cpp | 48 +++++++------- content/xbl/src/nsBindingManager.cpp | 4 +- embedding/components/find/src/nsFind.cpp | 20 ++---- layout/base/nsCSSFrameConstructor.cpp | 8 ++- layout/style/nsStyleSet.cpp | 6 +- layout/xul/base/src/nsBoxFrame.cpp | 2 +- 13 files changed, 99 insertions(+), 113 deletions(-) diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index 0cef8f6428d..a6053de2318 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -856,7 +856,7 @@ nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction, { // No access to anonymous content from the web! (bug 164086) nsIContent *content = static_cast(aObj); - if (content->IsNativeAnonymous()) { + if (content->IsInNativeAnonymousSubtree()) { rv = NS_ERROR_DOM_SECURITY_ERR; } } diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h index f0ba2f149d0..7d582485b4c 100644 --- a/content/base/public/nsContentUtils.h +++ b/content/base/public/nsContentUtils.h @@ -1128,12 +1128,6 @@ public: nsIURI *aLinkURI, const nsString& aTargetSpec, PRBool aClick, PRBool aIsUserTriggered); - /** - * Return true if aContent or one of its ancestors in the - * bindingParent chain is native anonymous. - */ - static PRBool IsNativeAnonymous(nsIContent* aContent); - /** * Return top-level widget in the parent chain. */ diff --git a/content/base/public/nsIContent.h b/content/base/public/nsIContent.h index 38c59b17031..8facd545e82 100644 --- a/content/base/public/nsIContent.h +++ b/content/base/public/nsIContent.h @@ -62,8 +62,8 @@ class nsIDocShell; // IID for the nsIContent interface #define NS_ICONTENT_IID \ -{ 0xfba9aa39, 0x016e, 0x4d5d, \ - { 0xab, 0x62, 0x22, 0xa1, 0xb8, 0x4a, 0x3c, 0x7b } } +{ 0xd3434698, 0x3a16, 0x4dbe, \ + { 0x9d, 0xed, 0xbe, 0x64, 0x16, 0x1a, 0xa3, 0x52 } } /** * A node of content in a document's content model. This interface @@ -160,6 +160,17 @@ public: SetFlags(NODE_IS_ANONYMOUS); } + /** + * Returns |this| if it is not native anonymous, otherwise + * first non native anonymous ancestor. + */ + virtual nsIContent* FindFirstNonNativeAnonymous() const; + + /** + * Returns PR_TRUE if |this| or any of its ancestors is native anonymous. + */ + virtual PRBool IsInNativeAnonymousSubtree() const; + /** * Get the namespace that this element's tag is defined in * @return the namespace diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp index ad0564faeb3..06788451475 100644 --- a/content/base/src/nsContentUtils.cpp +++ b/content/base/src/nsContentUtils.cpp @@ -3852,36 +3852,6 @@ nsContentUtils::TriggerLink(nsIContent *aContent, nsPresContext *aPresContext, } } -/* static */ -PRBool -nsContentUtils::IsNativeAnonymous(nsIContent* aContent) -{ - while (aContent) { - nsIContent* bindingParent = aContent->GetBindingParent(); - if (bindingParent == aContent) { - NS_ASSERTION(bindingParent->IsNativeAnonymous() || - bindingParent->IsNodeOfType(nsINode::eXUL), - "Bogus binding parent?"); - return PR_TRUE; - } - - // Nasty hack to work around spell-check resolving style on - // native-anonymous content that's already been torn down. Don't assert - // !IsNativeAnonymous() if aContent->GetCurrentDoc() is null. The caller - // will get "wrong" style data, but it's just asking for that sort of thing - // anyway. - NS_ASSERTION(!aContent->IsNativeAnonymous() || - !aContent->GetCurrentDoc() || - (aContent->GetParent() && - aContent->GetParent()->NodeInfo()-> - Equals(nsGkAtoms::use, kNameSpaceID_SVG)), - "Native anonymous node with wrong binding parent"); - aContent = bindingParent; - } - - return PR_FALSE; -} - /* static */ nsIWidget* nsContentUtils::GetTopLevelWidget(nsIWidget* aWidget) diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp index ee15f8022b3..1f63bceae90 100644 --- a/content/base/src/nsDocument.cpp +++ b/content/base/src/nsDocument.cpp @@ -5761,7 +5761,7 @@ nsDocument::MutationEventDispatched(nsINode* aTarget) for (PRInt32 i = 0; i < count; ++i) { nsINode* possibleTarget = mSubtreeModifiedTargets[i]; nsCOMPtr content = do_QueryInterface(possibleTarget); - if (content && content->IsNativeAnonymous()) { + if (content && content->IsInNativeAnonymousSubtree()) { if (realTargets.IndexOf(possibleTarget) == -1) { realTargets.AppendObject(possibleTarget); } diff --git a/content/base/src/nsGenericElement.cpp b/content/base/src/nsGenericElement.cpp index 4c51abc8ccb..778aa88c12a 100644 --- a/content/base/src/nsGenericElement.cpp +++ b/content/base/src/nsGenericElement.cpp @@ -427,6 +427,37 @@ nsIContent::UpdateEditableState() SetEditableFlag(parent && parent->HasFlag(NODE_IS_EDITABLE)); } +nsIContent* +nsIContent::FindFirstNonNativeAnonymous() const +{ + // This handles also nested native anonymous content. + nsIContent* content = GetBindingParent(); + nsIContent* possibleResult = + !IsNativeAnonymous() ? const_cast(this) : nsnull; + while (content) { + if (content->IsNativeAnonymous()) { + content = possibleResult = content->GetParent(); + } else { + content = content->GetBindingParent(); + } + } + + return possibleResult; +} + +PRBool +nsIContent::IsInNativeAnonymousSubtree() const +{ + nsIContent* content = GetBindingParent(); + while (content) { + if (content->IsNativeAnonymous()) { + return PR_TRUE; + } + content = content->GetBindingParent(); + } + return PR_FALSE; +} + //---------------------------------------------------------------------- nsChildContentList::~nsChildContentList() @@ -2073,12 +2104,12 @@ nsGenericElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, NS_PRECONDITION(!aParent || !aDocument || !aParent->HasFlag(NODE_FORCE_XBL_BINDINGS), "Parent in document but flagged as forcing XBL"); - // XXXbz XUL's SetNativeAnonymous is all weird, so can't assert - // anything here - NS_PRECONDITION(IsNodeOfType(eXUL) || - aBindingParent != this || IsNativeAnonymous(), + NS_PRECONDITION(aBindingParent != this || IsNativeAnonymous(), "Only native anonymous content should have itself as its " "own binding parent"); + NS_PRECONDITION(!IsNativeAnonymous() || aBindingParent == this, + "Native anonymous content must have itself as its " + "own binding parent"); if (!aBindingParent && aParent) { aBindingParent = aParent->GetBindingParent(); @@ -2272,24 +2303,6 @@ nsGenericElement::PreHandleEvent(nsEventChainPreVisitor& aVisitor) return nsGenericElement::doPreHandleEvent(this, aVisitor); } -static nsIContent* -FindFirstNonAnonContent(nsIContent* aContent) -{ - while (aContent && aContent->IsNativeAnonymous()) { - aContent = aContent->GetParent(); - } - return aContent; -} - -static PRBool -IsInAnonContent(nsIContent* aContent) -{ - while (aContent && !aContent->IsNativeAnonymous()) { - aContent = aContent->GetParent(); - } - return !!aContent; -} - nsresult nsGenericElement::doPreHandleEvent(nsIContent* aContent, nsEventChainPreVisitor& aVisitor) @@ -2315,10 +2328,12 @@ nsGenericElement::doPreHandleEvent(nsIContent* aContent, // must be updated. if (isAnonForEvents || aVisitor.mRelatedTargetIsInAnon || (aVisitor.mEvent->originalTarget == aContent && - (aVisitor.mRelatedTargetIsInAnon = IsInAnonContent(relatedTarget)))) { - nsIContent* nonAnon = FindFirstNonAnonContent(aContent); + (aVisitor.mRelatedTargetIsInAnon = + relatedTarget->IsInNativeAnonymousSubtree()))) { + nsIContent* nonAnon = aContent->FindFirstNonNativeAnonymous(); if (nonAnon) { - nsIContent* nonAnonRelated = FindFirstNonAnonContent(relatedTarget); + nsIContent* nonAnonRelated = + relatedTarget->FindFirstNonNativeAnonymous(); if (nonAnonRelated) { if (nonAnon == nonAnonRelated || nsContentUtils::ContentIsDescendantOf(nonAnonRelated, nonAnon)) { diff --git a/content/base/src/nsStyledElement.cpp b/content/base/src/nsStyledElement.cpp index 1aaf7872c50..e4fb82c9312 100644 --- a/content/base/src/nsStyledElement.cpp +++ b/content/base/src/nsStyledElement.cpp @@ -237,8 +237,8 @@ nsStyledElement::ParseStyleAttribute(nsIContent* aContent, if (doc && (aForceInDataDoc || !doc->IsLoadedAsData())) { PRBool isCSS = PR_TRUE; // assume CSS until proven otherwise - if (!aContent->IsNativeAnonymous()) { // native anonymous content - // always assumes CSS + if (!aContent->IsInNativeAnonymousSubtree()) { // native anonymous content + // always assumes CSS nsAutoString styleType; doc->GetHeaderData(nsGkAtoms::headerContentStyleType, styleType); if (!styleType.IsEmpty()) { diff --git a/content/events/src/nsEventListenerManager.cpp b/content/events/src/nsEventListenerManager.cpp index ae63d1bae75..b963cbb54bf 100644 --- a/content/events/src/nsEventListenerManager.cpp +++ b/content/events/src/nsEventListenerManager.cpp @@ -1468,29 +1468,31 @@ nsEventListenerManager::PrepareToUseCaretPosition(nsIWidget* aEventWidget, NS_ENSURE_SUCCESS(rv, PR_FALSE); NS_ENSURE_TRUE(node, PR_FALSE); nsCOMPtr content(do_QueryInterface(node)); - for ( ; content; content = content->GetParent()) { - if (!content->IsNativeAnonymous()) { - // It seems like selCon->ScrollSelectionIntoView should be enough, but it's - // not. The problem is that scrolling the selection into view when it is - // below the current viewport will align the top line of the frame exactly - // with the bottom of the window. This is fine, BUT, the popup event causes - // the control to be re-focused which does this exact call to - // ScrollContentIntoView, which has a one-pixel disagreement of whether the - // frame is actually in view. The result is that the frame is aligned with - // the top of the window, but the menu is still at the bottom. - // - // Doing this call first forces the frame to be in view, eliminating the - // problem. The only difference in the result is that if your cursor is in - // an edit box below the current view, you'll get the edit box aligned with - // the top of the window. This is arguably better behavior anyway. - rv = aShell->ScrollContentIntoView(content, - NS_PRESSHELL_SCROLL_IF_NOT_VISIBLE, - NS_PRESSHELL_SCROLL_IF_NOT_VISIBLE); - NS_ENSURE_SUCCESS(rv, PR_FALSE); - frame = aShell->GetPrimaryFrameFor(content); - NS_ASSERTION(frame, "No frame for focused content?"); - break; - } + if (content) { + nsIContent* nonNative = content->FindFirstNonNativeAnonymous(); + content = nonNative; + } + + if (content) { + // It seems like selCon->ScrollSelectionIntoView should be enough, but it's + // not. The problem is that scrolling the selection into view when it is + // below the current viewport will align the top line of the frame exactly + // with the bottom of the window. This is fine, BUT, the popup event causes + // the control to be re-focused which does this exact call to + // ScrollContentIntoView, which has a one-pixel disagreement of whether the + // frame is actually in view. The result is that the frame is aligned with + // the top of the window, but the menu is still at the bottom. + // + // Doing this call first forces the frame to be in view, eliminating the + // problem. The only difference in the result is that if your cursor is in + // an edit box below the current view, you'll get the edit box aligned with + // the top of the window. This is arguably better behavior anyway. + rv = aShell->ScrollContentIntoView(content, + NS_PRESSHELL_SCROLL_IF_NOT_VISIBLE, + NS_PRESSHELL_SCROLL_IF_NOT_VISIBLE); + NS_ENSURE_SUCCESS(rv, PR_FALSE); + frame = aShell->GetPrimaryFrameFor(content); + NS_WARN_IF_FALSE(frame, "No frame for focused content?"); } // Actually scroll the selection (ie caret) into view. Note that this must diff --git a/content/xbl/src/nsBindingManager.cpp b/content/xbl/src/nsBindingManager.cpp index aba4faf1ca4..ae3ba028568 100644 --- a/content/xbl/src/nsBindingManager.cpp +++ b/content/xbl/src/nsBindingManager.cpp @@ -1280,9 +1280,7 @@ nsBindingManager::WalkRules(nsStyleSet* aStyleSet, nsIContent* parent = content->GetBindingParent(); if (parent == content) { - NS_ASSERTION(content->IsNativeAnonymous() || - content->IsNodeOfType(nsINode::eXUL), - "Unexpected binding parent"); + NS_ASSERTION(content->IsNativeAnonymous(), "Unexpected binding parent"); break; // The anonymous content case is often deliberately hacked to // return itself to cut off style inheritance here. Do that. diff --git a/embedding/components/find/src/nsFind.cpp b/embedding/components/find/src/nsFind.cpp index 23a456bfc85..256a4fadc55 100644 --- a/embedding/components/find/src/nsFind.cpp +++ b/embedding/components/find/src/nsFind.cpp @@ -282,24 +282,16 @@ nsFindContentIterator::Reset() // see if the start node is an anonymous text node inside a text control nsCOMPtr startContent(do_QueryInterface(mStartNode)); - for ( ; startContent; startContent = startContent->GetParent()) { - if (!startContent->IsNativeAnonymous() && - (!startContent->GetBindingParent() || - !startContent->GetBindingParent()->IsNativeAnonymous())) { - mStartOuterNode = do_QueryInterface(startContent); - break; - } + if (startContent) { + mStartOuterNode = + do_QueryInterface(startContent->FindFirstNonNativeAnonymous()); } // see if the end node is an anonymous text node inside a text control nsCOMPtr endContent(do_QueryInterface(mEndNode)); - for ( ; endContent; endContent = endContent->GetParent()) { - if (!endContent->IsNativeAnonymous() && - (!endContent->GetBindingParent() || - !endContent->GetBindingParent()->IsNativeAnonymous())) { - mEndOuterNode = do_QueryInterface(endContent); - break; - } + if (endContent) { + mEndOuterNode = + do_QueryInterface(endContent->FindFirstNonNativeAnonymous()); } // Note: OK to just set up the outer iterator here; if our range has a native diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index d853cbe4a87..b42b2a69b6a 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -5743,16 +5743,18 @@ nsCSSFrameConstructor::CreateAnonymousFrames(nsFrameConstructorState& aState, nsIContent* content = newAnonymousItems[i]; NS_ASSERTION(content, "null anonymous content?"); - content->SetNativeAnonymous(); - nsIContent* bindingParent = content; #ifdef MOZ_SVG // least-surprise CSS binding until we do the SVG specified // cascading rules for - bug 265894 if (aParent && - aParent->NodeInfo()->Equals(nsGkAtoms::use, kNameSpaceID_SVG)) + aParent->NodeInfo()->Equals(nsGkAtoms::use, kNameSpaceID_SVG)) { bindingParent = aParent; + } else #endif + { + content->SetNativeAnonymous(); + } rv = content->BindToTree(aDocument, aParent, bindingParent, PR_TRUE); if (NS_FAILED(rv)) { diff --git a/layout/style/nsStyleSet.cpp b/layout/style/nsStyleSet.cpp index 71b4583ca9f..b08c21fb7e8 100644 --- a/layout/style/nsStyleSet.cpp +++ b/layout/style/nsStyleSet.cpp @@ -509,7 +509,8 @@ nsStyleSet::FileRules(nsIStyleRuleProcessor::EnumFunc aCollectorFunc, nsRuleNode* lastPresHintRN = mRuleWalker->GetCurrentNode(); mRuleWalker->SetLevel(eUserSheet, PR_FALSE); - PRBool skipUserStyles = nsContentUtils::IsNativeAnonymous(aData->mContent); + PRBool skipUserStyles = + aData->mContent && aData->mContent->IsInNativeAnonymousSubtree(); if (!skipUserStyles && mRuleProcessors[eUserSheet]) // NOTE: different (*aCollectorFunc)(mRuleProcessors[eUserSheet], aData); nsRuleNode* lastUserRN = mRuleWalker->GetCurrentNode(); @@ -573,7 +574,8 @@ nsStyleSet::WalkRuleProcessors(nsIStyleRuleProcessor::EnumFunc aFunc, if (mRuleProcessors[ePresHintSheet]) (*aFunc)(mRuleProcessors[ePresHintSheet], aData); - PRBool skipUserStyles = nsContentUtils::IsNativeAnonymous(aData->mContent); + PRBool skipUserStyles = + aData->mContent && aData->mContent->IsInNativeAnonymousSubtree(); if (!skipUserStyles && mRuleProcessors[eUserSheet]) // NOTE: different (*aFunc)(mRuleProcessors[eUserSheet], aData); diff --git a/layout/xul/base/src/nsBoxFrame.cpp b/layout/xul/base/src/nsBoxFrame.cpp index 28d08bbd597..9a29bfaf0e0 100644 --- a/layout/xul/base/src/nsBoxFrame.cpp +++ b/layout/xul/base/src/nsBoxFrame.cpp @@ -616,7 +616,7 @@ nsBoxFrame::DidReflow(nsPresContext* aPresContext, PRBool nsBoxFrame::HonorPrintBackgroundSettings() { - return (!mContent || !nsContentUtils::IsNativeAnonymous(mContent)) && + return (!mContent || !mContent->IsInNativeAnonymousSubtree()) && nsContainerFrame::HonorPrintBackgroundSettings(); }