Bug 1940653 - Make HTMLEditor::HandlerInsertText insert one Text when linefeeds are preformatted r=m_kato

The form of <https://discussions.apple.com/> handles newly inserted `Text` nodes
asynchronously after pasted.  Then, the text will be wrapped into `<p>`
elements.  However, the handler does not assume that multiple and consecutive
`Text` nodes are inserted by a pasting.  Therefore, they fail handling our
pasted text which was split to each line and each preformatted linefeed.

Chrome puts only one `Text` node at pasting multiline plaintext and following
this behavior fixes the issue in the form.  Therefore, we should follow the
behavior at least for now.

Differential Revision: https://phabricator.services.mozilla.com/D233619
This commit is contained in:
Masayuki Nakano
2025-01-10 12:14:58 +00:00
parent 0bc88f5a1d
commit fa19f63af9
4 changed files with 183 additions and 68 deletions

View File

@@ -79,13 +79,16 @@ SimpleTest.waitForFocus(async () => {
requestAnimationFrame(() => requestAnimationFrame(resolve));
});
const middleOfText = collapseAt > 0 && collapseAt < "XYZ".length;
const expectSplit = middleOfText && whiteSpace == "normal";
const firstLineLength = insertingText.indexOf("\n");
const rightTextLength = middleOfText
// First, the `Text` is split. At this time, the right half is removed from the
// `Text` and then, added it with new `Text`. Therefore, there is non-zero removed
const rightTextLength = expectSplit
// First, the `Text` is split if the linefeed is not preformatted.
// At this time, the right half is removed from the `Text` and then,
// added it with new `Text`. Therefore, there is non-zero removed
// length.
? "XYZ".length - collapseAt
// However, if collapsed at start or end of a `Text`, we don't split the `Text`.
// However, if the linefeeds are preformatted or
// collapsed at start or end of a `Text`, we don't split the `Text`.
: 0;
is(
stringifyTextChanges(textChanges),
@@ -106,16 +109,19 @@ SimpleTest.waitForFocus(async () => {
requestAnimationFrame(() => requestAnimationFrame(resolve));
});
// Finally, removing the inserted `Text` nodes and <br> elements causes joining the
// `Text` nodes around the inserted text. Therefore, the offset is always 0. Removed
// length is the inserted text length and right text length at first split. Then, added
// text length is the right text length.
const joinText = middleOfText || (firstLineLength > 0 && collapseAt == "XYZ".length);
const removeDataFromText = collapseAt == "XYZ".length && firstLineLength > 0;
// `Text` nodes around the inserted text. So, if the new text is just inserted, the
// text change data is same as above. On the other hand, if we did split, the offset
// is always 0, the removed length is the inserted text length and right text length
// at first split. Then, added text length is the right text length.
const joinText = expectSplit &&
(middleOfText || (firstLineLength > 0 && collapseAt == "XYZ".length));
const removeDataFromText =
expectSplit && collapseAt == "XYZ".length && firstLineLength > 0;
is(
stringifyTextChanges(textChanges),
stringifyTextChanges([
{
offset: collapseAt == "XYZ".length ? collapseAt : 0,
offset: !joinText || collapseAt == "XYZ".length ? collapseAt : 0,
removedLength: insertingText.length + (joinText && !removeDataFromText ? "XYZ".length : 0),
addedLength: joinText && !removeDataFromText ? "XYZ".length : 0,
},

View File

@@ -1357,67 +1357,98 @@ Result<EditActionResult, nsresult> HTMLEditor::HandleInsertText(
// its a lot cheaper to search the input string for only newlines than
// it is to search for both tabs and newlines.
if (!isWhiteSpaceCollapsible || IsPlaintextMailComposer()) {
uint32_t nextOffset = 0;
while (nextOffset < aInsertionString.Length()) {
const uint32_t lineStartOffset = nextOffset;
const int32_t inclusiveNextLinefeedOffset =
aInsertionString.FindChar(nsCRT::LF, lineStartOffset);
const uint32_t lineLength =
inclusiveNextLinefeedOffset != -1
? static_cast<uint32_t>(inclusiveNextLinefeedOffset) -
lineStartOffset
: aInsertionString.Length() - lineStartOffset;
if (lineLength) {
// lineText does not include the preformatted line break.
const nsDependentSubstring lineText(aInsertionString, lineStartOffset,
lineLength);
Result<InsertTextResult, nsresult> insertTextResult =
InsertTextWithTransaction(
*document, lineText, currentPoint,
GetInsertTextTo(inclusiveNextLinefeedOffset,
lineStartOffset));
if (MOZ_UNLIKELY(insertTextResult.isErr())) {
NS_WARNING("HTMLEditor::InsertTextWithTransaction() failed");
return insertTextResult.propagateErr();
}
// Ignore the caret suggestion because of `dontChangeMySelection`
// above.
insertTextResult.inspect().IgnoreCaretPointSuggestion();
if (insertTextResult.inspect().Handled()) {
pointToInsert = currentPoint = insertTextResult.unwrap()
.EndOfInsertedTextRef()
.To<EditorDOMPoint>();
} else {
pointToInsert = currentPoint;
}
if (inclusiveNextLinefeedOffset < 0) {
break; // We reached the last line
}
if (*lineBreakType == LineBreakType::Linefeed) {
// Both Chrome and us inserts a preformatted linefeed with its own
// `Text` node in various cases. However, when inserting multiline
// text, we should insert a `Text` because Chrome does so and the
// comment field in https://discussions.apple.com/ handles the new
// `Text` to split each line into a paragraph. At that time, it's
// not assumed that inserted text is split at every linefeed.
MOZ_ASSERT(*lineBreakType == LineBreakType::Linefeed);
Result<InsertTextResult, nsresult> insertTextResult =
InsertTextWithTransaction(
*document, aInsertionString, currentPoint,
InsertTextTo::ExistingTextNodeIfAvailable);
if (MOZ_UNLIKELY(insertTextResult.isErr())) {
NS_WARNING("HTMLEditor::InsertTextWithTransaction() failed");
return insertTextResult.propagateErr();
}
MOZ_ASSERT(inclusiveNextLinefeedOffset >= 0);
Result<CreateLineBreakResult, nsresult> insertLineBreakResultOrError =
InsertLineBreak(WithTransaction::Yes, *lineBreakType, currentPoint);
if (MOZ_UNLIKELY(insertLineBreakResultOrError.isErr())) {
NS_WARNING(nsPrintfCString("HTMLEditor::InsertLineBreak("
"WithTransaction::Yes, %s) failed",
ToString(*lineBreakType).c_str())
.get());
return insertLineBreakResultOrError.propagateErr();
// Ignore the caret suggestion because of `dontChangeMySelection`
// above.
insertTextResult.inspect().IgnoreCaretPointSuggestion();
if (insertTextResult.inspect().Handled()) {
pointToInsert = currentPoint = insertTextResult.unwrap()
.EndOfInsertedTextRef()
.To<EditorDOMPoint>();
} else {
pointToInsert = currentPoint;
}
CreateLineBreakResult insertLineBreakResult =
insertLineBreakResultOrError.unwrap();
// We don't want to update selection here because we've blocked
// InsertNodeTransaction updating selection with
// dontChangeMySelection.
insertLineBreakResult.IgnoreCaretPointSuggestion();
MOZ_ASSERT(!AllowsTransactionsToChangeSelection());
} else {
MOZ_ASSERT(*lineBreakType == LineBreakType::BRElement);
uint32_t nextOffset = 0;
while (nextOffset < aInsertionString.Length()) {
const uint32_t lineStartOffset = nextOffset;
const int32_t inclusiveNextLinefeedOffset =
aInsertionString.FindChar(nsCRT::LF, lineStartOffset);
const uint32_t lineLength =
inclusiveNextLinefeedOffset != -1
? static_cast<uint32_t>(inclusiveNextLinefeedOffset) -
lineStartOffset
: aInsertionString.Length() - lineStartOffset;
if (lineLength) {
// lineText does not include the preformatted line break.
const nsDependentSubstring lineText(aInsertionString,
lineStartOffset, lineLength);
Result<InsertTextResult, nsresult> insertTextResult =
InsertTextWithTransaction(
*document, lineText, currentPoint,
GetInsertTextTo(inclusiveNextLinefeedOffset,
lineStartOffset));
if (MOZ_UNLIKELY(insertTextResult.isErr())) {
NS_WARNING("HTMLEditor::InsertTextWithTransaction() failed");
return insertTextResult.propagateErr();
}
// Ignore the caret suggestion because of `dontChangeMySelection`
// above.
insertTextResult.inspect().IgnoreCaretPointSuggestion();
if (insertTextResult.inspect().Handled()) {
pointToInsert = currentPoint = insertTextResult.unwrap()
.EndOfInsertedTextRef()
.To<EditorDOMPoint>();
} else {
pointToInsert = currentPoint;
}
if (inclusiveNextLinefeedOffset < 0) {
break; // We reached the last line
}
}
MOZ_ASSERT(inclusiveNextLinefeedOffset >= 0);
Result<CreateLineBreakResult, nsresult> insertLineBreakResultOrError =
InsertLineBreak(WithTransaction::Yes, *lineBreakType,
currentPoint);
if (MOZ_UNLIKELY(insertLineBreakResultOrError.isErr())) {
NS_WARNING(nsPrintfCString("HTMLEditor::InsertLineBreak("
"WithTransaction::Yes, %s) failed",
ToString(*lineBreakType).c_str())
.get());
return insertLineBreakResultOrError.propagateErr();
}
CreateLineBreakResult insertLineBreakResult =
insertLineBreakResultOrError.unwrap();
// We don't want to update selection here because we've blocked
// InsertNodeTransaction updating selection with
// dontChangeMySelection.
insertLineBreakResult.IgnoreCaretPointSuggestion();
MOZ_ASSERT(!AllowsTransactionsToChangeSelection());
nextOffset = inclusiveNextLinefeedOffset + 1;
pointToInsert = insertLineBreakResult.AfterLineBreak<EditorDOMPoint>();
currentPoint.SetAfter(&insertLineBreakResult.LineBreakContentRef());
if (NS_WARN_IF(!pointToInsert.IsSetAndValidInComposedDoc()) ||
NS_WARN_IF(!currentPoint.IsSetAndValidInComposedDoc())) {
return Err(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE);
nextOffset = inclusiveNextLinefeedOffset + 1;
pointToInsert =
insertLineBreakResult.AfterLineBreak<EditorDOMPoint>();
currentPoint.SetAfter(&insertLineBreakResult.LineBreakContentRef());
if (NS_WARN_IF(!pointToInsert.IsSetAndValidInComposedDoc()) ||
NS_WARN_IF(!currentPoint.IsSetAndValidInComposedDoc())) {
return Err(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE);
}
}
}
} else {

View File

@@ -127,6 +127,24 @@ class EditorTestUtils {
);
}
sendPasteAsPlaintextShortcutKey() {
// Ctrl/Cmd - Shift - v on Chrome and Firefox
// Cmd - Alt - Shift - v on Safari
const accel = this.window.navigator.platform.includes("Mac") ? this.kMeta : this.kControl;
const isSafari = this.window.navigator.userAgent.includes("Safari");
let actions = new this.window.test_driver.Actions();
actions = actions.keyDown(accel).keyDown(this.kShift);
if (isSafari) {
actions = actions.keyDown(this.kAlt);
}
actions = actions.keyDown("v").keyUp("v");
actions = actions.keyUp(accel).keyUp(this.kShift);
if (isSafari) {
actions = actions.keyUp(this.kAlt);
}
return actions.send();
}
// Similar to `setupDiv` in editing/include/tests.js, this method sets
// innerHTML value of this.editingHost, and sets multiple selection ranges
// specified with the markers.

View File

@@ -0,0 +1,60 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<meta name="variant" content="?white-space=pre">
<meta name="variant" content="?white-space=pre-line">
<meta name="variant" content="?white-space=pre-wrap">
<title>Inserting multiline text shouldn't be split to multiple Text nodes unless using br elements</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<script src="../include/editor-test-utils.js"></script>
<script>
"use strict";
const searchParams = new URLSearchParams(document.location.search);
const whiteSpace = searchParams.get("white-space");
document.addEventListener("DOMContentLoaded", () => {
promise_test(async () => {
const editingHost = document.querySelector("div[contenteditable]");
editingHost.style.whiteSpace = whiteSpace;
const utils = new EditorTestUtils(editingHost);
const pre = document.querySelector("pre");
await test_driver.click(pre); // Ensure user activation
getSelection().selectAllChildren(pre);
await utils.sendCopyShortcutKey();
editingHost.focus();
utils.setupEditingHost("<p>{}<br></p>");
await utils.sendPasteAsPlaintextShortcutKey();
if (editingHost.innerHTML == "<p>abc<br>def<br>ghi</p>") {
// It's fine to use <br> for line breaks, at least, out of scope of this test.
assert_equals(editingHost.innerHTML, "<p>abc<br>def<br>ghi</p>");
return;
}
// The form in https://discussions.apple.com/ expects that pasted text is
// not split at each linefeed. For backward compatibility, browsers need
// to keep this behavior.
assert_equals(editingHost.innerHTML, "<p>abc\ndef\nghi</p>");
assert_equals(
editingHost.querySelector("p").childNodes.length,
1,
"Pasted text should be in a single Text node"
);
});
}, {once: true});
</script>
</head>
<body>
<pre>abc
def
ghi</pre>
<div contenteditable="true"></div>
</body>
</html>