Bug 1627175 - part 3: Move EditorBase::AreNodesSameType() to HTMLEditUtils r=m_kato

It's only non-`HTMLEditor` user is `EditorBase::JoinNodesDeepWithTransaction()`,
but it's called only by `HTMLEditor`.  Therefore, we can move it into
`HTMLEditUtils` and move `EditorBase::JoinNodesDeepWithTransaction()` to
`HTMLEditor`.

Depends on D70875

Differential Revision: https://phabricator.services.mozilla.com/D70876

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Masayuki Nakano 2020-04-15 15:55:20 +00:00
parent a56b91bd4e
commit 7d9227e497
9 changed files with 188 additions and 172 deletions

View File

@ -1167,26 +1167,23 @@ bool CSSEditUtils::IsCSSPrefChecked() const { return mIsCSSPrefChecked; }
// class
// static
bool CSSEditUtils::ElementsSameStyle(Element* aFirstElement,
Element* aSecondElement) {
MOZ_ASSERT(aFirstElement);
MOZ_ASSERT(aSecondElement);
if (aFirstElement->HasAttr(kNameSpaceID_None, nsGkAtoms::id) ||
aSecondElement->HasAttr(kNameSpaceID_None, nsGkAtoms::id)) {
bool CSSEditUtils::DoElementsHaveSameStyle(const Element& aElement,
const Element& aOtherElement) {
if (aElement.HasAttr(kNameSpaceID_None, nsGkAtoms::id) ||
aOtherElement.HasAttr(kNameSpaceID_None, nsGkAtoms::id)) {
// at least one of the spans carries an ID ; suspect a CSS rule applies to
// it and refuse to merge the nodes
return false;
}
nsAutoString firstClass, secondClass;
bool isFirstClassSet =
aFirstElement->GetAttr(kNameSpaceID_None, nsGkAtoms::_class, firstClass);
bool isSecondClassSet = aSecondElement->GetAttr(
kNameSpaceID_None, nsGkAtoms::_class, secondClass);
if (isFirstClassSet && isSecondClassSet) {
nsAutoString firstClass, otherClass;
bool isElementClassSet =
aElement.GetAttr(kNameSpaceID_None, nsGkAtoms::_class, firstClass);
bool isOtherElementClassSet =
aOtherElement.GetAttr(kNameSpaceID_None, nsGkAtoms::_class, otherClass);
if (isElementClassSet && isOtherElementClassSet) {
// both spans carry a class, let's compare them
if (!firstClass.Equals(secondClass)) {
if (!firstClass.Equals(otherClass)) {
// WARNING : technically, the comparison just above is questionable :
// from a pure HTML/CSS point of view class="a b" is NOT the same than
// class="b a" because a CSS rule could test the exact value of the class
@ -1195,27 +1192,27 @@ bool CSSEditUtils::ElementsSameStyle(Element* aFirstElement,
// need to discuss this issue before any modification.
return false;
}
} else if (isFirstClassSet || isSecondClassSet) {
} else if (isElementClassSet || isOtherElementClassSet) {
// one span only carries a class, early way out
return false;
}
nsCOMPtr<nsICSSDeclaration> firstCSSDecl, secondCSSDecl;
uint32_t firstLength, secondLength;
nsresult rv = GetInlineStyles(aFirstElement, getter_AddRefs(firstCSSDecl),
&firstLength);
nsCOMPtr<nsICSSDeclaration> firstCSSDecl, otherCSSDecl;
uint32_t firstLength, otherLength;
nsresult rv =
GetInlineStyles(aElement, getter_AddRefs(firstCSSDecl), &firstLength);
if (NS_FAILED(rv) || !firstCSSDecl) {
NS_WARNING("CSSEditUtils::GetInlineStyle() failed");
NS_WARNING("CSSEditUtils::GetInlineStyles() failed");
return false;
}
rv = GetInlineStyles(aSecondElement, getter_AddRefs(secondCSSDecl),
&secondLength);
if (NS_FAILED(rv) || !secondCSSDecl) {
rv = GetInlineStyles(aOtherElement, getter_AddRefs(otherCSSDecl),
&otherLength);
if (NS_FAILED(rv) || !otherCSSDecl) {
NS_WARNING("CSSEditUtils::GetInlineStyles() failed");
return false;
}
if (firstLength != secondLength) {
if (firstLength != otherLength) {
// early way out if we can
return false;
}
@ -1226,7 +1223,7 @@ bool CSSEditUtils::ElementsSameStyle(Element* aFirstElement,
}
nsAutoCString propertyNameString;
nsAutoString firstValue, secondValue;
nsAutoString firstValue, otherValue;
for (uint32_t i = 0; i < firstLength; i++) {
firstCSSDecl->Item(i, propertyNameString);
DebugOnly<nsresult> rvIgnored =
@ -1234,19 +1231,18 @@ bool CSSEditUtils::ElementsSameStyle(Element* aFirstElement,
NS_WARNING_ASSERTION(
NS_SUCCEEDED(rvIgnored),
"nsICSSDeclaration::GetPropertyValue() failed, but ignored");
rvIgnored =
secondCSSDecl->GetPropertyValue(propertyNameString, secondValue);
rvIgnored = otherCSSDecl->GetPropertyValue(propertyNameString, otherValue);
NS_WARNING_ASSERTION(
NS_SUCCEEDED(rvIgnored),
"nsICSSDeclaration::GetPropertyValue() failed, but ignored");
if (!firstValue.Equals(secondValue)) {
if (!firstValue.Equals(otherValue)) {
return false;
}
}
for (uint32_t i = 0; i < secondLength; i++) {
secondCSSDecl->Item(i, propertyNameString);
for (uint32_t i = 0; i < otherLength; i++) {
otherCSSDecl->Item(i, propertyNameString);
DebugOnly<nsresult> rvIgnored =
secondCSSDecl->GetPropertyValue(propertyNameString, secondValue);
otherCSSDecl->GetPropertyValue(propertyNameString, otherValue);
NS_WARNING_ASSERTION(
NS_SUCCEEDED(rvIgnored),
"nsICSSDeclaration::GetPropertyValue() failed, but ignored");
@ -1254,7 +1250,7 @@ bool CSSEditUtils::ElementsSameStyle(Element* aFirstElement,
NS_WARNING_ASSERTION(
NS_SUCCEEDED(rvIgnored),
"nsICSSDeclaration::GetPropertyValue() failed, but ignored");
if (!firstValue.Equals(secondValue)) {
if (!firstValue.Equals(otherValue)) {
return false;
}
}
@ -1263,15 +1259,16 @@ bool CSSEditUtils::ElementsSameStyle(Element* aFirstElement,
}
// static
nsresult CSSEditUtils::GetInlineStyles(Element* aElement,
nsresult CSSEditUtils::GetInlineStyles(const Element& aElement,
nsICSSDeclaration** aCssDecl,
uint32_t* aLength) {
if (NS_WARN_IF(!aElement) || NS_WARN_IF(!aLength)) {
if (NS_WARN_IF(!aLength)) {
return NS_ERROR_INVALID_ARG;
}
*aLength = 0;
// TODO: Perhaps, this method should take nsStyledElement& instead.
nsCOMPtr<nsStyledElement> styledElement = do_QueryInterface(aElement);
nsCOMPtr<nsStyledElement> styledElement =
do_QueryInterface(const_cast<Element*>(&aElement));
if (NS_WARN_IF(!styledElement)) {
return NS_ERROR_INVALID_ARG;
}

View File

@ -299,18 +299,17 @@ class CSSEditUtils final {
bool IsCSSPrefChecked() const;
/**
* ElementsSameStyle compares two elements and checks if they have the same
* specified CSS declarations in the STYLE attribute.
* The answer is always false if at least one of them carries an ID or a
* class.
* DoElementsHaveSameStyle compares two elements and checks if they have the
* same specified CSS declarations in the STYLE attribute. The answer is
* always false if at least one of them carries an ID or a class.
*
* @param aFirstNode [IN] A DOM node.
* @param aSecondNode [IN] A DOM node.
* @param aElement [IN] A DOM node.
* @param aOtherElement [IN] A DOM node.
* @return true if the two elements are considered to
* have same styles.
*/
static bool ElementsSameStyle(dom::Element* aFirstNode,
dom::Element* aSecondNode);
static bool DoElementsHaveSameStyle(const dom::Element& aElement,
const dom::Element& aOtherElement);
/**
* Get the specified inline styles (style attribute) for an element.
@ -320,7 +319,7 @@ class CSSEditUtils final {
* style attribute.
* @param aLength [OUT] The number of declarations in aCssDecl.
*/
static nsresult GetInlineStyles(dom::Element* aElement,
static nsresult GetInlineStyles(const dom::Element& aElement,
nsICSSDeclaration** aCssDecl,
uint32_t* aLength);

View File

@ -4139,27 +4139,6 @@ NS_IMETHODIMP EditorBase::ResetModificationCount() {
return NS_OK;
}
// static
bool EditorBase::AreNodesSameType(nsIContent& aNode1,
nsIContent& aNode2) const {
if (aNode1.NodeInfo()->NameAtom() != aNode2.NodeInfo()->NameAtom()) {
return false;
}
if (!AsHTMLEditor() || !AsHTMLEditor()->IsCSSEnabled()) {
return true;
}
// If this is an HTMLEditor in CSS mode and they are <span> elements,
// let's check their styles.
if (!aNode1.IsHTMLElement(nsGkAtoms::span)) {
return true;
}
if (!aNode1.IsElement() || !aNode2.IsElement()) {
return false;
}
return CSSEditUtils::ElementsSameStyle(aNode1.AsElement(),
aNode2.AsElement());
}
// static
nsIContent* EditorBase::GetNodeAtRangeOffsetPoint(
const RawRangeBoundary& aPoint) {
@ -4337,70 +4316,6 @@ SplitNodeResult EditorBase::SplitNodeDeepWithTransaction(
return SplitNodeResult(NS_ERROR_FAILURE);
}
EditorDOMPoint EditorBase::JoinNodesDeepWithTransaction(
nsIContent& aLeftNode, nsIContent& aRightNode) {
// While the rightmost children and their descendants of the left node match
// the leftmost children and their descendants of the right node, join them
// up.
nsCOMPtr<nsIContent> leftNodeToJoin = &aLeftNode;
nsCOMPtr<nsIContent> rightNodeToJoin = &aRightNode;
nsCOMPtr<nsINode> parentNode = aRightNode.GetParentNode();
EditorDOMPoint ret;
EditorType editorType = GetEditorType();
while (leftNodeToJoin && rightNodeToJoin && parentNode &&
AreNodesSameType(*leftNodeToJoin, *rightNodeToJoin)) {
uint32_t length = leftNodeToJoin->Length();
// Do the join
nsresult rv = JoinNodesWithTransaction(*leftNodeToJoin, *rightNodeToJoin);
if (NS_FAILED(rv)) {
NS_WARNING("EditorBase::JoinNodesWithTransaction() failed");
return EditorDOMPoint();
}
ret.Set(rightNodeToJoin, length);
if (parentNode->GetAsText()) {
// We've joined all the way down to text nodes, we're done!
return ret;
}
// Get new left and right nodes, and begin anew
parentNode = rightNodeToJoin;
rightNodeToJoin = parentNode->GetChildAt_Deprecated(length);
if (rightNodeToJoin) {
leftNodeToJoin = rightNodeToJoin->GetPreviousSibling();
} else {
leftNodeToJoin = nullptr;
}
// Skip over non-editable nodes
while (leftNodeToJoin &&
!EditorUtils::IsEditableContent(*leftNodeToJoin, editorType)) {
leftNodeToJoin = leftNodeToJoin->GetPreviousSibling();
}
if (!leftNodeToJoin) {
return ret;
}
while (rightNodeToJoin &&
!EditorUtils::IsEditableContent(*rightNodeToJoin, editorType)) {
rightNodeToJoin = rightNodeToJoin->GetNextSibling();
}
if (!rightNodeToJoin) {
return ret;
}
}
if (NS_WARN_IF(!ret.IsSet())) {
return EditorDOMPoint();
}
return ret;
}
nsresult EditorBase::EnsureNoPaddingBRElementForEmptyEditor() {
MOZ_ASSERT(IsEditActionDataAvailable());

View File

@ -1902,21 +1902,6 @@ class EditorBase : public nsIEditor,
const EditorDOMPoint& aDeepestStartOfRightNode,
SplitAtEdges aSplitAtEdges);
/**
* JoinNodesDeepWithTransaction() joins aLeftNode and aRightNode "deeply".
* First, they are joined simply, then, new right node is assumed as the
* child at length of the left node before joined and new left node is
* assumed as its previous sibling. Then, they will be joined again.
* And then, these steps are repeated.
*
* @param aLeftNode The node which will be removed form the tree.
* @param aRightNode The node which will be inserted the contents of
* aLeftNode.
* @return The point of the first child of the last right node.
*/
MOZ_CAN_RUN_SCRIPT EditorDOMPoint
JoinNodesDeepWithTransaction(nsIContent& aLeftNode, nsIContent& aRightNode);
/**
* EnsureNoPaddingBRElementForEmptyEditor() removes padding <br> element
* for empty editor if there is.
@ -2126,14 +2111,6 @@ class EditorBase : public nsIEditor,
*/
bool ShouldHandleIMEComposition() const;
/**
* AreNodesSameType() returns true if aNode1 and aNode2 are same type.
* If the instance is TextEditor, only their names are checked.
* If the instance is HTMLEditor in CSS mode and both of them are <span>
* element, their styles are also checked.
*/
bool AreNodesSameType(nsIContent& aNode1, nsIContent& aNode2) const;
static bool IsTextNode(nsINode* aNode) {
return aNode->NodeType() == nsINode::TEXT_NODE;
}

View File

@ -68,6 +68,7 @@ class nsISupports;
namespace mozilla {
using namespace dom;
using StyleDifference = HTMLEditUtils::StyleDifference;
enum { kLonely = 0, kPrevSib = 1, kNextSib = 2, kBothSibs = 3 };
@ -3283,7 +3284,10 @@ EditActionResult HTMLEditor::HandleDeleteNonCollapsedSelection(
// type of elements, we can merge them after deleting the selected contents.
// MOOSE: this could conceivably screw up a table.. fix me.
if (leftBlock->GetParentNode() == rightBlock->GetParentNode() &&
AreNodesSameType(*leftBlock, *rightBlock) &&
HTMLEditUtils::CanContentsBeJoined(
*leftBlock, *rightBlock,
IsCSSEnabled() ? StyleDifference::CompareIfSpanElements
: StyleDifference::Ignore) &&
// XXX What's special about these three types of block?
(leftBlock->IsHTMLElement(nsGkAtoms::p) ||
HTMLEditUtils::IsListItem(leftBlock) ||
@ -3299,17 +3303,16 @@ EditActionResult HTMLEditor::HandleDeleteNonCollapsedSelection(
return EditActionHandled(rv);
}
// Join blocks
EditorDOMPoint atFirstChildOfTheLastRightNode =
Result<EditorDOMPoint, nsresult> atFirstChildOfTheLastRightNodeOrError =
JoinNodesDeepWithTransaction(*leftBlock, *rightBlock);
if (NS_WARN_IF(Destroyed())) {
return EditActionHandled(NS_ERROR_EDITOR_DESTROYED);
}
if (!atFirstChildOfTheLastRightNode.IsSet()) {
NS_WARNING("EditorBase::JoinNodesDeepWithTransaction() failed");
return EditActionHandled(NS_ERROR_FAILURE);
if (atFirstChildOfTheLastRightNodeOrError.isErr()) {
NS_WARNING("HTMLEditor::JoinNodesDeepWithTransaction() failed");
return EditActionHandled(
atFirstChildOfTheLastRightNodeOrError.unwrapErr());
}
MOZ_ASSERT(atFirstChildOfTheLastRightNodeOrError.inspect().IsSet());
// Fix up selection
rv = CollapseSelectionTo(atFirstChildOfTheLastRightNode);
rv = CollapseSelectionTo(atFirstChildOfTheLastRightNodeOrError.inspect());
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"HTMLEditor::CollapseSelectionTo() failed");
return EditActionHandled(rv);
@ -3625,6 +3628,82 @@ EditorDOMPoint HTMLEditor::GetGoodCaretPointFor(
return EditorDOMPoint(&aContent);
}
Result<EditorDOMPoint, nsresult> HTMLEditor::JoinNodesDeepWithTransaction(
nsIContent& aLeftContent, nsIContent& aRightContent) {
// While the rightmost children and their descendants of the left node match
// the leftmost children and their descendants of the right node, join them
// up.
nsCOMPtr<nsIContent> leftContentToJoin = &aLeftContent;
nsCOMPtr<nsIContent> rightContentToJoin = &aRightContent;
nsCOMPtr<nsINode> parentNode = aRightContent.GetParentNode();
EditorDOMPoint ret;
const HTMLEditUtils::StyleDifference kCompareStyle =
IsCSSEnabled() ? StyleDifference::CompareIfSpanElements
: StyleDifference::Ignore;
while (leftContentToJoin && rightContentToJoin && parentNode &&
HTMLEditUtils::CanContentsBeJoined(
*leftContentToJoin, *rightContentToJoin, kCompareStyle)) {
uint32_t length = leftContentToJoin->Length();
// Do the join
nsresult rv =
JoinNodesWithTransaction(*leftContentToJoin, *rightContentToJoin);
if (NS_WARN_IF(Destroyed())) {
return Err(NS_ERROR_EDITOR_DESTROYED);
}
if (NS_FAILED(rv)) {
NS_WARNING("EditorBase::JoinNodesWithTransaction() failed");
return Err(rv);
}
// XXX rightContentToJoin may have fewer children or shorter length text.
// So, we need some adjustment here.
ret.Set(rightContentToJoin, length);
if (NS_WARN_IF(!ret.IsSet())) {
return Err(NS_ERROR_FAILURE);
}
if (parentNode->IsText()) {
// We've joined all the way down to text nodes, we're done!
return ret;
}
// Get new left and right nodes, and begin anew
parentNode = rightContentToJoin;
rightContentToJoin = parentNode->GetChildAt_Deprecated(length);
if (rightContentToJoin) {
leftContentToJoin = rightContentToJoin->GetPreviousSibling();
} else {
leftContentToJoin = nullptr;
}
// Skip over non-editable nodes
while (leftContentToJoin && !EditorUtils::IsEditableContent(
*leftContentToJoin, EditorType::HTML)) {
leftContentToJoin = leftContentToJoin->GetPreviousSibling();
}
if (!leftContentToJoin) {
return ret;
}
while (rightContentToJoin && !EditorUtils::IsEditableContent(
*rightContentToJoin, EditorType::HTML)) {
rightContentToJoin = rightContentToJoin->GetNextSibling();
}
if (!rightContentToJoin) {
return ret;
}
}
if (!ret.IsSet()) {
NS_WARNING("HTMLEditor::JoinNodesDeepWithTransaction() joined no contents");
return Err(NS_ERROR_FAILURE);
}
return ret;
}
EditActionResult HTMLEditor::TryToJoinBlocksWithTransaction(
nsIContent& aLeftContentInBlock, nsIContent& aRightContentInBlock) {
MOZ_ASSERT(IsEditActionDataAvailable());
@ -9817,16 +9896,9 @@ nsresult HTMLEditor::JoinNearestEditableNodesWithTransaction(
return rv;
}
// XXX This condition is odd. Despite of the name, `AreNodesSameType()`
// compairs its style with `CSSEditUtils::ElementsSameStyle()` only
// when `IsCSSEnabled()` returns true and `lastLeft` is a `<span>`
// element. However, calling `CSSEditUtils::ElementsSameStyle()`
// without those checking again.
if (HTMLEditor::AreNodesSameType(*lastLeft, *firstRight) &&
(lastLeft->IsText() ||
(lastLeft->IsElement() && firstRight->IsElement() &&
CSSEditUtils::ElementsSameStyle(lastLeft->AsElement(),
firstRight->AsElement())))) {
if ((lastLeft->IsText() || lastLeft->IsElement()) &&
HTMLEditUtils::CanContentsBeJoined(*lastLeft, *firstRight,
StyleDifference::CompareIfElements)) {
nsresult rv = JoinNearestEditableNodesWithTransaction(
*lastLeft, *firstRight, aNewFirstChildOfRightNode);
NS_WARNING_ASSERTION(

View File

@ -5,6 +5,7 @@
#include "HTMLEditUtils.h"
#include "CSSEditUtils.h" // for CSSEditUtils
#include "mozilla/ArrayUtils.h" // for ArrayLength
#include "mozilla/Assertions.h" // for MOZ_ASSERT, etc.
#include "mozilla/EditAction.h" // for EditAction
@ -26,6 +27,26 @@
namespace mozilla {
bool HTMLEditUtils::CanContentsBeJoined(const nsIContent& aLeftContent,
const nsIContent& aRightContent,
StyleDifference aStyleDifference) {
if (aLeftContent.NodeInfo()->NameAtom() !=
aRightContent.NodeInfo()->NameAtom()) {
return false;
}
if (aStyleDifference == StyleDifference::Ignore ||
!aLeftContent.IsElement()) {
return true;
}
if (aStyleDifference == StyleDifference::CompareIfSpanElements &&
!aLeftContent.IsHTMLElement(nsGkAtoms::span)) {
return true;
}
MOZ_DIAGNOSTIC_ASSERT(aRightContent.IsElement());
return CSSEditUtils::DoElementsHaveSameStyle(*aLeftContent.AsElement(),
*aRightContent.AsElement());
}
bool HTMLEditUtils::IsBlockElement(const nsIContent& aContent) {
if (!aContent.IsElement()) {
return false;

View File

@ -39,6 +39,23 @@ class HTMLEditUtils final {
aContent.GetParentNode()->IsEditable();
}
/**
* CanContentsBeJoined() returns true if aLeftContent and aRightContent can be
* joined. At least, Node.nodeName must be same when this returns true.
*/
enum class StyleDifference {
// Ignore style information so that callers may join different styled
// contents.
Ignore,
// Compare style information when the contents are any elements.
CompareIfElements,
// Compare style information only when the contents are <span> elements.
CompareIfSpanElements,
};
static bool CanContentsBeJoined(const nsIContent& aLeftContent,
const nsIContent& aRightContent,
StyleDifference aStyleDifference);
/**
* IsBlockElement() returns true if aContent is an element and it should
* be treated as a block. (This does not refer style information.)

View File

@ -11,6 +11,7 @@
#include "mozilla/CSSEditUtils.h"
#include "mozilla/EditorUtils.h"
#include "mozilla/ManualNAC.h"
#include "mozilla/Result.h"
#include "mozilla/StyleSheet.h"
#include "mozilla/TextEditor.h"
#include "mozilla/UniquePtr.h"
@ -2103,6 +2104,23 @@ class HTMLEditor final : public TextEditor,
const EditorDOMPoint& aPointToInsert,
MoveToEndOfContainer aMoveToEndOfContainer = MoveToEndOfContainer::No);
/**
* JoinNodesDeepWithTransaction() joins aLeftNode and aRightNode "deeply".
* First, they are joined simply, then, new right node is assumed as the
* child at length of the left node before joined and new left node is
* assumed as its previous sibling. Then, they will be joined again.
* And then, these steps are repeated.
*
* @param aLeftContent The node which will be removed form the tree.
* @param aRightContent The node which will be inserted the contents of
* aRightContent.
* @return The point of the first child of the last right node.
* The result is always set if this succeeded.
*/
MOZ_CAN_RUN_SCRIPT Result<EditorDOMPoint, nsresult>
JoinNodesDeepWithTransaction(nsIContent& aLeftContent,
nsIContent& aRightContent);
/**
* TryToJoinBlocksWithTransaction() tries to join two block elements. The
* right element is always joined to the left element. If the elements are

View File

@ -383,7 +383,7 @@ bool HTMLEditor::IsSimpleModifiableNode(nsIContent* aContent, nsAtom* aProperty,
aAttribute, aValue,
/*suppress transaction*/ true);
return CSSEditUtils::ElementsSameStyle(newSpanElement, element);
return CSSEditUtils::DoElementsHaveSameStyle(*newSpanElement, *element);
}
nsresult HTMLEditor::SetInlinePropertyOnTextNode(