diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h index b7a2a90f8f8e..cf41fdf589bf 100644 --- a/editor/libeditor/HTMLEditor.h +++ b/editor/libeditor/HTMLEditor.h @@ -3946,13 +3946,24 @@ class HTMLEditor final : public TextEditor, int32_t DiscoverPartialListsAndTables( nsTArray>& aPasteNodes, nsTArray>& aListsAndTables); - Element* ScanForListAndTableStructure(nsINode& aNodeMaybeInListOrTable, - Element& aListOrTable); + Element* ScanForTableStructure(nsINode& aNodeMaybeInTable, + Element& aTableElement); void ReplaceOrphanedStructure( StartOrEnd aStartOrEnd, nsTArray>& aNodeArray, nsTArray>& aListAndTableArray, int32_t aHighWaterMark); + /** + * IsReplaceableListElement() is a helper method of + * ReplaceOrphanedStructure(). If aNodeMaybeInListElement is a descendant + * of aListElement, returns true. Otherwise, false. + * + * @param aListElement Must be a list element. + * @param aNodeMaybeInListElement A node which may be in aListElement. + */ + static bool IsReplaceableListElement(Element& aListElement, + nsINode& aNodeMaybeInListElement); + /** * GetBetterInsertionPointFor() returns better insertion point to insert * aNodeToInsert. diff --git a/editor/libeditor/HTMLEditorDataTransfer.cpp b/editor/libeditor/HTMLEditorDataTransfer.cpp index 0ae525766649..b6cf76f604f1 100644 --- a/editor/libeditor/HTMLEditorDataTransfer.cpp +++ b/editor/libeditor/HTMLEditorDataTransfer.cpp @@ -2799,53 +2799,90 @@ int32_t HTMLEditor::DiscoverPartialListsAndTables( return ret; } -Element* HTMLEditor::ScanForListAndTableStructure( - nsINode& aNodeMaybeInListOrTable, Element& aListOrTable) { - // Look upward from first/last paste node for a piece of this list/table - bool isList = HTMLEditUtils::IsList(&aListOrTable); - for (Element* element = aNodeMaybeInListOrTable.IsElement() - ? aNodeMaybeInListOrTable.AsElement() - : aNodeMaybeInListOrTable.GetParentElement(); +Element* HTMLEditor::ScanForTableStructure(nsINode& aNodeMaybeInTable, + Element& aTableElement) { + for (Element* element = aNodeMaybeInTable.IsElement() + ? aNodeMaybeInTable.AsElement() + : aNodeMaybeInTable.GetParentElement(); element; element = element->GetParentElement()) { - if (!(isList && HTMLEditUtils::IsListItem(element)) && - !(!isList && HTMLEditUtils::IsTableElement(element) && - !element->IsHTMLElement(nsGkAtoms::table))) { + if (!HTMLEditUtils::IsTableElement(element) || + element->IsHTMLElement(nsGkAtoms::table)) { continue; } Element* structureElement = element->GetParentElement(); - if (isList) { - while (structureElement && !HTMLEditUtils::IsList(structureElement)) { - structureElement = structureElement->GetParentElement(); - } - } else { - while (structureElement && - !structureElement->IsHTMLElement(nsGkAtoms::table)) { - structureElement = structureElement->GetParentElement(); - } + while (structureElement && + !structureElement->IsHTMLElement(nsGkAtoms::table)) { + structureElement = structureElement->GetParentElement(); } - if (structureElement == &aListOrTable) { - return isList ? structureElement : element; + if (structureElement == &aTableElement) { + return element; } } return nullptr; } +// static +bool HTMLEditor::IsReplaceableListElement(Element& aListElement, + nsINode& aNodeMaybeInListElement) { + MOZ_ASSERT(HTMLEditUtils::IsList(&aListElement)); + // Perhaps, this is designed for climbing up the DOM tree from + // aNodeMaybeInListElement to aListElement and making sure that + // aNodeMaybeInListElement itself or its ancestor is an list item. + // 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 = aNodeMaybeInListElement.IsElement() + ? aNodeMaybeInListElement.AsElement() + : aNodeMaybeInListElement.GetParentElement(); + element; element = element->GetParentElement()) { + if (!HTMLEditUtils::IsListItem(element)) { + // XXX Perhaps, the original developer of this method assumed that + // aListElement won't be skipped because if it's assumed, we can + // stop climbing up the tree in that case. + NS_ASSERTION(element != &aListElement, + "The list element which is looking for is ignored"); + continue; + } + Element* listElement = nullptr; + for (Element* maybeListElement = element->GetParentElement(); + maybeListElement; + maybeListElement = maybeListElement->GetParentElement()) { + if (HTMLEditUtils::IsList(maybeListElement)) { + listElement = maybeListElement; + break; + } + } + if (listElement == &aListElement) { + return true; + } + // XXX If we find another list element, why don't we keep searching + // from its parent? + } + return false; +} + void HTMLEditor::ReplaceOrphanedStructure( StartOrEnd aStartOrEnd, nsTArray>& aNodeArray, nsTArray>& aListAndTableArray, int32_t aHighWaterMark) { MOZ_ASSERT(!aNodeArray.IsEmpty()); + OwningNonNull& edgeNode = + aStartOrEnd == StartOrEnd::end ? aNodeArray.LastElement() : aNodeArray[0]; OwningNonNull curNode = aListAndTableArray[aHighWaterMark]; // Find substructure of list or table that must be included in paste. - Element* replaceElment = ScanForListAndTableStructure( - aStartOrEnd == StartOrEnd::end ? *aNodeArray.LastElement() - : *aNodeArray[0], - curNode); - - if (!replaceElment) { - return; + Element* replaceElement; + if (HTMLEditUtils::IsList(curNode)) { + if (!HTMLEditor::IsReplaceableListElement(curNode, edgeNode)) { + return; + } + replaceElement = curNode; + } else { + replaceElement = ScanForTableStructure(edgeNode, curNode); + if (!replaceElement) { + return; + } } // If we found substructure, paste it instead of its descendants. @@ -2857,8 +2894,8 @@ void HTMLEditor::ReplaceOrphanedStructure( uint32_t idx = aStartOrEnd == StartOrEnd::start ? (i - removedCount) : (originalLength - i - 1); OwningNonNull endpoint = aNodeArray[idx]; - if (endpoint == replaceElment || - EditorUtils::IsDescendantOf(*endpoint, *replaceElment)) { + if (endpoint == replaceElement || + EditorUtils::IsDescendantOf(*endpoint, *replaceElement)) { aNodeArray.RemoveElementAt(idx); removedCount++; } @@ -2866,9 +2903,9 @@ void HTMLEditor::ReplaceOrphanedStructure( // Now replace the removed nodes with the structural parent if (aStartOrEnd == StartOrEnd::end) { - aNodeArray.AppendElement(*replaceElment); + aNodeArray.AppendElement(*replaceElement); } else { - aNodeArray.InsertElementAt(0, *replaceElment); + aNodeArray.InsertElementAt(0, *replaceElement); } }