From 9ae8cf68368bae01f7764e9a8584235dafd7b7ee Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Mon, 3 Aug 2020 08:24:14 +0000 Subject: [PATCH] Bug 1655706 - part 2-2: Move 2nd block of `HTMLEditor::TryToJoinBlocksWithTransaction()` into `WhiteSpaceVisibilityKeeper` r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D85532 --- editor/libeditor/HTMLEditSubActionHandler.cpp | 155 +--------------- editor/libeditor/WSRunObject.cpp | 171 ++++++++++++++++++ editor/libeditor/WSRunObject.h | 27 +++ 3 files changed, 207 insertions(+), 146 deletions(-) diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp index 463bc3416e83..fbdf54b0b407 100644 --- a/editor/libeditor/HTMLEditSubActionHandler.cpp +++ b/editor/libeditor/HTMLEditSubActionHandler.cpp @@ -4603,154 +4603,17 @@ EditActionResult HTMLEditor::TryToJoinBlocksWithTransaction( EditorDOMPoint atLeftBlockChild; if (EditorUtils::IsDescendantOf(*rightBlockElement, *leftBlockElement, &atLeftBlockChild)) { - MOZ_ASSERT(leftBlockElement == atLeftBlockChild.GetContainer()); - - nsresult rv = WhiteSpaceVisibilityKeeper::DeleteInvisibleASCIIWhiteSpaces( - *this, EditorDOMPoint(rightBlockElement, 0)); - if (NS_FAILED(rv)) { - NS_WARNING( - "WhiteSpaceVisibilityKeeper::DeleteInvisibleASCIIWhiteSpaces() " - "failed at right block"); - return EditActionIgnored(rv); - } - - OwningNonNull originalLeftBlockElement = *leftBlockElement; - { - // We can't just track leftBlockElement because it's an Element, so track - // something else. - AutoTrackDOMPoint tracker(RangeUpdaterRef(), &atLeftBlockChild); - rv = WhiteSpaceVisibilityKeeper::DeleteInvisibleASCIIWhiteSpaces( - *this, EditorDOMPoint(leftBlockElement, atLeftBlockChild.Offset())); - if (NS_FAILED(rv)) { - NS_WARNING( - "WhiteSpaceVisibilityKeeper::DeleteInvisibleASCIIWhiteSpaces() " - "failed at left block child"); - return EditActionIgnored(rv); - } - // XXX AutoTrackDOMPoint instance, tracker, hasn't been destroyed here. - // Do we really need to do update rightBlockElement here?? - if (atLeftBlockChild.GetContainerAsElement()) { - leftBlockElement = atLeftBlockChild.GetContainerAsElement(); - } else if (NS_WARN_IF(!atLeftBlockChild.GetContainerParentAsElement())) { - return EditActionIgnored(NS_ERROR_UNEXPECTED); - } else { - leftBlockElement = atLeftBlockChild.GetContainerParentAsElement(); - } - } - - // Do br adjustment. - RefPtr invisibleBRElementBeforeLeftBlockElement = - WSRunScanner::GetPrecedingBRElementUnlessVisibleContentFound( - *this, atLeftBlockChild); - EditActionResult ret(NS_OK); - if (newListElementTagNameOfRightListElement.isSome()) { - // XXX Why do we ignore the error from MoveChildrenWithTransaction()? - MOZ_ASSERT(originalLeftBlockElement == atLeftBlockChild.GetContainer(), - "This is not guaranteed, but assumed"); - MoveNodeResult moveNodeResult = MoveChildrenWithTransaction( - *rightBlockElement, - EditorDOMPoint(originalLeftBlockElement, atLeftBlockChild.Offset())); - if (NS_WARN_IF(moveNodeResult.EditorDestroyed())) { - return ret.SetResult(NS_ERROR_EDITOR_DESTROYED); - } - NS_WARNING_ASSERTION( - moveNodeResult.Succeeded(), - "HTMLEditor::MoveChildrenWithTransaction() failed, but ignored"); - if (moveNodeResult.Succeeded()) { - ret |= moveNodeResult; - } - // atLeftBlockChild was moved to rightListElement. So, it's invalid now. - atLeftBlockChild.Clear(); - } else { - // Left block is a parent of right block, and the parent of the previous - // visible content. Right block is a child and contains the contents we - // want to move. - - EditorDOMPoint atPreviousContent; - if (&aLeftContentInBlock == leftBlockElement) { - // We are working with valid HTML, aLeftContentInBlock is a block node, - // and is therefore allowed to contain rightBlockElement. This is the - // simple case, we will simply move the content in rightBlockElement - // out of its block. - atPreviousContent = atLeftBlockChild; - } else { - // We try to work as well as possible with HTML that's already invalid. - // Although "right block" is a block, and a block must not be contained - // in inline elements, reality is that broken documents do exist. The - // DIRECT parent of "left NODE" might be an inline element. Previous - // versions of this code skipped inline parents until the first block - // parent was found (and used "left block" as the destination). - // However, in some situations this strategy moves the content to an - // unexpected position. (see bug 200416) The new idea is to make the - // moving content a sibling, next to the previous visible content. - atPreviousContent.Set(&aLeftContentInBlock); - - // We want to move our content just after the previous visible node. - atPreviousContent.AdvanceOffset(); - } - - MOZ_ASSERT(atPreviousContent.IsSet()); - - // Because we don't want the moving content to receive the style of the - // previous content, we split the previous content's style. - - RefPtr editingHost = GetActiveEditingHost(); - // XXX It's odd to continue handling this edit action if there is no - // editing host. - if (!editingHost || &aLeftContentInBlock != editingHost) { - SplitNodeResult splitResult = SplitAncestorStyledInlineElementsAt( - atPreviousContent, nullptr, nullptr); - if (splitResult.Failed()) { - NS_WARNING( - "HTMLEditor::SplitAncestorStyledInlineElementsAt() failed"); - return EditActionIgnored(splitResult.Rv()); - } - - if (splitResult.Handled()) { - if (splitResult.GetNextNode()) { - atPreviousContent.Set(splitResult.GetNextNode()); - if (!atPreviousContent.IsSet()) { - NS_WARNING("Next node of split point was orphaned"); - return EditActionIgnored(NS_ERROR_NULL_POINTER); - } - } else { - atPreviousContent = splitResult.SplitPoint(); - if (!atPreviousContent.IsSet()) { - NS_WARNING("Split node was orphaned"); - return EditActionIgnored(NS_ERROR_NULL_POINTER); - } - } - } - } - - ret |= MoveOneHardLineContents(EditorDOMPoint(rightBlockElement, 0), - atPreviousContent); - if (ret.Failed()) { - NS_WARNING("HTMLEditor::MoveOneHardLineContents() failed"); - return ret; - } - } - - if (!invisibleBRElementBeforeLeftBlockElement) { - return ret; - } - - rv = DeleteNodeWithTransaction(*invisibleBRElementBeforeLeftBlockElement); - if (NS_WARN_IF(Destroyed())) { - return ret.SetResult(NS_ERROR_EDITOR_DESTROYED); - } - NS_WARNING_ASSERTION( - NS_SUCCEEDED(rv), - "HTMLEditor::DeleteNodeWithTransaction() failed, but ignored"); - if (NS_SUCCEEDED(rv)) { - ret.MarkAsHandled(); - } - return ret; + EditActionResult result = WhiteSpaceVisibilityKeeper:: + MergeFirstLineOfRightBlockElementIntoAncestorLeftBlockElement( + *this, *leftBlockElement, *rightBlockElement, atLeftBlockChild, + aLeftContentInBlock, newListElementTagNameOfRightListElement); + NS_WARNING_ASSERTION(result.Succeeded(), + "WhiteSpaceVisibilityKeeper::" + "MergeFirstLineOfRightBlockElementIntoAncestorLeftBloc" + "kElement() failed"); + return result; } - MOZ_DIAGNOSTIC_ASSERT(!atRightBlockChild.IsSet()); - MOZ_DIAGNOSTIC_ASSERT(!atLeftBlockChild.IsSet()); - // Normal case. Blocks are siblings, or at least close enough. An example // of the latter is

paragraph

  • one
  • two
  • three
. The // first li and the p are not true siblings, but we still want to join them diff --git a/editor/libeditor/WSRunObject.cpp b/editor/libeditor/WSRunObject.cpp index 7df144abf543..fda278643582 100644 --- a/editor/libeditor/WSRunObject.cpp +++ b/editor/libeditor/WSRunObject.cpp @@ -129,6 +129,177 @@ nsresult WhiteSpaceVisibilityKeeper::PrepareToSplitAcrossBlocks( return rv; } +// static +EditActionResult WhiteSpaceVisibilityKeeper:: + MergeFirstLineOfRightBlockElementIntoAncestorLeftBlockElement( + HTMLEditor& aHTMLEditor, Element& aLeftBlockElement, + Element& aRightBlockElement, const EditorDOMPoint& aAtLeftBlockChild, + nsIContent& aLeftContentInBlock, + const Maybe& aListElementTagName) { + MOZ_ASSERT( + EditorUtils::IsDescendantOf(aRightBlockElement, aLeftBlockElement)); + MOZ_ASSERT( + &aLeftBlockElement == &aLeftContentInBlock || + EditorUtils::IsDescendantOf(aLeftContentInBlock, aLeftBlockElement)); + MOZ_ASSERT(&aLeftBlockElement == aAtLeftBlockChild.GetContainer()); + + // NOTE: This method may extend deletion range: + // - to delete invisible white-spaces at start of aRightBlockElement + // - to delete invisible white-spaces before aRightBlockElement + // - to delete invisible white-spaces at start of aAtLeftBlockChild.GetChild() + // - to delete invisible white-spaces before aAtLeftBlockChild.GetChild() + // - to delete invisible `
` element before aAtLeftBlockChild.GetChild() + + nsresult rv = WhiteSpaceVisibilityKeeper::DeleteInvisibleASCIIWhiteSpaces( + aHTMLEditor, EditorDOMPoint(&aRightBlockElement, 0)); + if (NS_FAILED(rv)) { + NS_WARNING( + "WhiteSpaceVisibilityKeeper::DeleteInvisibleASCIIWhiteSpaces() failed " + "at right block"); + return EditActionIgnored(rv); + } + + OwningNonNull originalLeftBlockElement = aLeftBlockElement; + OwningNonNull leftBlockElement = aLeftBlockElement; + EditorDOMPoint atLeftBlockChild(aAtLeftBlockChild); + { + // We can't just track leftBlockElement because it's an Element, so track + // something else. + AutoTrackDOMPoint tracker(aHTMLEditor.RangeUpdaterRef(), &atLeftBlockChild); + nsresult rv = WhiteSpaceVisibilityKeeper::DeleteInvisibleASCIIWhiteSpaces( + aHTMLEditor, EditorDOMPoint(atLeftBlockChild.GetContainer(), + atLeftBlockChild.Offset())); + if (NS_FAILED(rv)) { + NS_WARNING( + "WhiteSpaceVisibilityKeeper::DeleteInvisibleASCIIWhiteSpaces() " + "failed at left block child"); + return EditActionIgnored(rv); + } + // XXX AutoTrackDOMPoint instance, tracker, hasn't been destroyed here. + // Do we really need to do update aRightBlockElement here?? + // XXX And atLeftBlockChild.GetContainerAsElement() always returns + // an element pointer so that probably here should not use + // accessors of EditorDOMPoint, should use DOM API directly instead. + if (atLeftBlockChild.GetContainerAsElement()) { + leftBlockElement = *atLeftBlockChild.GetContainerAsElement(); + } else if (NS_WARN_IF(!atLeftBlockChild.GetContainerParentAsElement())) { + return EditActionIgnored(NS_ERROR_UNEXPECTED); + } else { + leftBlockElement = *atLeftBlockChild.GetContainerParentAsElement(); + } + } + + // Do br adjustment. + RefPtr invisibleBRElementBeforeLeftBlockElement = + WSRunScanner::GetPrecedingBRElementUnlessVisibleContentFound( + aHTMLEditor, atLeftBlockChild); + EditActionResult ret(NS_OK); + if (aListElementTagName.isSome()) { + // XXX Why do we ignore the error from MoveChildrenWithTransaction()? + MOZ_ASSERT(originalLeftBlockElement == atLeftBlockChild.GetContainer(), + "This is not guaranteed, but assumed"); + MoveNodeResult moveNodeResult = aHTMLEditor.MoveChildrenWithTransaction( + aRightBlockElement, EditorDOMPoint(atLeftBlockChild.GetContainer(), + atLeftBlockChild.Offset())); + if (NS_WARN_IF(moveNodeResult.EditorDestroyed())) { + return ret.SetResult(NS_ERROR_EDITOR_DESTROYED); + } + NS_WARNING_ASSERTION( + moveNodeResult.Succeeded(), + "HTMLEditor::MoveChildrenWithTransaction() failed, but ignored"); + if (moveNodeResult.Succeeded()) { + ret |= moveNodeResult; + } + // atLeftBlockChild was moved to rightListElement. So, it's invalid now. + atLeftBlockChild.Clear(); + } else { + // Left block is a parent of right block, and the parent of the previous + // visible content. Right block is a child and contains the contents we + // want to move. + + EditorDOMPoint atPreviousContent; + if (&aLeftContentInBlock == leftBlockElement) { + // We are working with valid HTML, aLeftContentInBlock is a block node, + // and is therefore allowed to contain aRightBlockElement. This is the + // simple case, we will simply move the content in aRightBlockElement + // out of its block. + atPreviousContent = atLeftBlockChild; + } else { + // We try to work as well as possible with HTML that's already invalid. + // Although "right block" is a block, and a block must not be contained + // in inline elements, reality is that broken documents do exist. The + // DIRECT parent of "left NODE" might be an inline element. Previous + // versions of this code skipped inline parents until the first block + // parent was found (and used "left block" as the destination). + // However, in some situations this strategy moves the content to an + // unexpected position. (see bug 200416) The new idea is to make the + // moving content a sibling, next to the previous visible content. + atPreviousContent.Set(&aLeftContentInBlock); + + // We want to move our content just after the previous visible node. + atPreviousContent.AdvanceOffset(); + } + + MOZ_ASSERT(atPreviousContent.IsSet()); + + // Because we don't want the moving content to receive the style of the + // previous content, we split the previous content's style. + + Element* editingHost = aHTMLEditor.GetActiveEditingHost(); + // XXX It's odd to continue handling this edit action if there is no + // editing host. + if (!editingHost || &aLeftContentInBlock != editingHost) { + SplitNodeResult splitResult = + aHTMLEditor.SplitAncestorStyledInlineElementsAt(atPreviousContent, + nullptr, nullptr); + if (splitResult.Failed()) { + NS_WARNING("HTMLEditor::SplitAncestorStyledInlineElementsAt() failed"); + return EditActionIgnored(splitResult.Rv()); + } + + if (splitResult.Handled()) { + if (splitResult.GetNextNode()) { + atPreviousContent.Set(splitResult.GetNextNode()); + if (!atPreviousContent.IsSet()) { + NS_WARNING("Next node of split point was orphaned"); + return EditActionIgnored(NS_ERROR_NULL_POINTER); + } + } else { + atPreviousContent = splitResult.SplitPoint(); + if (!atPreviousContent.IsSet()) { + NS_WARNING("Split node was orphaned"); + return EditActionIgnored(NS_ERROR_NULL_POINTER); + } + } + } + } + + ret |= aHTMLEditor.MoveOneHardLineContents( + EditorDOMPoint(&aRightBlockElement, 0), atPreviousContent); + if (ret.Failed()) { + NS_WARNING("HTMLEditor::MoveOneHardLineContents() failed"); + return ret; + } + } + + if (!invisibleBRElementBeforeLeftBlockElement) { + return ret; + } + + rv = aHTMLEditor.DeleteNodeWithTransaction( + *invisibleBRElementBeforeLeftBlockElement); + if (NS_WARN_IF(aHTMLEditor.Destroyed())) { + return ret.SetResult(NS_ERROR_EDITOR_DESTROYED); + } + NS_WARNING_ASSERTION( + NS_SUCCEEDED(rv), + "HTMLEditor::DeleteNodeWithTransaction() failed, but ignored"); + if (NS_SUCCEEDED(rv)) { + ret.MarkAsHandled(); + } + return ret; +} + // static EditActionResult WhiteSpaceVisibilityKeeper:: MergeFirstLineOfRightBlockElementIntoLeftBlockElement( diff --git a/editor/libeditor/WSRunObject.h b/editor/libeditor/WSRunObject.h index 166109a871f1..c0d482b007bf 100644 --- a/editor/libeditor/WSRunObject.h +++ b/editor/libeditor/WSRunObject.h @@ -1211,6 +1211,33 @@ class WhiteSpaceVisibilityKeeper final { HTMLEditor& aHTMLEditor, nsCOMPtr* aSplitNode, int32_t* aSplitOffset); + /** + * MergeFirstLineOfRightBlockElementIntoAncestorLeftBlockElement() merges + * first line in aRightBlockElement into end of aLeftBlockElement which + * is an ancestor of aRightBlockElement, then, removes aRightBlockElement + * if it becomes empty. + * + * @param aHTMLEditor The HTML editor. + * @param aLeftBlockElement The content will be merged into end of + * this element. + * @param aRightBlockElement The first line in this element will be + * moved to aLeftBlockElement and maybe + * removed when this becomes empty. + * @param aAtLeftBlockChild At a child of aLeftBlockElement and inclusive + * ancestor of aRightBlockElement. + * @param aLeftContentInBlock The content whose inclusive ancestor is + * aLeftBlockElement. + * @param aListElementTagName Set some if aRightBlockElement is a list + * element and it'll be merged with another + * list element. + */ + [[nodiscard]] MOZ_CAN_RUN_SCRIPT static EditActionResult + MergeFirstLineOfRightBlockElementIntoAncestorLeftBlockElement( + HTMLEditor& aHTMLEditor, Element& aLeftBlockElement, + Element& aRightBlockElement, const EditorDOMPoint& aAtLeftBlockChild, + nsIContent& aLeftContentInBlock, + const Maybe& aListElementTagName); + /** * MergeFirstLineOfRightBlockElementIntoLeftBlockElement() merges first * line in aRightBlockElement into end of aLeftBlockElement and removes