commit fc1439912b28d700739d81fffb3fe8eb7d5388a7
parent c12ca56703967736a197cf534209e480e104d7d1
Author: Masayuki Nakano <masayuki@d-toybox.com>
Date: Mon, 20 Oct 2025 23:35:30 +0000
Bug 1994615 - Make `AutoBlockElementsJoiner::JoinBlockElementsInSameParent` delete unnecessary invisible line break from the left content first r=m_kato
The new behavior is compatible with Chrome. However, if the `<br>` is
immediately after a child block boundary, the result is different from
Chrome. However, I'm not sure whether Chrome's behavior is better
because users won't be able to insert new content where the empty line
was. Therefore, this patch does not fix the latter behavior.
Differential Revision: https://phabricator.services.mozilla.com/D268999
Diffstat:
6 files changed, 68 insertions(+), 25 deletions(-)
diff --git a/editor/libeditor/HTMLEditorDeleteHandler.cpp b/editor/libeditor/HTMLEditorDeleteHandler.cpp
@@ -3581,6 +3581,34 @@ Result<EditActionResult, nsresult> HTMLEditor::AutoDeleteRangesHandler::
nsIEditor::DirectionIsBackspace(aDirectionAndAmount);
const OwningNonNull<nsRange> rangeToDelete(aRangeToDelete);
+
+ // If mLeftContent ends with an invisible line break, we should delete it
+ // before joining the blocks because that will appear as a visible line break
+ // if JoinNodesDeepWithTransaction() moves the first line content of
+ // mRightContent to end of the line break in mLeftContent.
+ if (HTMLEditUtils::IsBlockElement(
+ *mLeftContent, BlockInlineCheck::UseComputedDisplayOutsideStyle)) {
+ const WSScanResult lastThingInLeftBlock =
+ WSRunScanner::ScanPreviousVisibleNodeOrBlockBoundary(
+ WSRunScanner::Scan::All, EditorRawDOMPoint::AtEndOf(*mLeftContent),
+ BlockInlineCheck::UseComputedDisplayOutsideStyle);
+ if (lastThingInLeftBlock.ReachedLineBreak()) {
+ const EditorLineBreak lineBreak =
+ lastThingInLeftBlock.CreateEditorLineBreak<EditorLineBreak>();
+ // FIXME: If the line break is wrapped in an non-editable inline element,
+ // we should delete its parent too or should fail.
+ Result<EditorDOMPoint, nsresult> exLineBreakPointOrError =
+ aHTMLEditor.DeleteLineBreakWithTransaction(
+ lineBreak, nsIEditor::eNoStrip, aEditingHost);
+ if (MOZ_UNLIKELY(exLineBreakPointOrError.isErr())) {
+ NS_WARNING("HTMLEditor::DeleteLineBreakWithTransaction() failed");
+ return exLineBreakPointOrError.propagateErr();
+ }
+ }
+ }
+
+ // Then, normalizer white-spaces at end of mLeftContent and at start of
+ // mRightContent to keep their visibility.
Result<EditorDOMRange, nsresult> rangeToDeleteOrError =
WhiteSpaceVisibilityKeeper::NormalizeSurroundingWhiteSpacesToJoin(
aHTMLEditor, EditorDOMRange(*rangeToDelete));
@@ -3595,6 +3623,9 @@ Result<EditActionResult, nsresult> HTMLEditor::AutoDeleteRangesHandler::
NS_WARNING("EditorDOMRange::SetToRange() failed");
return Err(rv);
}
+
+ // Finally, delete the selected content and move first line of mRightContent
+ // to end of mLeftContent.
if (!rangeToDelete->Collapsed()) {
AutoClonedSelectionRangeArray rangesToDelete(rangeToDelete,
aLimitersAndCaretData);
diff --git a/editor/libeditor/WSRunScanner.h b/editor/libeditor/WSRunScanner.h
@@ -8,7 +8,8 @@
#include "EditorBase.h"
#include "EditorForwards.h"
-#include "EditorDOMPoint.h" // for EditorDOMPoint
+#include "EditorDOMPoint.h" // for EditorDOMPoint
+#include "EditorLineBreak.h" // for EditorLineBreakBase
#include "HTMLEditor.h"
#include "HTMLEditUtils.h"
@@ -162,6 +163,18 @@ class MOZ_STACK_CLASS WSScanResult final {
return mContent->AsText();
}
+ template <typename EditorLineBreakType>
+ MOZ_NEVER_INLINE_DEBUG EditorLineBreakType CreateEditorLineBreak() const {
+ if (ReachedBRElement()) {
+ return EditorLineBreakType(*BRElementPtr());
+ }
+ if (ReachedPreformattedLineBreak()) {
+ return EditorLineBreakType(*TextPtr(), *mOffset);
+ }
+ MOZ_CRASH("Didn't reach a line break");
+ return EditorLineBreakType(*BRElementPtr());
+ }
+
/**
* Returns true if found or reached content is editable.
*/
@@ -288,6 +301,15 @@ class MOZ_STACK_CLASS WSScanResult final {
}
/**
+ * Return true if reached a <br> element or a preformatted line break.
+ * Return false when reached a block boundary. Use ReachedLineBoundary() if
+ * you want it to return true in the case too.
+ */
+ [[nodiscard]] bool ReachedLineBreak() const {
+ return ReachedBRElement() || ReachedPreformattedLineBreak();
+ }
+
+ /**
* The scanner reached a <hr> element.
*/
bool ReachedHRElement() const {
diff --git a/testing/web-platform/meta/editing/run/delete.html.ini b/testing/web-platform/meta/editing/run/delete.html.ini
@@ -456,18 +456,6 @@
[[["defaultparagraphseparator","p"\],["delete",""\]\] "foo[<div><p>\]bar</div>" compare innerHTML]
expected: FAIL
- [[["defaultparagraphseparator","div"\],["delete",""\]\] "<p>foo<br>{</p><p>}bar</p>" compare innerHTML]
- expected: FAIL
-
- [[["defaultparagraphseparator","p"\],["delete",""\]\] "<p>foo<br>{</p><p>}bar</p>" compare innerHTML]
- expected: FAIL
-
- [[["defaultparagraphseparator","div"\],["delete",""\]\] "<p>foo<br><br>{</p><p>}bar</p>" compare innerHTML]
- expected: FAIL
-
- [[["defaultparagraphseparator","p"\],["delete",""\]\] "<p>foo<br><br>{</p><p>}bar</p>" compare innerHTML]
- expected: FAIL
-
[delete.html?6001-7000]
expected:
diff --git a/testing/web-platform/meta/editing/run/forwarddelete.html.ini b/testing/web-platform/meta/editing/run/forwarddelete.html.ini
@@ -390,18 +390,6 @@
[[["defaultparagraphseparator","p"\],["forwarddelete",""\]\] "foo[<div><p>\]bar</div>" compare innerHTML]
expected: FAIL
- [[["defaultparagraphseparator","div"\],["forwarddelete",""\]\] "<p>foo<br>{</p><p>}bar</p>" compare innerHTML]
- expected: FAIL
-
- [[["defaultparagraphseparator","p"\],["forwarddelete",""\]\] "<p>foo<br>{</p><p>}bar</p>" compare innerHTML]
- expected: FAIL
-
- [[["defaultparagraphseparator","div"\],["forwarddelete",""\]\] "<p>foo<br><br>{</p><p>}bar</p>" compare innerHTML]
- expected: FAIL
-
- [[["defaultparagraphseparator","p"\],["forwarddelete",""\]\] "<p>foo<br><br>{</p><p>}bar</p>" compare innerHTML]
- expected: FAIL
-
[[["forwarddelete",""\]\] "<ol><li>fo[o</ol><ol><li>b\]ar</ol>" compare innerHTML]
expected: FAIL
diff --git a/testing/web-platform/tests/editing/data/delete.js b/testing/web-platform/tests/editing/data/delete.js
@@ -3393,4 +3393,11 @@ var browserTests = [
"<div style=\"display:inline-grid\"><span>{}<br></span></div><div>abc</div>",
[true],
{}],
+// XXX I'm not sure which result is better.
+["<div><div>abc</div><br></div><div>[]def</div>",
+ [["delete",""]],
+ ["<div><div>abc</div>def</div>",
+ "<div><div>abc</div></div><div>def</div>"],
+ [true],
+ {}],
]
diff --git a/testing/web-platform/tests/editing/data/forwarddelete.js b/testing/web-platform/tests/editing/data/forwarddelete.js
@@ -3255,4 +3255,11 @@ var browserTests = [
"<div>abc</div><div style=\"display:inline-grid\"><span><br></span></div>",
[true],
{}],
+// XXX I'm not sure which result is better.
+["<div><div>abc</div>{}<br></div><div>def</div>",
+ [["delete",""]],
+ ["<div><div>abc</div>def</div>",
+ "<div><div>abc</div></div><div>def</div>"],
+ [true],
+ {}],
]
