From 1110bedb45d48a43b88a666f15fe8d766d6bcc6f Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Fri, 21 Feb 2020 02:47:05 +0000 Subject: [PATCH] Bug 1613830 - Add `nsINode::GetAsElementOrParentElement()` r=smaug This patch assumes that only element node can have content node. I.e., we won't hit the following `MOZ_ASSERT`: ``` Element* element = nullptr; nsIContent* content = aContent; while (content) { if (content->IsElement()) { element = content->AsElement(); break; } content = content->GetParent(); } MOZ_ASSERT(!content || content == element || content->GetParent() == element); ``` Differential Revision: https://phabricator.services.mozilla.com/D63308 --HG-- extra : moz-landing-system : lando --- dom/base/Element.h | 5 ++ dom/base/FragmentOrElement.cpp | 18 +++--- dom/base/nsContentUtils.cpp | 32 +++-------- dom/base/nsINode.h | 6 ++ editor/libeditor/HTMLEditor.cpp | 64 ++++++++++----------- editor/libeditor/HTMLEditorDataTransfer.cpp | 19 +++--- gfx/layers/apz/util/DoubleTapToZoom.cpp | 9 ++- layout/base/TouchManager.cpp | 14 ++--- layout/generic/nsFrame.cpp | 5 +- widget/windows/nsWindow.cpp | 19 +++--- 10 files changed, 84 insertions(+), 107 deletions(-) diff --git a/dom/base/Element.h b/dom/base/Element.h index 93ed88f54108..df39271dcbe0 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -1996,6 +1996,11 @@ inline mozilla::dom::Element* nsINode::GetPreviousElementSibling() const { return nullptr; } +inline mozilla::dom::Element* nsINode::GetAsElementOrParentElement() const { + return IsElement() ? const_cast(AsElement()) + : GetParentElement(); +} + inline mozilla::dom::Element* nsINode::GetNextElementSibling() const { nsIContent* nextSibling = GetNextSibling(); while (nextSibling) { diff --git a/dom/base/FragmentOrElement.cpp b/dom/base/FragmentOrElement.cpp index 1c9a226a9bd6..6235c69bf566 100644 --- a/dom/base/FragmentOrElement.cpp +++ b/dom/base/FragmentOrElement.cpp @@ -276,22 +276,18 @@ nsresult nsIContent::LookupNamespaceURIInternal( } // Trace up the content parent chain looking for the namespace // declaration that declares aNamespacePrefix. - const nsIContent* content = this; - do { - if (content->IsElement() && - content->AsElement()->GetAttr(kNameSpaceID_XMLNS, name, aNamespaceURI)) + for (Element* element = GetAsElementOrParentElement(); element; + element = element->GetParentElement()) { + if (element->GetAttr(kNameSpaceID_XMLNS, name, aNamespaceURI)) { return NS_OK; - } while ((content = content->GetParent())); + } + } return NS_ERROR_FAILURE; } nsAtom* nsIContent::GetLang() const { - for (const auto* content = this; content; content = content->GetParent()) { - if (!content->IsElement()) { - continue; - } - - auto* element = content->AsElement(); + for (const Element* element = GetAsElementOrParentElement(); element; + element = element->GetParentElement()) { if (!element->GetAttrCount()) { continue; } diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index c539730f0065..aa7c7403ce34 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -4667,19 +4667,11 @@ already_AddRefed nsContentUtils::CreateContextualFragment( RefPtr frag = new DocumentFragment(document->NodeInfoManager()); - nsCOMPtr contextAsContent = do_QueryInterface(aContextNode); - if (contextAsContent && !contextAsContent->IsElement()) { - contextAsContent = contextAsContent->GetParent(); - if (contextAsContent && !contextAsContent->IsElement()) { - // can this even happen? - contextAsContent = nullptr; - } - } - - if (contextAsContent && !contextAsContent->IsHTMLElement(nsGkAtoms::html)) { + Element* element = aContextNode->GetAsElementOrParentElement(); + if (element && !element->IsHTMLElement(nsGkAtoms::html)) { aRv = ParseFragmentHTML( - aFragment, frag, contextAsContent->NodeInfo()->NameAtom(), - contextAsContent->GetNameSpaceID(), + aFragment, frag, element->NodeInfo()->NameAtom(), + element->GetNameSpaceID(), (document->GetCompatibilityMode() == eCompatibility_NavQuirks), aPreventScriptExecution); } else { @@ -4694,11 +4686,8 @@ already_AddRefed nsContentUtils::CreateContextualFragment( AutoTArray tagStack; nsAutoString uriStr, nameStr; - nsCOMPtr content = do_QueryInterface(aContextNode); - // just in case we have a text node - if (content && !content->IsElement()) content = content->GetParent(); - - while (content && content->IsElement()) { + for (Element* element = aContextNode->GetAsElementOrParentElement(); element; + element = element->GetParentElement()) { nsString& tagName = *tagStack.AppendElement(); // It mostly doesn't actually matter what tag name we use here: XML doesn't // have parsing that depends on the open tag stack, apart from namespace @@ -4718,14 +4707,13 @@ already_AddRefed nsContentUtils::CreateContextualFragment( tagName.AssignLiteral("notacustomelement"); // see if we need to add xmlns declarations - uint32_t count = content->AsElement()->GetAttrCount(); + uint32_t count = element->GetAttrCount(); bool setDefaultNamespace = false; if (count > 0) { uint32_t index; for (index = 0; index < count; index++) { - const BorrowedAttrInfo info = - content->AsElement()->GetAttrInfoAt(index); + const BorrowedAttrInfo info = element->GetAttrInfoAt(index); const nsAttrName* name = info.mName; if (name->NamespaceEquals(kNameSpaceID_XMLNS)) { info.mValue->ToString(uriStr); @@ -4747,7 +4735,7 @@ already_AddRefed nsContentUtils::CreateContextualFragment( } if (!setDefaultNamespace) { - mozilla::dom::NodeInfo* info = content->NodeInfo(); + mozilla::dom::NodeInfo* info = element->NodeInfo(); if (!info->GetPrefixAtom() && info->NamespaceID() != kNameSpaceID_None) { // We have no namespace prefix, but have a namespace ID. Push // default namespace attr in, so that our kids will be in our @@ -4758,8 +4746,6 @@ already_AddRefed nsContentUtils::CreateContextualFragment( tagName.Append('"'); } } - - content = content->GetParent(); } RefPtr frag; diff --git a/dom/base/nsINode.h b/dom/base/nsINode.h index a97f3e6cff04..47a3eebd3111 100644 --- a/dom/base/nsINode.h +++ b/dom/base/nsINode.h @@ -979,6 +979,12 @@ class nsINode : public mozilla::dom::EventTarget { */ mozilla::dom::Element* GetParentElementCrossingShadowRoot() const; + /** + * Get closest element node for the node. Meaning that if the node is an + * element node, returns itself. Otherwise, returns parent element or null. + */ + inline mozilla::dom::Element* GetAsElementOrParentElement() const; + /** * Get the root of the subtree this node belongs to. This never returns * null. It may return 'this' (e.g. for document nodes, and nodes that diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 3413d6d69459..8e470bab4e24 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -2367,8 +2367,7 @@ Element* HTMLEditor::GetElementOrParentByTagNameInternal(const nsAtom& aTagName, nsINode& aNode) const { MOZ_ASSERT(&aTagName != nsGkAtoms::_empty); - Element* currentElement = - aNode.IsElement() ? aNode.AsElement() : aNode.GetParentElement(); + Element* currentElement = aNode.GetAsElementOrParentElement(); if (NS_WARN_IF(!currentElement)) { // Neither aNode nor its parent is an element, so no ancestor is MOZ_ASSERT(!aNode.GetParentNode() || @@ -3556,32 +3555,29 @@ bool HTMLEditor::IsTextPropertySetByContent(nsINode* aNode, nsAtom* aProperty, nsAString* outValue) { MOZ_ASSERT(aNode && aProperty); - while (aNode) { - if (aNode->IsElement()) { - Element* element = aNode->AsElement(); - if (aProperty == element->NodeInfo()->NameAtom()) { - if (!aAttribute) { - return true; - } - nsAutoString value; - element->GetAttr(kNameSpaceID_None, aAttribute, value); - if (outValue) { - *outValue = value; - } - if (!value.IsEmpty()) { - if (!aValue) { - return true; - } - if (aValue->Equals(value, nsCaseInsensitiveStringComparator())) { - return true; - } - // We found the prop with the attribute, but the value doesn't - // match. - break; - } - } + for (Element* element = aNode->GetAsElementOrParentElement(); element; + element = element->GetParentElement()) { + if (aProperty != element->NodeInfo()->NameAtom()) { + continue; + } + if (!aAttribute) { + return true; + } + nsAutoString value; + element->GetAttr(kNameSpaceID_None, aAttribute, value); + if (outValue) { + *outValue = value; + } + if (!value.IsEmpty()) { + if (!aValue) { + return true; + } + if (aValue->Equals(value, nsCaseInsensitiveStringComparator())) { + return true; + } + // We found the prop with the attribute, but the value doesn't match. + return false; } - aNode = aNode->GetParentNode(); } return false; } @@ -4448,12 +4444,14 @@ nsresult HTMLEditor::CopyLastEditableChildStylesWithTransaction( deepestEditableContent = GetPreviousEditableHTMLNode(*deepestEditableContent); } - Element* deepestVisibleEditableElement = nullptr; - if (deepestEditableContent) { - deepestVisibleEditableElement = - deepestEditableContent->IsElement() - ? deepestEditableContent->AsElement() - : deepestEditableContent->GetParentElement(); + if (!deepestEditableContent) { + return NS_OK; + } + + Element* deepestVisibleEditableElement = + deepestEditableContent->GetAsElementOrParentElement(); + if (!deepestVisibleEditableElement) { + return NS_OK; } // Clone inline elements to keep current style in the new block. diff --git a/editor/libeditor/HTMLEditorDataTransfer.cpp b/editor/libeditor/HTMLEditorDataTransfer.cpp index cc640e4b46c0..8a11e1d1db91 100644 --- a/editor/libeditor/HTMLEditorDataTransfer.cpp +++ b/editor/libeditor/HTMLEditorDataTransfer.cpp @@ -593,9 +593,7 @@ nsresult HTMLEditor::DoInsertHTMLWithContext( if (containerContent) { Element* mostAncestorTableRelatedElement = nullptr; for (Element* maybeTableRelatedElement = - containerContent->IsElement() - ? containerContent->AsElement() - : containerContent->GetParentElement(); + containerContent->GetAsElementOrParentElement(); maybeTableRelatedElement && maybeTableRelatedElement != lastInsertedContent; maybeTableRelatedElement = @@ -2739,9 +2737,8 @@ void HTMLEditor::AutoHTMLFragmentBoundariesFixer:: nsIContent& aContent, nsTArray>& aOutArrayOfListAndTableElements) const { - for (Element* element = aContent.IsElement() ? aContent.AsElement() - : aContent.GetParentElement(); - element; element = element->GetParentElement()) { + for (Element* element = aContent.GetAsElementOrParentElement(); element; + element = element->GetParentElement()) { if (HTMLEditUtils::IsList(element) || HTMLEditUtils::IsTable(element)) { aOutArrayOfListAndTableElements.AppendElement(*element); } @@ -2839,9 +2836,8 @@ HTMLEditor::AutoHTMLFragmentBoundariesFixer::FindReplaceableTableElement( // But this looks really buggy because this loop may skip aTableElement // as the following NS_ASSERTION. We should write automated tests and // check right behavior. - for (Element* element = aContentMaybeInTableElement.IsElement() - ? aContentMaybeInTableElement.AsElement() - : aContentMaybeInTableElement.GetParentElement(); + for (Element* element = + aContentMaybeInTableElement.GetAsElementOrParentElement(); element; element = element->GetParentElement()) { if (!HTMLEditUtils::IsTableElement(element) || element->IsHTMLElement(nsGkAtoms::table)) { @@ -2879,9 +2875,8 @@ bool HTMLEditor::AutoHTMLFragmentBoundariesFixer::IsReplaceableListElement( // But this looks really buggy because this loop may skip aListElement // as the following NS_ASSERTION. We should write automated tests and // check right behavior. - for (Element* element = aContentMaybeInListElement.IsElement() - ? aContentMaybeInListElement.AsElement() - : aContentMaybeInListElement.GetParentElement(); + for (Element* element = + aContentMaybeInListElement.GetAsElementOrParentElement(); element; element = element->GetParentElement()) { if (!HTMLEditUtils::IsListItem(element)) { // XXX Perhaps, the original developer of this method assumed that diff --git a/gfx/layers/apz/util/DoubleTapToZoom.cpp b/gfx/layers/apz/util/DoubleTapToZoom.cpp index 990795290b62..51db9484dd36 100644 --- a/gfx/layers/apz/util/DoubleTapToZoom.cpp +++ b/gfx/layers/apz/util/DoubleTapToZoom.cpp @@ -54,12 +54,11 @@ static already_AddRefed ElementFromPoint( // FIXME(emilio): This should probably use the flattened tree, GetParent() is // not guaranteed to be an element in presence of shadow DOM. nsIContent* content = frame->GetContent(); - if (content && !content->IsElement()) { - content = content->GetParent(); + if (!content) { + return nullptr; } - if (content && content->IsElement()) { - nsCOMPtr result = content->AsElement(); - return result.forget(); + if (dom::Element* element = content->GetAsElementOrParentElement()) { + return do_AddRef(element); } return nullptr; } diff --git a/layout/base/TouchManager.cpp b/layout/base/TouchManager.cpp index 4e8ddce764ed..8ea29962542e 100644 --- a/layout/base/TouchManager.cpp +++ b/layout/base/TouchManager.cpp @@ -132,10 +132,9 @@ nsIFrame* TouchManager::SetupTarget(WidgetTouchEvent* aEvent, if (target) { nsCOMPtr targetContent; target->GetContentForEvent(aEvent, getter_AddRefs(targetContent)); - while (targetContent && !targetContent->IsElement()) { - targetContent = targetContent->GetParent(); - } - touch->SetTouchTarget(targetContent); + touch->SetTouchTarget(targetContent + ? targetContent->GetAsElementOrParentElement() + : nullptr); } else { aEvent->mTouches.RemoveElementAt(i); } @@ -206,10 +205,9 @@ nsIFrame* TouchManager::SuppressInvalidPointsAndGetTargetedFrame( } else { targetFrame = newTargetFrame; targetFrame->GetContentForEvent(aEvent, getter_AddRefs(targetContent)); - while (targetContent && !targetContent->IsElement()) { - targetContent = targetContent->GetParent(); - } - touch->SetTouchTarget(targetContent); + touch->SetTouchTarget(targetContent + ? targetContent->GetAsElementOrParentElement() + : nullptr); } } if (targetFrame) { diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 325889e92ce5..f152fb1bcc36 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -2409,10 +2409,7 @@ static Element* FindElementAncestorForMozSelection(nsIContent* aContent) { aContent = aContent->GetClosestNativeAnonymousSubtreeRootParent(); } NS_ASSERTION(aContent, "aContent isn't in non-anonymous tree?"); - while (aContent && !aContent->IsElement()) { - aContent = aContent->GetParent(); - } - return aContent ? aContent->AsElement() : nullptr; + return aContent ? aContent->GetAsElementOrParentElement() : nullptr; } already_AddRefed nsIFrame::ComputeSelectionStyle( diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index 8a7ee9dacc67..1e5ed01f2ef3 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -4163,22 +4163,19 @@ bool nsWindow::TouchEventShouldStartDrag(EventMessage aEventMessage, hittest.mInputSource = MouseEvent_Binding::MOZ_SOURCE_TOUCH; DispatchInputEvent(&hittest); - EventTarget* target = hittest.GetDOMEventTarget(); - if (target) { - nsCOMPtr node = do_QueryInterface(target); - - // Check if the element or any parent element has the - // attribute we're looking for. - while (node) { - if (node->IsElement()) { + if (EventTarget* target = hittest.GetDOMEventTarget()) { + if (nsCOMPtr content = do_QueryInterface(target)) { + // Check if the element or any parent element has the + // attribute we're looking for. + for (Element* element = content->GetAsElementOrParentElement(); element; + element = element->GetParentElement()) { nsAutoString startDrag; - node->AsElement()->GetAttribute( - NS_LITERAL_STRING("touchdownstartsdrag"), startDrag); + element->GetAttribute(NS_LITERAL_STRING("touchdownstartsdrag"), + startDrag); if (!startDrag.IsEmpty()) { return true; } } - node = node->GetParent(); } } }