From 6d173e82825e4b2a0ef22c04c9e6ff63bbeea29f Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Mon, 22 Feb 2021 10:18:48 +0000 Subject: [PATCH] Bug 1675779 - HandleDeleteAtomicContent should handle non-editable text node. r=masayuki Since `WSScanResult` can return non-editable/non-removable text node, `HandleDeleteAtomicContent` is called with non editable text node, then it cannot remove atomic content. This fix doesn't follow other block case such as added newer test. Differential Revision: https://phabricator.services.mozilla.com/D96298 --- editor/libeditor/HTMLEditorDeleteHandler.cpp | 75 ++++++++++++++++++- .../meta/editing/run/delete.html.ini | 3 + .../meta/editing/run/forwarddelete.html.ini | 4 +- .../meta/editing/run/insertparagraph.html.ini | 2 +- ...target-ranges-backspace.tentative.html.ini | 3 - .../web-platform/tests/editing/data/delete.js | 25 +++++++ .../tests/editing/data/forwarddelete.js | 25 +++++++ 7 files changed, 128 insertions(+), 9 deletions(-) diff --git a/editor/libeditor/HTMLEditorDeleteHandler.cpp b/editor/libeditor/HTMLEditorDeleteHandler.cpp index 87f276002dbf..bdc70c4d8122 100644 --- a/editor/libeditor/HTMLEditorDeleteHandler.cpp +++ b/editor/libeditor/HTMLEditorDeleteHandler.cpp @@ -257,6 +257,23 @@ class MOZ_STACK_CLASS HTMLEditor::AutoDeleteRangesHandler final { const HTMLEditor& aHTMLEditor, const nsIContent& aAtomicContent, AutoRangeArray& aRangesToDelete) const; + /** + * GetAtomicContnetToDelete() returns better content that is deletion of + * atomic element. If aScanFromCaretPointResult is special, since this + * point may not be editable, we look for better point to remove atomic + * content. + * + * @param aDirectionAndAmount Direction of the deletion. + * @param aWSRunScannerAtCaret WSRunScanner instance which was + * initialized with the caret point. + * @param aScanFromCaretPointResult Scan result of aWSRunScannerAtCaret + * toward aDirectionAndAmount. + */ + static nsIContent* GetAtomicContentToDelete( + nsIEditor::EDirection aDirectionAndAmount, + const WSRunScanner& aWSRunScannerAtCaret, + const WSScanResult& aScanFromCaretPointResult) MOZ_NONNULL_RETURN; + /** * HandleDeleteHRElement() handles deletion around `
` element. If * aDirectionAndAmount is nsIEditor::ePrevious, aHTElement is removed only @@ -1637,8 +1654,16 @@ HTMLEditor::AutoDeleteRangesHandler::ComputeRangesToDeleteAroundCollapsedRanges( aWSRunScannerAtCaret.GetEditingHost()) { return NS_OK; } + nsIContent* atomicContent = GetAtomicContentToDelete( + aDirectionAndAmount, aWSRunScannerAtCaret, aScanFromCaretPointResult); + if (!HTMLEditUtils::IsRemovableNode(*atomicContent)) { + NS_WARNING( + "AutoDeleteRangesHandler::GetAtomicContentToDelete() cannot find " + "removable atomic content"); + return NS_ERROR_FAILURE; + } nsresult rv = ComputeRangesToDeleteAtomicContent( - aHTMLEditor, *aScanFromCaretPointResult.GetContent(), aRangesToDelete); + aHTMLEditor, *atomicContent, aRangesToDelete); NS_WARNING_ASSERTION( NS_SUCCEEDED(rv), "AutoDeleteRangesHandler::ComputeRangesToDeleteAtomicContent() failed"); @@ -1665,6 +1690,10 @@ HTMLEditor::AutoDeleteRangesHandler::ComputeRangesToDeleteAroundCollapsedRanges( if (NS_WARN_IF(!aScanFromCaretPointResult.GetContent()->IsElement())) { return NS_ERROR_FAILURE; } + // TODO(m_kato): + // When aScanFromCaretPointResult.GetContent isn't editable and is + // removable, PrepareToDeleteAtOtherBlockBoundary will return false. + // But this should be removed. AutoBlockElementsJoiner joiner(*this); if (!joiner.PrepareToDeleteAtOtherBlockBoundary( aHTMLEditor, aDirectionAndAmount, @@ -1764,9 +1793,17 @@ HTMLEditor::AutoDeleteRangesHandler::HandleDeleteAroundCollapsedRanges( aWSRunScannerAtCaret.GetEditingHost()) { return EditActionHandled(); } + nsCOMPtr atomicContent = GetAtomicContentToDelete( + aDirectionAndAmount, aWSRunScannerAtCaret, aScanFromCaretPointResult); + if (!HTMLEditUtils::IsRemovableNode(*atomicContent)) { + NS_WARNING( + "AutoDeleteRangesHandler::GetAtomicContentToDelete() cannot find " + "removable atomic content"); + return EditActionResult(NS_ERROR_FAILURE); + } EditActionResult result = HandleDeleteAtomicContent( - aHTMLEditor, MOZ_KnownLive(*aScanFromCaretPointResult.GetContent()), - aWSRunScannerAtCaret.ScanStartRef(), aWSRunScannerAtCaret); + aHTMLEditor, *atomicContent, aWSRunScannerAtCaret.ScanStartRef(), + aWSRunScannerAtCaret); NS_WARNING_ASSERTION( result.Succeeded(), "AutoDeleteRangesHandler::HandleDeleteAtomicContent() failed"); @@ -2264,6 +2301,38 @@ EditActionResult HTMLEditor::AutoDeleteRangesHandler::HandleDeleteHRElement( return EditActionHandled(rv); } +// static +nsIContent* HTMLEditor::AutoDeleteRangesHandler::GetAtomicContentToDelete( + nsIEditor::EDirection aDirectionAndAmount, + const WSRunScanner& aWSRunScannerAtCaret, + const WSScanResult& aScanFromCaretPointResult) { + MOZ_ASSERT(aScanFromCaretPointResult.GetContent()); + + if (!aScanFromCaretPointResult.ReachedSpecialContent()) { + return aScanFromCaretPointResult.GetContent(); + } + + if (!aScanFromCaretPointResult.GetContent()->IsText() || + HTMLEditUtils::IsRemovableNode(*aScanFromCaretPointResult.GetContent())) { + return aScanFromCaretPointResult.GetContent(); + } + + // aScanFromCaretPointResult is non-removable text node. + // Since we try removing atomic content, we look for removable node from + // scanned point that is non-removable text. + nsIContent* removableRoot = aScanFromCaretPointResult.GetContent(); + while (removableRoot && !HTMLEditUtils::IsRemovableNode(*removableRoot)) { + removableRoot = removableRoot->GetParent(); + } + + if (removableRoot) { + return removableRoot; + } + + // Not found better content. This content may not be removable. + return aScanFromCaretPointResult.GetContent(); +} + nsresult HTMLEditor::AutoDeleteRangesHandler::ComputeRangesToDeleteAtomicContent( const HTMLEditor& aHTMLEditor, const nsIContent& aAtomicContent, diff --git a/testing/web-platform/meta/editing/run/delete.html.ini b/testing/web-platform/meta/editing/run/delete.html.ini index 3d5c51191eb8..305a606a332a 100644 --- a/testing/web-platform/meta/editing/run/delete.html.ini +++ b/testing/web-platform/meta/editing/run/delete.html.ini @@ -697,3 +697,6 @@ [[["delete",""\]\] "
a[\]bc
" compare innerHTML] expected: FAIL + [[["delete",""\]\] "foo
bar
[\]baz" compare innerHTML] + expected: FAIL + diff --git a/testing/web-platform/meta/editing/run/forwarddelete.html.ini b/testing/web-platform/meta/editing/run/forwarddelete.html.ini index 7e83c412450a..9773dc119248 100644 --- a/testing/web-platform/meta/editing/run/forwarddelete.html.ini +++ b/testing/web-platform/meta/editing/run/forwarddelete.html.ini @@ -506,8 +506,8 @@ [forwarddelete.html?6001-last] - max-asserts: 3 - min-asserts: 3 + max-asserts: 6 + min-asserts: 6 [[["forwarddelete",""\]\] "
  1. foo

{}

  1. bar
" compare innerHTML] expected: FAIL diff --git a/testing/web-platform/meta/editing/run/insertparagraph.html.ini b/testing/web-platform/meta/editing/run/insertparagraph.html.ini index 8443d1a6d221..5922b603bc99 100644 --- a/testing/web-platform/meta/editing/run/insertparagraph.html.ini +++ b/testing/web-platform/meta/editing/run/insertparagraph.html.ini @@ -300,7 +300,7 @@ [insertparagraph.html?5001-6000] - min-asserts: 14 # Retrieving meaningless raw DOM point from WSScannerResult + min-asserts: 10 # Retrieving meaningless raw DOM point from WSScannerResult max-asserts: 16 # Only on W-fis on Linux1804-64-qr and on Android, there are 2 additional assertions. [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "" compare innerHTML] expected: FAIL diff --git a/testing/web-platform/meta/input-events/input-events-get-target-ranges-backspace.tentative.html.ini b/testing/web-platform/meta/input-events/input-events-get-target-ranges-backspace.tentative.html.ini index 9b753b961a6b..a97a1d771491 100644 --- a/testing/web-platform/meta/input-events/input-events-get-target-ranges-backspace.tentative.html.ini +++ b/testing/web-platform/meta/input-events/input-events-get-target-ranges-backspace.tentative.html.ini @@ -52,6 +52,3 @@ [Backspace at "
abc
  • [\] def
ghi
" - comparing innerHTML] expected: FAIL - [Backspace at "

abcdef[\]ghi

"] - expected: FAIL - diff --git a/testing/web-platform/tests/editing/data/delete.js b/testing/web-platform/tests/editing/data/delete.js index 7fa2c4cfdfc4..138a749500b7 100644 --- a/testing/web-platform/tests/editing/data/delete.js +++ b/testing/web-platform/tests/editing/data/delete.js @@ -2560,4 +2560,29 @@ var browserTests = [ "foo{}bar", [true], {"delete":[false,false,"",false,false,""]}], +["foobar[]baz", + [["delete",""]], + "foo{}baz", + [true], + {"delete":[false,false,"",false,false,""]}], +["foobar[]baz", + [["delete",""]], + "foo{}baz", + [true], + {"delete":[false,false,"",false,false,""]}], +["foo
bar
[]baz", + [["delete",""]], + "foo{}baz", + [true], + {"delete":[false,false,"",false,false,""]}], +["foobar[]baz", + [["delete",""]], + "foo{}baz", + [true], + {"delete":[false,false,"",false,false,""]}], +["foobarbaz[]qux", + [["delete",""]], + "foobar{}qux", + [true], + {"delete":[false,false,"",false,false,""]}], ] diff --git a/testing/web-platform/tests/editing/data/forwarddelete.js b/testing/web-platform/tests/editing/data/forwarddelete.js index 90310f8c2c8b..cc67b35c27e3 100644 --- a/testing/web-platform/tests/editing/data/forwarddelete.js +++ b/testing/web-platform/tests/editing/data/forwarddelete.js @@ -2445,4 +2445,29 @@ var browserTests = [ "foo{}bar", [true], {"forwarddelete":[false,false,"",false,false,""]}], +["foo[]barbaz", + [["forwarddelete",""]], + "foo{}baz", + [true], + {"forwarddelete":[false,false,"",false,false,""]}], +["foo[]barbaz", + [["forwarddelete",""]], + "foo{}baz", + [true], + {"forwarddelete":[false,false,"",false,false,""]}], +["foo[]
bar
baz", + [["forwarddelete",""]], + "foo{}baz", + [true], + {"forwarddelete":[false,false,"",false,false,""]}], +["foo[]barbaz", + [["forwarddelete",""]], + "foo{}baz", + [true], + {"forwarddelete":[false,false,"",false,false,""]}], +["foobar[]bazqux", + [["forwarddelete",""]], + "foobar{}qux", + [true], + {"forwarddelete":[false,false,"",false,false,""]}], ]