From 1347910ac900af9e60771e67d746d79a3b6f29d5 Mon Sep 17 00:00:00 2001 From: "L. David Baron" Date: Tue, 22 Jul 2008 21:50:20 -0700 Subject: [PATCH] Change the binding parent for native anonymous content to work like the binding parent for XBL anonymous content, and be the parent of the anonymous content subtree rather than the root of the anonymous content subtree. (Bug 436453) r=bzbarsky,smaug,surkov sr=bzbarsky --- accessible/src/base/nsAccessNode.cpp | 3 +- accessible/src/base/nsAccessibilityUtils.cpp | 2 +- accessible/src/base/nsAccessible.cpp | 1 + accessible/src/base/nsDocAccessible.cpp | 5 +- accessible/src/html/nsHyperTextAccessible.cpp | 4 +- content/base/public/nsIContent.h | 38 ++++++++++++-- content/base/src/nsContentUtils.cpp | 2 +- content/base/src/nsDocument.cpp | 4 +- content/base/src/nsGenericDOMDataNode.cpp | 14 ++++-- content/base/src/nsGenericElement.cpp | 37 +++++++------- content/events/src/nsDOMEvent.cpp | 6 +-- content/xbl/src/nsBindingManager.cpp | 10 ++-- content/xbl/src/nsXBLService.cpp | 7 ++- content/xul/content/src/nsXULElement.cpp | 1 + editor/libeditor/base/nsEditor.cpp | 4 +- .../libeditor/html/nsHTMLAnonymousUtils.cpp | 2 +- editor/libeditor/html/nsHTMLEditor.cpp | 2 +- embedding/components/find/src/nsFind.cpp | 2 +- .../components/find/src/nsWebBrowserFind.cpp | 17 +------ layout/base/nsCSSFrameConstructor.cpp | 50 ++++++++++--------- layout/base/nsPresShell.cpp | 2 +- layout/generic/nsBRFrame.cpp | 6 ++- layout/generic/nsFrame.cpp | 5 +- layout/inspector/src/inDOMView.cpp | 2 +- 24 files changed, 120 insertions(+), 106 deletions(-) diff --git a/accessible/src/base/nsAccessNode.cpp b/accessible/src/base/nsAccessNode.cpp index 6e2221eaf7e7..84c0a61de161 100755 --- a/accessible/src/base/nsAccessNode.cpp +++ b/accessible/src/base/nsAccessNode.cpp @@ -191,8 +191,7 @@ NS_IMETHODIMP nsAccessNode::Init() // so that nsDocAccessible::RefreshNodes() can find the anonymous subtree to release when // the root node goes away nsCOMPtr content = do_QueryInterface(mDOMNode); - if (content && (content->IsNativeAnonymous() || - content->GetBindingParent())) { + if (content && content->IsInAnonymousSubtree()) { // Specific examples of where this is used: and nsCOMPtr parentAccessible; docAccessible->GetAccessibleInParentChain(mDOMNode, PR_TRUE, getter_AddRefs(parentAccessible)); diff --git a/accessible/src/base/nsAccessibilityUtils.cpp b/accessible/src/base/nsAccessibilityUtils.cpp index d9f111b792d7..6e186316a946 100755 --- a/accessible/src/base/nsAccessibilityUtils.cpp +++ b/accessible/src/base/nsAccessibilityUtils.cpp @@ -796,7 +796,7 @@ nsAccUtils::FindNeighbourPointingToNode(nsIContent *aForNode, nsAutoString controlID; if (!nsAccUtils::GetID(aForNode, controlID)) { binding = aForNode->GetBindingParent(); - if (binding == aForNode) + if (aForNode->IsRootOfNativeAnonymousSubtree()) // XXX Was this the intent? return nsnull; aForNode->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::anonid, controlID); diff --git a/accessible/src/base/nsAccessible.cpp b/accessible/src/base/nsAccessible.cpp index 99b992c00647..bf3c96299122 100644 --- a/accessible/src/base/nsAccessible.cpp +++ b/accessible/src/base/nsAccessible.cpp @@ -1941,6 +1941,7 @@ nsresult nsAccessible::GetXULName(nsAString& aLabel, PRBool aCanAggregateSubtree // Can get text from title of if we're a child of a nsIContent *bindingParent = content->GetBindingParent(); + // XXXldb: Why do we skip to bindingParent's parent? nsIContent *parent = bindingParent? bindingParent->GetParent() : content->GetParent(); while (parent) { diff --git a/accessible/src/base/nsDocAccessible.cpp b/accessible/src/base/nsDocAccessible.cpp index 4752e25e6042..da82c1c52c3d 100644 --- a/accessible/src/base/nsDocAccessible.cpp +++ b/accessible/src/base/nsDocAccessible.cpp @@ -1760,9 +1760,8 @@ void nsDocAccessible::RefreshNodes(nsIDOMNode *aStartNode) getter_AddRefs(childAccessNode)); childAccessNode->GetDOMNode(getter_AddRefs(possibleAnonNode)); nsCOMPtr iterContent = do_QueryInterface(possibleAnonNode); - if (iterContent && (iterContent->IsNativeAnonymous() || - iterContent->GetBindingParent())) { - // GetBindingParent() check is a perf win -- make sure we don't + if (iterContent && iterContent->IsInAnonymousSubtree()) { + // IsInAnonymousSubtree() check is a perf win -- make sure we don't // shut down the same subtree twice since we'll reach non-anon content via // DOM traversal later in this method RefreshNodes(possibleAnonNode); diff --git a/accessible/src/html/nsHyperTextAccessible.cpp b/accessible/src/html/nsHyperTextAccessible.cpp index 04626d2735eb..02d4be2b4a3f 100644 --- a/accessible/src/html/nsHyperTextAccessible.cpp +++ b/accessible/src/html/nsHyperTextAccessible.cpp @@ -640,7 +640,9 @@ nsresult nsHyperTextAccessible::DOMPointToHypertextOffset(nsIDOMNode* aNode, PRI if (findContent->IsNodeOfType(nsINode::eHTML) && findContent->NodeInfo()->Equals(nsAccessibilityAtoms::br)) { nsIContent *parent = findContent->GetParent(); - if (parent && parent->IsNativeAnonymous() && parent->GetChildCount() == 1) { + if (parent && + parent->IsRootOfNativeAnonymousSubtree() && + parent->GetChildCount() == 1) { // This
is the only node in a text control, therefore it is the hacky // "bogus node" used when there is no text in a control *aHyperTextOffset = 0; diff --git a/content/base/public/nsIContent.h b/content/base/public/nsIContent.h index f6be28e75c67..01efdd5e81b4 100644 --- a/content/base/public/nsIContent.h +++ b/content/base/public/nsIContent.h @@ -146,7 +146,7 @@ public: * @see nsIAnonymousContentCreator * @return whether this content is anonymous */ - PRBool IsNativeAnonymous() const + PRBool IsRootOfNativeAnonymousSubtree() const { return HasFlag(NODE_IS_ANONYMOUS); } @@ -177,7 +177,7 @@ public: } nsIContent* content = GetBindingParent(); while (content) { - if (content->IsNativeAnonymous()) { + if (content->IsRootOfNativeAnonymousSubtree()) { NS_ERROR("Element not marked to be in native anonymous subtree!"); break; } @@ -189,6 +189,31 @@ public: #endif } + /** + * Returns true if and only if this node has a parent, but is not in + * its parent's child list. + */ + PRBool IsRootOfAnonymousSubtree() const + { + NS_ASSERTION(!IsRootOfNativeAnonymousSubtree() || + (GetParent() && GetBindingParent() == GetParent()), + "root of native anonymous subtree must have parent equal " + "to binding parent"); + nsIContent *bindingParent = GetBindingParent(); + return bindingParent && bindingParent == GetParent(); + } + + /** + * Returns true if and only if there is NOT a path through child lists + * from the top of this node's parent chain back to this node. + */ + PRBool IsInAnonymousSubtree() const + { + NS_ASSERTION(!IsInNativeAnonymousSubtree() || GetBindingParent(), + "must have binding parent when in native anonymous subtree"); + return GetBindingParent() != nsnull; + } + /** * Get the namespace that this element's tag is defined in * @return the namespace @@ -563,9 +588,12 @@ public: } /** - * Gets content node with the binding responsible for our construction (and - * existence). Used by anonymous content (XBL-generated). null for all - * explicit content. + * Gets content node with the binding (or native code, possibly on the + * frame) responsible for our construction (and existence). Used by + * anonymous content (both XBL-generated and native-anonymous). + * + * null for all explicit content (i.e., content reachable from the top + * of its GetParent() chain via child lists). * * @return the binding parent */ diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp index e29f9bcf092d..d03640f72ffe 100644 --- a/content/base/src/nsContentUtils.cpp +++ b/content/base/src/nsContentUtils.cpp @@ -1890,7 +1890,7 @@ nsContentUtils::GenerateStateKey(nsIContent* aContent, NS_ENSURE_TRUE(aContent, NS_ERROR_FAILURE); // Don't capture state for anonymous content - if (aContent->IsNativeAnonymous() || aContent->GetBindingParent()) { + if (aContent->IsInAnonymousSubtree()) { return NS_OK; } diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp index c2d1352fe5e3..5cb783b2f102 100644 --- a/content/base/src/nsDocument.cpp +++ b/content/base/src/nsDocument.cpp @@ -2145,8 +2145,8 @@ nsDocument::ElementFromPoint(PRInt32 aX, PRInt32 aY, nsIDOMElement** aReturn) // replace it with the first non-anonymous parent node of type element. while (ptContent && !ptContent->IsNodeOfType(nsINode::eELEMENT) || - ptContent->GetBindingParent() || - ptContent->IsNativeAnonymous()) { + ptContent->IsInAnonymousSubtree()) { + // XXXldb: Faster to jump to GetBindingParent if non-null? ptContent = ptContent->GetParent(); } diff --git a/content/base/src/nsGenericDOMDataNode.cpp b/content/base/src/nsGenericDOMDataNode.cpp index b8f0695c2a7b..9f230097e9dd 100644 --- a/content/base/src/nsGenericDOMDataNode.cpp +++ b/content/base/src/nsGenericDOMDataNode.cpp @@ -592,8 +592,11 @@ nsGenericDOMDataNode::BindToTree(nsIDocument* aDocument, nsIContent* aParent, (!aBindingParent && aParent && aParent->GetBindingParent() == GetBindingParent()), "Already have a binding parent. Unbind first!"); - NS_PRECONDITION(aBindingParent != this || IsNativeAnonymous(), - "Only native anonymous content should have itself as its " + NS_PRECONDITION(aBindingParent != this, + "Content must not be its own binding parent"); + NS_PRECONDITION(!IsRootOfNativeAnonymousSubtree() || + aBindingParent == aParent, + "Native anonymous content must have its parent as its " "own binding parent"); if (!aBindingParent && aParent) { @@ -605,13 +608,14 @@ nsGenericDOMDataNode::BindToTree(nsIDocument* aDocument, nsIContent* aParent, nsDataSlots *slots = GetDataSlots(); NS_ENSURE_TRUE(slots, NS_ERROR_OUT_OF_MEMORY); - NS_ASSERTION(IsNativeAnonymous() || !HasFlag(NODE_IS_IN_ANONYMOUS_SUBTREE) || + NS_ASSERTION(IsRootOfNativeAnonymousSubtree() || + !HasFlag(NODE_IS_IN_ANONYMOUS_SUBTREE) || aBindingParent->IsInNativeAnonymousSubtree(), "Trying to re-bind content from native anonymous subtree to" "non-native anonymous parent!"); slots->mBindingParent = aBindingParent; // Weak, so no addref happens. - if (IsNativeAnonymous() || - aBindingParent->IsInNativeAnonymousSubtree()) { + if (IsRootOfNativeAnonymousSubtree() || + aParent->IsInNativeAnonymousSubtree()) { SetFlags(NODE_IS_IN_ANONYMOUS_SUBTREE); } } diff --git a/content/base/src/nsGenericElement.cpp b/content/base/src/nsGenericElement.cpp index deab1aea64b0..a5896ea9de1c 100644 --- a/content/base/src/nsGenericElement.cpp +++ b/content/base/src/nsGenericElement.cpp @@ -435,18 +435,15 @@ 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(); + for (const nsIContent *content = this; content; + content = content->GetBindingParent()) { + if (!content->IsInNativeAnonymousSubtree()) { + // Oops, this function signature allows casting const to + // non-const. (Then again, so does GetChildAt(0)->GetParent().) + return const_cast(content); } } - - return possibleResult; + return nsnull; } //---------------------------------------------------------------------- @@ -2091,11 +2088,11 @@ nsGenericElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, NS_PRECONDITION(!aParent || !aDocument || !aParent->HasFlag(NODE_FORCE_XBL_BINDINGS), "Parent in document but flagged as forcing XBL"); - 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 " + NS_PRECONDITION(aBindingParent != this, + "Content must not be its own binding parent"); + NS_PRECONDITION(!IsRootOfNativeAnonymousSubtree() || + aBindingParent == aParent, + "Native anonymous content must have its parent as its " "own binding parent"); if (!aBindingParent && aParent) { @@ -2121,13 +2118,13 @@ nsGenericElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, slots->mBindingParent = aBindingParent; // Weak, so no addref happens. } } - NS_ASSERTION(!aBindingParent || IsNativeAnonymous() || + NS_ASSERTION(!aBindingParent || IsRootOfNativeAnonymousSubtree() || !HasFlag(NODE_IS_IN_ANONYMOUS_SUBTREE) || aBindingParent->IsInNativeAnonymousSubtree(), "Trying to re-bind content from native anonymous subtree to" "non-native anonymous parent!"); - if (IsNativeAnonymous() || - aBindingParent && aBindingParent->IsInNativeAnonymousSubtree()) { + if (IsRootOfNativeAnonymousSubtree() || + aParent && aParent->IsInNativeAnonymousSubtree()) { SetFlags(NODE_IS_IN_ANONYMOUS_SUBTREE); } @@ -2302,7 +2299,7 @@ FindNativeAnonymousSubtreeOwner(nsIContent* aContent) if (aContent->IsInNativeAnonymousSubtree()) { PRBool isNativeAnon = PR_FALSE; while (aContent && !isNativeAnon) { - isNativeAnon = aContent->IsNativeAnonymous(); + isNativeAnon = aContent->IsRootOfNativeAnonymousSubtree(); aContent = aContent->GetParent(); } } @@ -2318,7 +2315,7 @@ nsGenericElement::doPreHandleEvent(nsIContent* aContent, // Don't propagate mouseover and mouseout events when mouse is moving // inside native anonymous content. - PRBool isAnonForEvents = aContent->IsNativeAnonymous(); + PRBool isAnonForEvents = aContent->IsRootOfNativeAnonymousSubtree(); if ((aVisitor.mEvent->message == NS_MOUSE_ENTER_SYNTH || aVisitor.mEvent->message == NS_MOUSE_EXIT_SYNTH) && // This is an optimization - try to stop event propagation when diff --git a/content/events/src/nsDOMEvent.cpp b/content/events/src/nsDOMEvent.cpp index 29a9212dab60..d8a53fd19089 100644 --- a/content/events/src/nsDOMEvent.cpp +++ b/content/events/src/nsDOMEvent.cpp @@ -134,10 +134,8 @@ nsDOMEvent::nsDOMEvent(nsPresContext* aPresContext, nsEvent* aEvent) mExplicitOriginalTarget = GetTargetFromFrame(); mTmpRealOriginalTarget = mExplicitOriginalTarget; nsCOMPtr content = do_QueryInterface(mExplicitOriginalTarget); - if (content) { - if (content->IsNativeAnonymous() || content->GetBindingParent()) { - mExplicitOriginalTarget = nsnull; - } + if (content && content->IsInAnonymousSubtree()) { + mExplicitOriginalTarget = nsnull; } } } diff --git a/content/xbl/src/nsBindingManager.cpp b/content/xbl/src/nsBindingManager.cpp index 705078554399..c383f61f7d8c 100644 --- a/content/xbl/src/nsBindingManager.cpp +++ b/content/xbl/src/nsBindingManager.cpp @@ -1263,15 +1263,11 @@ nsBindingManager::WalkRules(nsStyleSet* aStyleSet, } } - nsIContent* parent = content->GetBindingParent(); - if (parent == content) { - 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. + if (content->IsRootOfNativeAnonymousSubtree()) { + break; // Deliberately cut off style inheritance here. } - content = parent; + content = content->GetBindingParent(); } while (content); // If "content" is non-null that means we cut off inheritance at some point diff --git a/content/xbl/src/nsXBLService.cpp b/content/xbl/src/nsXBLService.cpp index c1c6cb923cfd..44336015edff 100644 --- a/content/xbl/src/nsXBLService.cpp +++ b/content/xbl/src/nsXBLService.cpp @@ -110,11 +110,10 @@ IsAncestorBinding(nsIDocument* aDocument, NS_ASSERTION(aChild, "expected a child content"); PRUint32 bindingRecursion = 0; - nsIContent* bindingParent = aChild->GetBindingParent(); nsBindingManager* bindingManager = aDocument->BindingManager(); - for (nsIContent* prev = aChild; - bindingParent && prev != bindingParent; - prev = bindingParent, bindingParent = bindingParent->GetBindingParent()) { + for (nsIContent *bindingParent = aChild->GetBindingParent(); + bindingParent; + bindingParent = bindingParent->GetBindingParent()) { nsXBLBinding* binding = bindingManager->GetBinding(bindingParent); if (!binding) { continue; diff --git a/content/xul/content/src/nsXULElement.cpp b/content/xul/content/src/nsXULElement.cpp index 1e15a6e39f31..bd8d8ac58346 100644 --- a/content/xul/content/src/nsXULElement.cpp +++ b/content/xul/content/src/nsXULElement.cpp @@ -1086,6 +1086,7 @@ nsXULElement::UnregisterAccessKey(const nsAString& aOldValue) if (mNodeInfo->Equals(nsGkAtoms::label)) { // For anonymous labels the unregistering must // occur on the binding parent control. + // XXXldb: And what if the binding parent is null? content = GetBindingParent(); } diff --git a/editor/libeditor/base/nsEditor.cpp b/editor/libeditor/base/nsEditor.cpp index 52516df0d828..b437e1e2991f 100644 --- a/editor/libeditor/base/nsEditor.cpp +++ b/editor/libeditor/base/nsEditor.cpp @@ -440,7 +440,7 @@ nsEditor::GetDesiredSpellCheckState() return PR_FALSE; } - if (content->IsNativeAnonymous()) { + if (content->IsRootOfNativeAnonymousSubtree()) { content = content->GetParent(); } @@ -5241,7 +5241,7 @@ nsEditor::GetPIDOMEventTarget() nsCOMPtr content = do_QueryInterface(rootElement); - if (content && content->IsNativeAnonymous()) + if (content && content->IsRootOfNativeAnonymousSubtree()) { mEventTarget = do_QueryInterface(content->GetParent()); piTarget = mEventTarget; diff --git a/editor/libeditor/html/nsHTMLAnonymousUtils.cpp b/editor/libeditor/html/nsHTMLAnonymousUtils.cpp index 8ddd54bf4c63..0f8bcee953d3 100644 --- a/editor/libeditor/html/nsHTMLAnonymousUtils.cpp +++ b/editor/libeditor/html/nsHTMLAnonymousUtils.cpp @@ -188,7 +188,7 @@ nsHTMLEditor::CreateAnonymousElement(const nsAString & aTag, nsIDOMNode * aPare // establish parenthood of the element newContent->SetNativeAnonymous(); - res = newContent->BindToTree(doc, parentContent, newContent, PR_TRUE); + res = newContent->BindToTree(doc, parentContent, parentContent, PR_TRUE); if (NS_FAILED(res)) { newContent->UnbindFromTree(); return res; diff --git a/editor/libeditor/html/nsHTMLEditor.cpp b/editor/libeditor/html/nsHTMLEditor.cpp index cb408c94bd0f..e30b8bbe2f91 100644 --- a/editor/libeditor/html/nsHTMLEditor.cpp +++ b/editor/libeditor/html/nsHTMLEditor.cpp @@ -5928,7 +5928,7 @@ nsHTMLEditor::IsAnonymousElement(nsIDOMElement * aElement, PRBool * aReturn) { NS_ENSURE_TRUE(aElement, NS_ERROR_NULL_POINTER); nsCOMPtr content = do_QueryInterface(aElement); - *aReturn = content->IsNativeAnonymous(); + *aReturn = content->IsRootOfNativeAnonymousSubtree(); return NS_OK; } diff --git a/embedding/components/find/src/nsFind.cpp b/embedding/components/find/src/nsFind.cpp index 86a4e05b6888..ae9b498c2f32 100644 --- a/embedding/components/find/src/nsFind.cpp +++ b/embedding/components/find/src/nsFind.cpp @@ -366,7 +366,7 @@ nsFindContentIterator::SetupInnerIterator(nsIContent* aContent) if (!aContent) { return; } - NS_ASSERTION(!aContent->IsNativeAnonymous(), "invalid call"); + NS_ASSERTION(!aContent->IsRootOfNativeAnonymousSubtree(), "invalid call"); nsIDocument* doc = aContent->GetDocument(); nsIPresShell* shell = doc ? doc->GetPrimaryShell() : nsnull; diff --git a/embedding/components/find/src/nsWebBrowserFind.cpp b/embedding/components/find/src/nsWebBrowserFind.cpp index ee901f267f82..11ed7e5694d3 100644 --- a/embedding/components/find/src/nsWebBrowserFind.cpp +++ b/embedding/components/find/src/nsWebBrowserFind.cpp @@ -411,21 +411,6 @@ FocusElementButNotDocument(nsIDocument* aDocument, nsIContent* aContent) esm->SetFocusedContent(nsnull); } -static PRBool -IsInNativeAnonymousSubtree(nsIContent* aContent) -{ - while (aContent) { - nsIContent* bindingParent = aContent->GetBindingParent(); - if (bindingParent == aContent) { - return PR_TRUE; - } - - aContent = bindingParent; - } - - return PR_FALSE; -} - void nsWebBrowserFind::SetSelectionAndScroll(nsIDOMWindow* aWindow, nsIDOMRange* aRange) { @@ -451,7 +436,7 @@ void nsWebBrowserFind::SetSelectionAndScroll(nsIDOMWindow* aWindow, //