commit 521a873deb991d970c6e8761daa8342a659be29e
parent 72dc90136c2ea085b7225df230c7ba7033662c86
Author: Masayuki Nakano <masayuki@d-toybox.com>
Date: Thu, 18 Dec 2025 06:50:20 +0000
Bug 2005895 - Make `AutoEmptyBlockAncestorDeleter::GetNewCaretPosition()` never return point after a line break before a block boundary r=m_kato
The caret should be put before the last line break in a block if there
is no visible content between the line break and the end block boundary.
Additionally,
`WhiteSpaceVisibilityKeeper::DeleteContentNodeAndJoinTextNodesAroundIt()`
should delete the preceding block's invisible `<br>` too.
Differential Revision: https://phabricator.services.mozilla.com/D276756
Diffstat:
10 files changed, 210 insertions(+), 26 deletions(-)
diff --git a/editor/libeditor/HTMLEditUtils.cpp b/editor/libeditor/HTMLEditUtils.cpp
@@ -146,6 +146,31 @@ template Result<EditorRawDOMPoint, nsresult>
HTMLEditUtils::ComputePointToPutCaretInElementIfOutside(
const Element& aElement, const EditorRawDOMPoint& aCurrentPoint);
+template Maybe<EditorLineBreak>
+HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorDOMPoint&, const Element&);
+template Maybe<EditorRawLineBreak>
+HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorDOMPoint&, const Element&);
+template Maybe<EditorLineBreak>
+HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorRawDOMPoint&, const Element&);
+template Maybe<EditorRawLineBreak>
+HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorRawDOMPoint&, const Element&);
+template Maybe<EditorLineBreak>
+HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorDOMPointInText&, const Element&);
+template Maybe<EditorRawLineBreak>
+HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorDOMPointInText&, const Element&);
+template Maybe<EditorLineBreak>
+HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorRawDOMPointInText&, const Element&);
+template Maybe<EditorRawLineBreak>
+HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorRawDOMPointInText&, const Element&);
+
template bool HTMLEditUtils::IsSameCSSColorValue(const nsAString& aColorA,
const nsAString& aColorB);
template bool HTMLEditUtils::IsSameCSSColorValue(const nsACString& aColorA,
@@ -2970,6 +2995,31 @@ HTMLEditUtils::ComputePointToPutCaretInElementIfOutside(
}
// static
+template <typename EditorLineBreakType, typename EditorDOMPointType>
+Maybe<EditorLineBreakType>
+HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorDOMPointType& aPoint, const Element& aEditingHost) {
+ MOZ_ASSERT(aPoint.IsSet());
+ if (MOZ_UNLIKELY(!aPoint.IsInContentNode())) {
+ return Nothing{};
+ }
+ const WSScanResult previousThing =
+ WSRunScanner::ScanPreviousVisibleNodeOrBlockBoundary({}, aPoint,
+ &aEditingHost);
+ if (!previousThing.ReachedLineBreak()) {
+ return Nothing{}; // No preceding line break.
+ }
+ const WSScanResult nextThing =
+ WSRunScanner::ScanInclusiveNextVisibleNodeOrBlockBoundary({}, aPoint,
+ &aEditingHost);
+ if (!nextThing.ReachedBlockBoundary()) {
+ return Nothing{}; // The line break is not followed by a block boundary so
+ // that it's a visible line break.
+ }
+ return Some(previousThing.CreateEditorLineBreak<EditorLineBreakType>());
+}
+
+// static
bool HTMLEditUtils::IsInlineStyleSetByElement(
const nsIContent& aContent, const EditorInlineStyle& aStyle,
const nsAString* aValue, nsAString* aOutValue /* = nullptr */) {
diff --git a/editor/libeditor/HTMLEditUtils.h b/editor/libeditor/HTMLEditUtils.h
@@ -2844,6 +2844,15 @@ class HTMLEditUtils final {
const Element& aElement, const EditorDOMPointTypeInput& aCurrentPoint);
/**
+ * Return a line break if aPoint is after a line break which is immediately
+ * before a block boundary.
+ */
+ template <typename EditorLineBreakType, typename EditorDOMPointType>
+ static Maybe<EditorLineBreakType>
+ GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem(
+ const EditorDOMPointType& aPoint, const Element& aEditingHost);
+
+ /**
* Content-based query returns true if
* <mHTMLProperty mAttribute=mAttributeValue> effects aContent. If there is
* such a element, but another element whose attribute value does not match
diff --git a/editor/libeditor/HTMLEditorDeleteHandler.cpp b/editor/libeditor/HTMLEditorDeleteHandler.cpp
@@ -7191,7 +7191,8 @@ HTMLEditor::AutoDeleteRangesHandler::AutoEmptyBlockAncestorDeleter::
Result<CaretPoint, nsresult> HTMLEditor::AutoDeleteRangesHandler::
AutoEmptyBlockAncestorDeleter::GetNewCaretPosition(
const HTMLEditor& aHTMLEditor,
- nsIEditor::EDirection aDirectionAndAmount) const {
+ nsIEditor::EDirection aDirectionAndAmount,
+ const Element& aEditingHost) const {
MOZ_ASSERT(mEmptyInclusiveAncestorBlockElement);
MOZ_ASSERT(mEmptyInclusiveAncestorBlockElement->GetParentElement());
MOZ_ASSERT(aHTMLEditor.IsEditActionDataAvailable());
@@ -7206,8 +7207,7 @@ Result<CaretPoint, nsresult> HTMLEditor::AutoDeleteRangesHandler::
EditorDOMPoint::After(mEmptyInclusiveAncestorBlockElement));
MOZ_ASSERT(afterEmptyBlock.IsSet());
if (nsIContent* nextContentOfEmptyBlock = HTMLEditUtils::GetNextContent(
- afterEmptyBlock, {}, BlockInlineCheck::Unused,
- aHTMLEditor.ComputeEditingHost())) {
+ afterEmptyBlock, {}, BlockInlineCheck::Unused, &aEditingHost)) {
EditorDOMPoint pt = HTMLEditUtils::GetGoodCaretPointFor<EditorDOMPoint>(
*nextContentOfEmptyBlock, aDirectionAndAmount);
if (!pt.IsSet()) {
@@ -7225,20 +7225,30 @@ Result<CaretPoint, nsresult> HTMLEditor::AutoDeleteRangesHandler::
case nsIEditor::ePreviousWord:
case nsIEditor::eToBeginningOfLine: {
// Collapse Selection to previous editable node of the empty block
- // if there is. Otherwise, to after the empty block.
+ // if there is.
EditorRawDOMPoint atEmptyBlock(mEmptyInclusiveAncestorBlockElement);
- if (nsIContent* previousContentOfEmptyBlock =
+ if (nsIContent* const previousContentOfEmptyBlock =
HTMLEditUtils::GetPreviousContent(
atEmptyBlock, {WalkTreeOption::IgnoreNonEditableNode},
- BlockInlineCheck::Unused, aHTMLEditor.ComputeEditingHost())) {
- EditorDOMPoint pt = HTMLEditUtils::GetGoodCaretPointFor<EditorDOMPoint>(
- *previousContentOfEmptyBlock, aDirectionAndAmount);
- if (!pt.IsSet()) {
+ BlockInlineCheck::Unused, &aEditingHost)) {
+ const EditorRawDOMPoint atEndOfPreviousContent =
+ HTMLEditUtils::GetGoodCaretPointFor<EditorRawDOMPoint>(
+ *previousContentOfEmptyBlock, aDirectionAndAmount);
+ if (!atEndOfPreviousContent.IsSet()) {
NS_WARNING("HTMLEditUtils::GetGoodCaretPointFor() failed");
return Err(NS_ERROR_FAILURE);
}
- return CaretPoint(std::move(pt));
- }
+ // If the previous content is between a preceding line break and the
+ // block boundary of current empty block, let's move caret to the line
+ // break if there is no visible things between them.
+ const Maybe<EditorRawLineBreak> precedingLineBreak =
+ HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem<
+ EditorRawLineBreak>(atEndOfPreviousContent, aEditingHost);
+ return precedingLineBreak.isSome()
+ ? CaretPoint(precedingLineBreak->To<EditorDOMPoint>())
+ : CaretPoint(atEndOfPreviousContent.To<EditorDOMPoint>());
+ }
+ // Otherwise, let's put caret next to the deleting block.
auto afterEmptyBlock =
EditorDOMPoint::After(*mEmptyInclusiveAncestorBlockElement);
if (NS_WARN_IF(!afterEmptyBlock.IsSet())) {
@@ -7309,7 +7319,7 @@ HTMLEditor::AutoDeleteRangesHandler::AutoEmptyBlockAncestorDeleter::Run(
: EditorDOMPoint());
}
Result<CaretPoint, nsresult> caretPointOrError =
- GetNewCaretPosition(aHTMLEditor, aDirectionAndAmount);
+ GetNewCaretPosition(aHTMLEditor, aDirectionAndAmount, aEditingHost);
NS_WARNING_ASSERTION(
caretPointOrError.isOk(),
"AutoEmptyBlockAncestorDeleter::GetNewCaretPosition() failed");
@@ -7343,10 +7353,10 @@ HTMLEditor::AutoDeleteRangesHandler::AutoEmptyBlockAncestorDeleter::Run(
"DeleteContentNodeAndJoinTextNodesAroundIt() failed");
return caretPointOrError.propagateErr();
}
+ trackPointToPutCaret.Flush(StopTracking::Yes);
caretPointOrError.unwrap().MoveCaretPointTo(
pointToPutCaret, {SuggestCaret::OnlyIfHasSuggestion});
- trackEmptyBlockPoint.FlushAndStopTracking();
- trackPointToPutCaret.FlushAndStopTracking();
+ trackEmptyBlockPoint.Flush(StopTracking::Yes);
if (NS_WARN_IF(!atEmptyInclusiveAncestorBlockElement
.IsInContentNodeAndValidInComposedDoc()) ||
NS_WARN_IF(pointToPutCaret.IsSet() &&
diff --git a/editor/libeditor/HTMLEditorNestedClasses.h b/editor/libeditor/HTMLEditorNestedClasses.h
@@ -1690,8 +1690,8 @@ HTMLEditor::AutoDeleteRangesHandler::AutoEmptyBlockAncestorDeleter final {
* `mEmptyInclusiveAncestorBlockElement`.
*/
[[nodiscard]] Result<CaretPoint, nsresult> GetNewCaretPosition(
- const HTMLEditor& aHTMLEditor,
- nsIEditor::EDirection aDirectionAndAmount) const;
+ const HTMLEditor& aHTMLEditor, nsIEditor::EDirection aDirectionAndAmount,
+ const Element& aEditingHost) const;
RefPtr<Element> mEmptyInclusiveAncestorBlockElement;
};
diff --git a/editor/libeditor/WSRunScanner.h b/editor/libeditor/WSRunScanner.h
@@ -375,6 +375,25 @@ class MOZ_STACK_CLASS WSScanResult final {
}
}
+ friend std::ostream& operator<<(std::ostream& aStream,
+ const ScanDirection& aDirection) {
+ return aStream << (aDirection == ScanDirection::Backward
+ ? "ScanDirection::Backward"
+ : "ScanDirection::Forward");
+ }
+
+ friend std::ostream& operator<<(std::ostream& aStream,
+ const WSScanResult& aResult) {
+ aStream << "{ mReason: " << aResult.mReason;
+ if (aResult.mReason == WSType::NotInitialized ||
+ aResult.mReason == WSType::InUncomposedDoc) {
+ return aStream << " }";
+ }
+ return aStream << ", mContent: " << aResult.mContent
+ << ", mOffset: " << aResult.mOffset
+ << ", mDirection: " << aResult.mDirection << " }";
+ }
+
private:
nsCOMPtr<nsIContent> mContent;
Maybe<uint32_t> mOffset;
diff --git a/editor/libeditor/WhiteSpaceVisibilityKeeper.cpp b/editor/libeditor/WhiteSpaceVisibilityKeeper.cpp
@@ -2399,12 +2399,13 @@ WhiteSpaceVisibilityKeeper::DeleteContentNodeAndJoinTextNodesAroundIt(
WSScanResult nextThingOfCaretPoint =
WSRunScanner::ScanInclusiveNextVisibleNodeOrBlockBoundary(
{}, pointToPutCaret);
- if (nextThingOfCaretPoint.ReachedBRElement() ||
- nextThingOfCaretPoint.ReachedPreformattedLineBreak()) {
+ Maybe<EditorLineBreak> lineBreak;
+ if (nextThingOfCaretPoint.ReachedLineBreak()) {
+ lineBreak.emplace(
+ nextThingOfCaretPoint.CreateEditorLineBreak<EditorLineBreak>());
nextThingOfCaretPoint =
WSRunScanner::ScanInclusiveNextVisibleNodeOrBlockBoundary(
- {}, nextThingOfCaretPoint
- .PointAfterReachedContent<EditorRawDOMPoint>());
+ {}, lineBreak->After<EditorRawDOMPoint>());
}
if (nextThingOfCaretPoint.ReachedBlockBoundary()) {
const EditorDOMPoint atBlockBoundary =
@@ -2423,6 +2424,24 @@ WhiteSpaceVisibilityKeeper::DeleteContentNodeAndJoinTextNodesAroundIt(
if (NS_WARN_IF(!aContentToDelete.IsInComposedDoc())) {
return Err(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE);
}
+ // If the previous content ends with an invisible line break, let's
+ // delete it.
+ if (lineBreak.isSome() && lineBreak->IsInComposedDoc()) {
+ const WSScanResult prevThing =
+ WSRunScanner::ScanPreviousVisibleNodeOrBlockBoundary(
+ {}, lineBreak->To<EditorRawDOMPoint>(), &aEditingHost);
+ if (!prevThing.ReachedLineBoundary()) {
+ Result<EditorDOMPoint, nsresult> pointOrError =
+ aHTMLEditor.DeleteLineBreakWithTransaction(
+ lineBreak.ref(), nsIEditor::eStrip, aEditingHost);
+ if (MOZ_UNLIKELY(pointOrError.isErr())) {
+ NS_WARNING(
+ "HTMLEditor::DeleteLineBreakWithTransaction() failed");
+ return pointOrError.propagateErr();
+ }
+ trackPointToPutCaret->Flush(StopTracking::No);
+ }
+ }
}
}
// Similarly, we may put caret into the following block (this is the
diff --git a/editor/libeditor/gtest/TestHTMLEditUtils.cpp b/editor/libeditor/gtest/TestHTMLEditUtils.cpp
@@ -1701,4 +1701,75 @@ TEST(HTMLEditUtilsTest, GetPreviousLeafContentOrPreviousBlockElement_Content)
// TODO: Test GetPreviousLeafContentOrPreviousBlockElement() which takes
// EditorDOMPoint
+struct MOZ_STACK_CLASS LineBreakBeforeBlockBoundaryTest final {
+ const char16_t* const mInnerHTML;
+ const char* const mContainer;
+ const Maybe<uint32_t>
+ mContainerIndex; // Set if need to use CharacterData in mContainer.
+ const uint32_t mOffset;
+ const bool mExpectedResult; // true if the method return a line break
+
+ friend std::ostream& operator<<(
+ std::ostream& aStream, const LineBreakBeforeBlockBoundaryTest& aTest) {
+ aStream << "Scan from { container: " << aTest.mContainer;
+ if (aTest.mContainerIndex) {
+ aStream << "'s " << aTest.mContainerIndex.value() + 1 << "th child";
+ }
+ return aStream << ", offset: " << aTest.mOffset << " } in "
+ << NS_ConvertUTF16toUTF8(aTest.mInnerHTML).get() << "\"";
+ }
+};
+
+TEST(HTMLEditUtilsTest, GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem)
+{
+ const RefPtr<Document> doc = CreateHTMLDoc();
+ const RefPtr<nsGenericHTMLElement> body = doc->GetBody();
+ MOZ_RELEASE_ASSERT(body);
+ for (const auto& testData : {
+ LineBreakBeforeBlockBoundaryTest{u"<div contenteditable>abc</div>",
+ "div", Some(0), 3, false},
+ LineBreakBeforeBlockBoundaryTest{u"<div contenteditable>abc</div>",
+ "div", Nothing{}, 1, false},
+ LineBreakBeforeBlockBoundaryTest{u"<div contenteditable><br></div>",
+ "div", Nothing{}, 0, false},
+ LineBreakBeforeBlockBoundaryTest{u"<div contenteditable><br></div>",
+ "div", Nothing{}, 1, true},
+ LineBreakBeforeBlockBoundaryTest{
+ u"<div contenteditable><br> </div>", "div", Some(1), 2, true},
+ LineBreakBeforeBlockBoundaryTest{
+ u"<div contenteditable><br><!-- X --></div>", "div", Nothing{},
+ 2,
+ // FIXME: Currently, this fails with a bug of WSRunScanner
+ // (actually, a bug of
+ // HTMLEditUtils::GetPreviousLeafContentOrPreviousBlockElement)
+ false},
+ LineBreakBeforeBlockBoundaryTest{
+ u"<div contenteditable><br><br></div>", "div", Nothing{}, 1,
+ false},
+ LineBreakBeforeBlockBoundaryTest{
+ u"<div contenteditable><br><p>abc</p></div>", "div", Nothing{},
+ 1, true},
+ }) {
+ body->SetInnerHTMLTrusted(nsDependentString(testData.mInnerHTML),
+ doc->NodePrincipal(), IgnoreErrors());
+ const Element* const containerElement = body->QuerySelector(
+ nsDependentCString(testData.mContainer), IgnoreErrors());
+ MOZ_ASSERT(containerElement);
+ const Element* const editingHost =
+ body->QuerySelector("[contenteditable]"_ns, IgnoreErrors());
+ MOZ_ASSERT(editingHost);
+ const nsIContent* const container =
+ testData.mContainerIndex
+ ? containerElement->GetChildAt_Deprecated(*testData.mContainerIndex)
+ : containerElement;
+ MOZ_RELEASE_ASSERT(container);
+ const Maybe<EditorRawLineBreak> result =
+ HTMLEditUtils::GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem<
+ EditorRawLineBreak>(EditorRawDOMPoint(container, testData.mOffset),
+ *editingHost);
+ EXPECT_EQ(result.isSome(), testData.mExpectedResult)
+ << "GetLineBreakBeforeBlockBoundaryIfPointIsBetweenThem: " << testData;
+ }
+}
+
} // namespace mozilla
diff --git a/testing/web-platform/meta/editing/run/delete.html.ini b/testing/web-platform/meta/editing/run/delete.html.ini
@@ -137,12 +137,6 @@
[[["delete",""\]\] "<ol><li>foo<br></ol>{}<br>" compare innerHTML]
expected: FAIL
- [[["defaultparagraphseparator","div"\],["delete",""\]\] "<ol><li>foo<br></ol><p>{}<br>" compare innerHTML]
- expected: FAIL
-
- [[["defaultparagraphseparator","p"\],["delete",""\]\] "<ol><li>foo<br></ol><p>{}<br>" compare innerHTML]
- expected: FAIL
-
[delete.html?2001-3000]
expected:
diff --git a/testing/web-platform/tests/editing/data/delete.js b/testing/web-platform/tests/editing/data/delete.js
@@ -3500,4 +3500,10 @@ var browserTests = [
"abdef",
[true],
{}],
+
+["<p>abc<br> </p><p>{}<br></p>",
+ [["delete",""]],
+ "<p>abc</p>",
+ [true],
+ {}],
]
diff --git a/testing/web-platform/tests/editing/data/forwarddelete.js b/testing/web-platform/tests/editing/data/forwarddelete.js
@@ -3362,4 +3362,10 @@ var browserTests = [
"abcef",
[true],
{}],
+
+["<p>abc{}<br> </p><p><br></p>",
+ [["forwarddelete",""]],
+ "<p>abc</p>",
+ [true],
+ {}],
]
