Bug 1764895 - part 1: Make nsIEditor.insertNode and nsIEditor.deleteNode take an optional parameter to preserve selection r=m_kato

`nsIEditor.setShouldTxnSetSelection` can preserve selection across multiple
editing.  Therefore, we cannot manage the state only in the stack.

It's used only in comm-central, and used only with `insertNode` and
`deleteNode`.  Therefore, adding new param to them to preserve selection
must be enough.

While I'm writing this patch, I realized that `input` event is not fired
by these methods because nobody set a placeholder transaction.  That may
lead Thunderbird only IME crash bugs due to `IMEContentObserver` is not
notified editor properly.  Therefore, this may fix some Thunderbird only
crashes.

Note that `deleteNode` should not update selection.  However, I'm not 100%
sure that.  Therefore, I add new param to `deleteNode` too.  However,
some reviewers think it's unnecessary, I'll remove it before landing.

Finally, `beforeinput` and `input` caused by the method calls start updating
selection.  However, I think that it should be better behavior.  If Thunderbird
needs to guarantee that selection is set to whether the user expected when
it calls these methods with preserving selection.

Differential Revision: https://phabricator.services.mozilla.com/D196004
This commit is contained in:
Masayuki Nakano 2023-12-13 07:50:42 +00:00
parent 90b77204fb
commit 05bce4692d
7 changed files with 533 additions and 33 deletions

View file

@ -2287,8 +2287,13 @@ NS_IMETHODIMP EditorBase::SetSpellcheckUserOverride(bool enable) {
}
NS_IMETHODIMP EditorBase::InsertNode(nsINode* aNodeToInsert,
nsINode* aContainer, uint32_t aOffset) {
nsCOMPtr<nsIContent> contentToInsert = do_QueryInterface(aNodeToInsert);
nsINode* aContainer, uint32_t aOffset,
bool aPreserveSelection,
uint8_t aOptionalArgCount) {
MOZ_DIAGNOSTIC_ASSERT(IsHTMLEditor());
nsCOMPtr<nsIContent> contentToInsert =
nsIContent::FromNodeOrNull(aNodeToInsert);
if (NS_WARN_IF(!contentToInsert) || NS_WARN_IF(!aContainer)) {
return NS_ERROR_NULL_POINTER;
}
@ -2301,6 +2306,17 @@ NS_IMETHODIMP EditorBase::InsertNode(nsINode* aNodeToInsert,
return EditorBase::ToGenericNSResult(rv);
}
// Make dispatch `input` event after stopping preserving selection.
AutoPlaceholderBatch treatAsOneTransaction(
*this,
ScrollSelectionIntoView::No, // not a user interaction
__FUNCTION__);
Maybe<AutoTransactionsConserveSelection> preseveSelection;
if (aOptionalArgCount && aPreserveSelection) {
preseveSelection.emplace(*this);
}
const uint32_t offset = std::min(aOffset, aContainer->Length());
Result<CreateContentResult, nsresult> insertContentResult =
InsertNodeWithTransaction(*contentToInsert,
@ -2411,23 +2427,10 @@ EditorBase::InsertPaddingBRElementForEmptyLastLineWithTransaction(
return insertBRElementResult;
}
NS_IMETHODIMP EditorBase::DeleteNode(nsINode* aNode) {
if (NS_WARN_IF(!aNode) || NS_WARN_IF(!aNode->IsContent())) {
return NS_ERROR_INVALID_ARG;
}
AutoEditActionDataSetter editActionData(*this, EditAction::eRemoveNode);
nsresult rv = editActionData.CanHandleAndMaybeDispatchBeforeInputEvent();
if (NS_FAILED(rv)) {
NS_WARNING_ASSERTION(rv == NS_ERROR_EDITOR_ACTION_CANCELED,
"CanHandleAndMaybeDispatchBeforeInputEvent() failed");
return EditorBase::ToGenericNSResult(rv);
}
rv = DeleteNodeWithTransaction(MOZ_KnownLive(*aNode->AsContent()));
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"EditorBase::DeleteNodeWithTransaction() failed");
return EditorBase::ToGenericNSResult(rv);
NS_IMETHODIMP EditorBase::DeleteNode(nsINode* aNode, bool aPreserveSelection,
uint8_t aOptionalArgCount) {
MOZ_ASSERT_UNREACHABLE("Do not use this API with TextEditor");
return NS_ERROR_NOT_IMPLEMENTED;
}
nsresult EditorBase::DeleteNodeWithTransaction(nsIContent& aContent) {

View file

@ -3960,7 +3960,8 @@ nsresult HTMLEditor::DeleteAllChildrenWithTransaction(Element& aElement) {
return NS_OK;
}
NS_IMETHODIMP HTMLEditor::DeleteNode(nsINode* aNode) {
NS_IMETHODIMP HTMLEditor::DeleteNode(nsINode* aNode, bool aPreserveSelection,
uint8_t aOptionalArgCount) {
if (NS_WARN_IF(!aNode) || NS_WARN_IF(!aNode->IsContent())) {
return NS_ERROR_INVALID_ARG;
}
@ -3973,6 +3974,17 @@ NS_IMETHODIMP HTMLEditor::DeleteNode(nsINode* aNode) {
return EditorBase::ToGenericNSResult(rv);
}
// Make dispatch `input` event after stopping preserving selection.
AutoPlaceholderBatch treatAsOneTransaction(
*this,
ScrollSelectionIntoView::No, // not a user interaction
__FUNCTION__);
Maybe<AutoTransactionsConserveSelection> preserveSelection;
if (aOptionalArgCount && aPreserveSelection) {
preserveSelection.emplace(*this);
}
rv = DeleteNodeWithTransaction(MOZ_KnownLive(*aNode->AsContent()));
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"EditorBase::DeleteNodeWithTransaction() failed");

View file

@ -174,7 +174,9 @@ class HTMLEditor final : public EditorBase,
bool CanPaste(int32_t aClipboardType) const final;
using EditorBase::CanPaste;
MOZ_CAN_RUN_SCRIPT NS_IMETHOD DeleteNode(nsINode* aNode) final;
MOZ_CAN_RUN_SCRIPT NS_IMETHOD DeleteNode(nsINode* aNode,
bool aPreseveSelection,
uint8_t aOptionalArgCount) final;
MOZ_CAN_RUN_SCRIPT NS_IMETHOD InsertLineBreak() final;

View file

@ -490,12 +490,16 @@ skip-if = ["xorigin"] # Testing internal API for comm-central
["test_nsIEditor_clearUndoRedo.html"]
["test_nsIEditor_deleteNode.html"]
["test_nsIEditor_documentCharacterSet.html"]
["test_nsIEditor_documentIsEmpty.html"]
["test_nsIEditor_insertLineBreak.html"]
["test_nsIEditor_insertNode.html"]
["test_nsIEditor_isSelectionEditable.html"]
["test_nsIEditor_outputToString.html"]

View file

@ -0,0 +1,242 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>nsIEditor.insertNode</title>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
<script>
"use strict";
function stringifyInputEvent(aEvent) {
if (!aEvent) {
return "null";
}
return `${aEvent.type}: { inputType=${aEvent.inputType} }`;
}
function getRangeDescription(range) {
function getNodeDescription(node) {
if (!node) {
return "null";
}
switch (node.nodeType) {
case Node.TEXT_NODE:
return `${node.nodeName} "${node.data}"`;
case Node.ELEMENT_NODE:
return `<${node.nodeName.toLowerCase()}>`;
default:
return `${node.nodeName}`;
}
}
if (range === null) {
return "null";
}
if (range === undefined) {
return "undefined";
}
return range.startContainer == range.endContainer &&
range.startOffset == range.endOffset
? `(${getNodeDescription(range.startContainer)}, ${range.startOffset})`
: `(${getNodeDescription(range.startContainer)}, ${
range.startOffset
}) - (${getNodeDescription(range.endContainer)}, ${range.endOffset})`;
}
SimpleTest.waitForExplicitFinish();
SimpleTest.waitForFocus(() => {
const editingHost = document.querySelector("div[contenteditable]");
const editor =
SpecialPowers.wrap(window).docShell.editingSession.getEditorForWindow(window);
editingHost.focus();
let events = [];
editingHost.addEventListener("input", event => events.push(event));
(function test_delete_node_before_selection() {
editingHost.innerHTML = "<span>abc</span><span>def</span>";
getSelection().collapse(editingHost.querySelector("span + span").firstChild, 0);
editor.deleteNode(editingHost.querySelector("span"));
is(
editingHost.innerHTML,
"<span>def</span>",
"test_delete_node_before_selection: deleteNode() should delete the node"
);
is(
events.length,
1,
"test_delete_node_before_selection: Only one input event should be fired when deleteNode() deletes a node"
);
is(
stringifyInputEvent(events[0]),
stringifyInputEvent({ type: "input", inputType: "" }),
"test_delete_node_before_selection: input event should be fired when deleting a node"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost.firstChild.firstChild,
startOffset: 0,
endContainer: editingHost.firstChild.firstChild,
endOffset: 0,
}),
"test_delete_node_before_selection: selection shouldn't be updated"
);
})();
(function test_delete_node_after_selection() {
events = [];
editingHost.innerHTML = "<span>abc</span><span>def</span>";
getSelection().collapse(editingHost.querySelector("span").firstChild, 0);
editor.deleteNode(editingHost.querySelector("span + span"));
is(
editingHost.innerHTML,
"<span>abc</span>",
"test_delete_node_after_selection: deleteNode() should delete the node"
);
is(
events.length,
1,
"test_delete_node_after_selection: Only one input event should be fired when deleteNode() deletes a node"
);
is(
stringifyInputEvent(events[0]),
stringifyInputEvent({ type: "input", inputType: "" }),
"test_delete_node_after_selection: input event should be fired when deleting a node"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost.firstChild.firstChild,
startOffset: 0,
endContainer: editingHost.firstChild.firstChild,
endOffset: 0,
}),
"test_delete_node_after_selection: selection shouldn't be updated"
);
})();
(function test_delete_node_containing_selection() {
events = [];
editingHost.innerHTML = "<span>abc</span><span>def</span>";
getSelection().collapse(editingHost.querySelector("span").firstChild, 0);
editor.deleteNode(editingHost.querySelector("span"));
is(
editingHost.innerHTML,
"<span>def</span>",
"test_delete_node_containing_selection: deleteNode() should delete the node"
);
is(
events.length,
1,
"test_delete_node_containing_selection: Only one input event should be fired when deleteNode() deletes a node"
);
is(
stringifyInputEvent(events[0]),
stringifyInputEvent({ type: "input", inputType: "" }),
"test_delete_node_containing_selection: input event should be fired when deleting a node"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost,
startOffset: 0,
endContainer: editingHost,
endOffset: 0,
}),
"test_delete_node_containing_selection: selection should be updated whether node was"
);
})();
(function test_delete_node_containing_selection_with_preserving_selection() {
events = [];
editingHost.innerHTML = "<span>abc</span><span>def</span>";
getSelection().collapse(editingHost.querySelector("span").firstChild, 0);
editor.deleteNode(editingHost.querySelector("span"), true);
is(
editingHost.innerHTML,
"<span>def</span>",
"test_delete_node_containing_selection_with_preserving_selection: deleteNode() should delete the node"
);
is(
events.length,
1,
"test_delete_node_containing_selection_with_preserving_selection: Only one input event should be fired when deleteNode() deletes a node"
);
is(
stringifyInputEvent(events[0]),
stringifyInputEvent({ type: "input", inputType: "" }),
"test_delete_node_containing_selection_with_preserving_selection: input event should be fired when deleting a node"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost,
startOffset: 0,
endContainer: editingHost,
endOffset: 0,
}),
"test_delete_node_containing_selection_with_preserving_selection: selection should be updated whether node was"
);
})();
(function test_not_preserve_selection_nested_by_beforeinput() {
editingHost.innerHTML = "<span>abc</span><span>ghi</span>";
const span = document.createElement("span");
span.textContent = "def";
getSelection().collapse(editingHost, 0);
editingHost.addEventListener("beforeinput", () => {
editor.insertNode(span, editingHost, 1);
}, {once: true});
editor.deleteNode(editingHost.querySelector("span + span"), true);
is(
editingHost.innerHTML,
"<span>abc</span><span>def</span>",
"test_not_preserve_selection_nested_by_beforeinput: both insertNode() and deleteNode() should work"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost,
startOffset: 2,
endContainer: editingHost,
endOffset: 2,
}),
"test_not_preserve_selection_nested_by_beforeinput: only insertNode() called in beforeinput listener should update selection"
);
})();
(function test_not_preserve_selection_nested_by_input() {
editingHost.innerHTML = "<span>abc</span><span>ghi</span>";
const span = document.createElement("span");
span.textContent = "def";
getSelection().collapse(editingHost, 0);
editingHost.addEventListener("input", () => {
editor.insertNode(span, editingHost, 1);
}, {once: true});
editor.deleteNode(editingHost.querySelector("span + span"), true);
is(
editingHost.innerHTML,
"<span>abc</span><span>def</span>",
"test_not_preserve_selection_nested_by_input: both insertNode() and deleteNode() should work"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost,
startOffset: 2,
endContainer: editingHost,
endOffset: 2,
}),
"test_not_preserve_selection_nested_by_input: only insertNode() called in input listener should update selection"
);
})();
SimpleTest.finish();
});
</script>
</head>
<body><div contenteditable><br></div></body>
</html>

View file

@ -0,0 +1,214 @@
<!doctype>
<html>
<head>
<meta charset="utf-8">
<title>nsIEditor.insertNode</title>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
<script>
"use strict";
function stringifyInputEvent(aEvent) {
if (!aEvent) {
return "null";
}
return `${aEvent.type}: { inputType=${aEvent.inputType} }`;
}
function getRangeDescription(range) {
function getNodeDescription(node) {
if (!node) {
return "null";
}
switch (node.nodeType) {
case Node.TEXT_NODE:
return `${node.nodeName} "${node.data}"`;
case Node.ELEMENT_NODE:
return `<${node.nodeName.toLowerCase()}>`;
default:
return `${node.nodeName}`;
}
}
if (range === null) {
return "null";
}
if (range === undefined) {
return "undefined";
}
return range.startContainer == range.endContainer &&
range.startOffset == range.endOffset
? `(${getNodeDescription(range.startContainer)}, ${range.startOffset})`
: `(${getNodeDescription(range.startContainer)}, ${
range.startOffset
}) - (${getNodeDescription(range.endContainer)}, ${range.endOffset})`;
}
SimpleTest.waitForExplicitFinish();
SimpleTest.waitForFocus(() => {
const editingHost = document.querySelector("div[contenteditable]");
const editor =
SpecialPowers.wrap(window).docShell.editingSession.getEditorForWindow(window);
editingHost.focus();
let events = [];
editingHost.addEventListener("input", event => events.push(event));
(function test_insert_text_to_start() {
editor.insertNode(document.createTextNode("abc"), editingHost, 0);
is(
editingHost.innerHTML,
"abc<br>",
"test_insert_text_to_start: insertNode() should insert new text node at start of the container"
);
is(
events.length,
1,
"test_insert_text_to_start: Only one input event should be fired when insertNode() inserts a text node"
);
is(
stringifyInputEvent(events[0]),
stringifyInputEvent({ type: "input", inputType: "" }),
"test_insert_text_to_start: input event should be fired when inserting a node"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost,
startOffset: 1,
endContainer: editingHost,
endOffset: 1,
}),
"test_insert_text_to_start: insertNode() should collapse selection after the inserted text node"
);
})();
(function test_insert_span_to_big_index() {
events = [];
editingHost.innerHTML = "abc";
const span = document.createElement("span");
span.textContent = "def";
editor.insertNode(span, editingHost, 1000);
is(
editingHost.innerHTML,
"abc<span>def</span>",
"test_insert_span_to_big_index: insertNode() with big index should insert new node at end of the container"
);
is(
events.length,
1,
"test_insert_span_to_big_index: Only one input event should be fired when insertNode() inserts a node"
);
is(
stringifyInputEvent(events[0]),
stringifyInputEvent({ type: "input", inputType: "" }),
"test_insert_span_to_big_index: input event should be fired when inserting a node"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost,
startOffset: 2,
endContainer: editingHost,
endOffset: 2,
}),
"test_insert_span_to_big_index: insertNode() should collapse selection after the inserted node"
);
})();
(function test_preserve_selection() {
events = [];
editingHost.innerHTML = "abc";
const span = document.createElement("span");
span.textContent = "def";
getSelection().collapse(editingHost, 0);
editor.insertNode(span, editingHost, 1, true);
is(
editingHost.innerHTML,
"abc<span>def</span>",
"test_preserve_selection: insertNode() should insert new node at end of the container"
);
is(
events.length,
1,
"test_preserve_selection: Only one input event should be fired when insertNode() inserts a node"
);
is(
stringifyInputEvent(events[0]),
stringifyInputEvent({ type: "input", inputType: "" }),
"test_preserve_selection: input event should be fired when inserting a node"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost,
startOffset: 0,
endContainer: editingHost,
endOffset: 0,
}),
"test_preserve_selection: insertNode() should not collapse selection after the inserted node"
);
})();
(function test_not_preserve_selection_nested_by_beforeinput() {
editingHost.innerHTML = "abc";
const span1 = document.createElement("span");
span1.textContent = "def";
const span2 = document.createElement("span");
span2.textContent = "ghi";
getSelection().collapse(editingHost, 0);
editingHost.addEventListener("beforeinput", () => {
editor.insertNode(span1, editingHost, 1);
}, {once: true});
editor.insertNode(span2, editingHost, 2, true);
is(
editingHost.innerHTML,
"abc<span>def</span><span>ghi</span>",
"test_not_preserve_selection_nested_by_beforeinput: both insertNode() should work"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost,
startOffset: 2,
endContainer: editingHost,
endOffset: 2,
}),
"test_not_preserve_selection_nested_by_beforeinput: only insertNode() called in beforeinput listener should update selection"
);
})();
(function test_not_preserve_selection_nested_by_input() {
editingHost.innerHTML = "abc";
const span1 = document.createElement("span");
span1.textContent = "def";
const span2 = document.createElement("span");
span2.textContent = "ghi";
getSelection().collapse(editingHost, 0);
editingHost.addEventListener("input", () => {
editor.insertNode(span2, editingHost, 2);
}, {once: true});
editor.insertNode(span1, editingHost, 1, true);
is(
editingHost.innerHTML,
"abc<span>def</span><span>ghi</span>",
"test_not_preserve_selection_nested_by_input: both insertNode() should work"
);
is(
getRangeDescription(getSelection().getRangeAt(0)),
getRangeDescription({
startContainer: editingHost,
startOffset: 3,
endContainer: editingHost,
endOffset: 3,
}),
"test_not_preserve_selection_nested_by_input: only insertNode() called in input listener should update selection"
);
})();
SimpleTest.finish();
});
</script>
</head>
<body><div contenteditable><br></div></body>
</html>

View file

@ -476,27 +476,50 @@ interface nsIEditor : nsISupports
void cloneAttributes(in Element aDestElement, in Element aSourceElement);
/**
* insertNode inserts aNode into aParent at aPosition.
* insertNode inserts aNode into aParent at aPosition and this operation is
* undoable.
* No checking is done to verify the legality of the insertion.
* That is the responsibility of the caller.
* @param aNode The DOM Node to insert.
* @param aParent The node to insert the new object into
* @param aPosition The place in aParent to insert the new node
* 0=first child, 1=second child, etc.
* any number > number of current children = last child
* TODO: Move this method to nsIHTMLEditor, TextEditor does not allow chrome
* script to customize its anonymous subtree.
*
* @param aNode The DOM Node to insert.
* @param aParent The node to insert the new object into
* @param aPosition The place in aParent to insert the new node
* 0=first child, 1=second child, etc.
* If larger than number of children of aParent,
* this will append aNode into aParent.
* @param aPreseveSelection The default value is false. If set to true,
* the insert node handler does not update
* Selection.
* FYI: If somebody handles `beforeinput` event or
* `input` event caused by this and it does
* something undoable, selection may be changed by
* that.
*/
[can_run_script]
[optional_argc, can_run_script]
void insertNode(in Node node,
in Node parent,
in unsigned long aPosition);
in unsigned long aPosition,
[optional] in boolean aPreserveSelection);
/**
* deleteNode removes aChild from aParent.
* @param aChild The node to delete
* deleteNode removes aChild from aParent and this operation is undobable.
* TODO: Move this method to nsIHTMLEditor, TextEditor does not allow chrome
* script to customize its anonymous subtree.
*
* @param aChild The node to delete
* @param aPreseveSelection The default value is false. If set to true,
* the insert node handler does not update
* Selection.
* FYI: If somebody handles `beforeinput` event or
* `input` event caused by this and it does
* something undoable, selection may be changed by
* that.
*/
[can_run_script]
void deleteNode(in Node child);
[optional_argc, can_run_script]
void deleteNode(in Node child, [optional] in boolean aPreserveSelection);
/* ------------ Output methods -------------- */