Bug 1467796 - part 1: Split TextEditor::InsertTextAsAction() to itself and TextEditor::InsertTextAsSubAction() for internal use r?m_kato
For
bug 1465702, we need to split TextEditor::InsertTextAsAction() to 2 methods.
One is for root of handling an edit operation. The other is for internal use,
e.g., handling as a part of an edit operation. Therefore, this patch creates
InsertTextAsSubAction() for the internal use.
MozReview-Commit-ID: CIU5zdp0owP
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -2349,17 +2349,17 @@ HTMLEditor::Indent(const nsAString& aInd
return NS_ERROR_FAILURE;
}
// put a space in it so layout will draw the list item
ErrorResult error;
selection->Collapse(RawRangeBoundary(newBQ, 0), error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
- rv = InsertTextAsAction(NS_LITERAL_STRING(" "));
+ rv = InsertTextAsSubAction(NS_LITERAL_STRING(" "));
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
// Reposition selection to before the space character.
firstRange = selection->GetRangeAt(0);
if (NS_WARN_IF(!firstRange)) {
return NS_ERROR_FAILURE;
}
--- a/editor/libeditor/HTMLEditorDataTransfer.cpp
+++ b/editor/libeditor/HTMLEditorDataTransfer.cpp
@@ -1676,16 +1676,17 @@ HTMLEditor::PasteAsPlaintextQuotation(in
return rv;
}
NS_IMETHODIMP
HTMLEditor::InsertTextWithQuotations(const nsAString& aStringToInsert)
{
// The whole operation should be undoable in one transaction:
BeginTransaction();
+ AutoPlaceholderBatch beginBatching(this);
// We're going to loop over the string, collecting up a "hunk"
// that's all the same type (quoted or not),
// Whenever the quotedness changes (or we reach the string's end)
// we will insert the hunk all at once, quoted or non.
static const char16_t cite('>');
bool curHunkIsQuoted = (aStringToInsert.First() == cite);
@@ -1750,17 +1751,17 @@ HTMLEditor::InsertTextWithQuotations(con
// If no newline found, lineStart is now strEnd and we can finish up,
// inserting from curHunk to lineStart then returning.
const nsAString &curHunk = Substring(hunkStart, lineStart);
nsCOMPtr<nsINode> dummyNode;
if (curHunkIsQuoted) {
rv = InsertAsPlaintextQuotation(curHunk, false,
getter_AddRefs(dummyNode));
} else {
- rv = InsertTextAsAction(curHunk);
+ rv = InsertTextAsSubAction(curHunk);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"Failed to insert a line of the quoted text");
}
if (!found) {
break;
}
curHunkIsQuoted = quoted;
hunkStart = lineStart;
@@ -1770,16 +1771,17 @@ HTMLEditor::InsertTextWithQuotations(con
return rv;
}
NS_IMETHODIMP
HTMLEditor::InsertAsQuotation(const nsAString& aQuotedText,
nsINode** aNodeInserted)
{
+ AutoPlaceholderBatch beginBatching(this);
if (IsPlaintextEditor()) {
return InsertAsPlaintextQuotation(aQuotedText, true, aNodeInserted);
}
nsAutoString citation;
return InsertAsCitedQuotation(aQuotedText, citation, false,
aNodeInserted);
}
@@ -1797,17 +1799,16 @@ HTMLEditor::InsertAsPlaintextQuotation(c
*aNodeInserted = nullptr;
}
RefPtr<Selection> selection = GetSelection();
if (NS_WARN_IF(!selection)) {
return NS_ERROR_FAILURE;
}
- AutoPlaceholderBatch beginBatching(this);
AutoTopLevelEditSubActionNotifier maybeTopLevelEditSubAction(
*this, EditSubAction::eInsertQuotation,
nsIEditor::eNext);
// give rules a chance to handle or cancel
EditSubActionInfo subActionInfo(EditSubAction::eInsertElement);
bool cancel, handled;
// Protect the edit rules object from dying
@@ -1855,17 +1856,17 @@ HTMLEditor::InsertAsPlaintextQuotation(c
"Failed to collapse selection into the new node");
}
if (aAddCites) {
rv = InsertWithQuotationsAsSubAction(aQuotedText);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"Failed to insert the text with quotations");
} else {
- rv = InsertTextAsAction(aQuotedText);
+ rv = InsertTextAsSubAction(aQuotedText);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"Failed to insert the quoted text as plain text");
}
// Note that if !aAddCites, aNodeInserted isn't set.
// That's okay because the routines that use aAddCites
// don't need to know the inserted node.
if (aNodeInserted && NS_SUCCEEDED(rv)) {
@@ -1922,29 +1923,30 @@ HTMLEditor::Rewrap(bool aRespectNewlines
}
NS_IMETHODIMP
HTMLEditor::InsertAsCitedQuotation(const nsAString& aQuotedText,
const nsAString& aCitation,
bool aInsertHTML,
nsINode** aNodeInserted)
{
+ AutoPlaceholderBatch beginBatching(this);
+
// Don't let anyone insert HTML when we're in plaintext mode.
if (IsPlaintextEditor()) {
NS_ASSERTION(!aInsertHTML,
"InsertAsCitedQuotation: trying to insert html into plaintext editor");
return InsertAsPlaintextQuotation(aQuotedText, true, aNodeInserted);
}
RefPtr<Selection> selection = GetSelection();
if (NS_WARN_IF(!selection)) {
return NS_ERROR_FAILURE;
}
- AutoPlaceholderBatch beginBatching(this);
AutoTopLevelEditSubActionNotifier maybeTopLevelEditSubAction(
*this, EditSubAction::eInsertQuotation,
nsIEditor::eNext);
// give rules a chance to handle or cancel
EditSubActionInfo subActionInfo(EditSubAction::eInsertElement);
bool cancel, handled;
// Protect the edit rules object from dying
@@ -1973,17 +1975,17 @@ HTMLEditor::InsertAsCitedQuotation(const
}
// Set the selection inside the blockquote so aQuotedText will go there:
selection->Collapse(newNode, 0);
if (aInsertHTML) {
rv = LoadHTML(aQuotedText);
} else {
- rv = InsertTextAsAction(aQuotedText); // XXX ignore charset
+ rv = InsertTextAsSubAction(aQuotedText); // XXX ignore charset
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to insert the quoted text");
}
if (aNodeInserted && NS_SUCCEEDED(rv)) {
*aNodeInserted = newNode;
NS_IF_ADDREF(*aNodeInserted);
}
--- a/editor/libeditor/TextEditor.cpp
+++ b/editor/libeditor/TextEditor.cpp
@@ -416,17 +416,17 @@ TextEditor::HandleKeyPressEvent(WidgetKe
nsAutoString str(aKeyboardEvent->mCharCode);
return OnInputText(str);
}
nsresult
TextEditor::OnInputText(const nsAString& aStringToInsert)
{
AutoPlaceholderBatch batch(this, nsGkAtoms::TypingTxnName);
- nsresult rv = InsertTextAsAction(aStringToInsert);
+ nsresult rv = InsertTextAsSubAction(aStringToInsert);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
return NS_OK;
}
nsresult
TextEditor::OnInputParagraphSeparator()
@@ -956,31 +956,47 @@ TextEditor::InsertText(const nsAString&
return rv;
}
return NS_OK;
}
nsresult
TextEditor::InsertTextAsAction(const nsAString& aStringToInsert)
{
+ // Showing this assertion is fine if this method is called by outside via
+ // mutation event listener or something. Otherwise, this is called by
+ // wrong method.
+ NS_ASSERTION(!mPlaceholderBatch,
+ "Should be called only when this is the only edit action of the operation "
+ "unless mutation event listener nests some operations");
+
+ AutoPlaceholderBatch batch(this, nullptr);
+ nsresult rv = InsertTextAsSubAction(aStringToInsert);
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ return rv;
+ }
+ return NS_OK;
+}
+
+nsresult
+TextEditor::InsertTextAsSubAction(const nsAString& aStringToInsert)
+{
+ MOZ_ASSERT(mPlaceholderBatch);
+
if (!mRules) {
return NS_ERROR_NOT_INITIALIZED;
}
- // Protect the edit rules object from dying
+ // Protect the edit rules object from dying.
RefPtr<TextEditRules> rules(mRules);
- EditSubAction editSubAction = EditSubAction::eInsertText;
- if (ShouldHandleIMEComposition()) {
- // So, the string must come from IME as new composition string or
- // commit string.
- editSubAction = EditSubAction::eInsertTextComingFromIME;
- }
+ EditSubAction editSubAction =
+ ShouldHandleIMEComposition() ?
+ EditSubAction::eInsertTextComingFromIME : EditSubAction::eInsertText;
- AutoPlaceholderBatch batch(this, nullptr);
AutoTopLevelEditSubActionNotifier maybeTopLevelEditSubAction(
*this, editSubAction, nsIEditor::eNext);
RefPtr<Selection> selection = GetSelection();
if (NS_WARN_IF(!selection)) {
return NS_ERROR_FAILURE;
}
@@ -1001,17 +1017,21 @@ TextEditor::InsertTextAsAction(const nsA
}
if (!cancel && !handled) {
// we rely on rules code for now - no default implementation
}
if (cancel) {
return NS_OK;
}
// post-process
- return rules->DidDoAction(selection, subActionInfo, NS_OK);
+ rv = rules->DidDoAction(selection, subActionInfo, NS_OK);
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ return rv;
+ }
+ return NS_OK;
}
NS_IMETHODIMP
TextEditor::InsertLineBreak()
{
return InsertParagraphSeparatorAsAction();
}
@@ -1162,17 +1182,17 @@ TextEditor::SetText(const nsAString& aSt
} else {
rv = EditorBase::SelectEntireDocument(selection);
}
if (NS_SUCCEEDED(rv)) {
if (aString.IsEmpty()) {
rv = DeleteSelectionAsSubAction(eNone, eStrip);
NS_WARNING_ASSERTION(NS_FAILED(rv), "Failed to remove all text");
} else {
- rv = InsertTextAsAction(aString);
+ rv = InsertTextAsSubAction(aString);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to insert the new text");
}
}
}
// post-process
return rules->DidDoAction(selection, subActionInfo, rv);
}
@@ -1251,17 +1271,17 @@ TextEditor::OnCompositionChange(WidgetCo
RefPtr<nsCaret> caretP = presShell->GetCaret();
nsresult rv;
{
AutoPlaceholderBatch batch(this, nsGkAtoms::IMETxnName);
MOZ_ASSERT(mIsInEditSubAction,
"AutoPlaceholderBatch should've notified the observes of before-edit");
- rv = InsertTextAsAction(aCompsitionChangeEvent.mData);
+ rv = InsertTextAsSubAction(aCompsitionChangeEvent.mData);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"Failed to insert new composition string");
if (caretP) {
caretP->SetSelection(selection);
}
}
@@ -1931,18 +1951,17 @@ TextEditor::InsertWithQuotationsAsSubAct
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
if (cancel) {
return NS_OK; // Rules canceled the operation.
}
MOZ_ASSERT(!handled, "WillDoAction() shouldn't handle in this case");
if (!handled) {
- // TODO: Use InsertTextAsSubAction() when bug 1467796 is fixed.
- rv = InsertTextAsAction(quotedStuff);
+ rv = InsertTextAsSubAction(quotedStuff);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
}
// XXX Why don't we call TextEditRules::DidDoAction()?
return NS_OK;
}
--- a/editor/libeditor/TextEditor.h
+++ b/editor/libeditor/TextEditor.h
@@ -110,17 +110,18 @@ public:
virtual nsresult HandleKeyPressEvent(
WidgetKeyboardEvent* aKeyboardEvent) override;
virtual dom::EventTarget* GetDOMEventTarget() override;
/**
* InsertTextAsAction() inserts aStringToInsert at selection.
* Although this method is implementation of nsIPlaintextEditor.insertText(),
- * this treats the input is an edit action.
+ * this treats the input is an edit action. If you'd like to insert text
+ * as part of edit action, you probably should use InsertTextAsSubAction().
*
* @param aStringToInsert The string to insert.
*/
nsresult InsertTextAsAction(const nsAString& aStringToInsert);
/**
* PasteAsQuotationAsAction() pastes content in clipboard as quotation.
* If the editor is TextEditor or in plaintext mode, will paste the content
@@ -226,16 +227,24 @@ protected: // May be called by friends.
virtual nsresult SetAttributeOrEquivalent(Element* aElement,
nsAtom* aAttribute,
const nsAString& aValue,
bool aSuppressTransaction) override;
using EditorBase::RemoveAttributeOrEquivalent;
using EditorBase::SetAttributeOrEquivalent;
/**
+ * InsertTextAsSubAction() inserts aStringToInsert at selection. This
+ * should be used for handling it as an edit sub-action.
+ *
+ * @param aStringToInsert The string to insert.
+ */
+ nsresult InsertTextAsSubAction(const nsAString& aStringToInsert);
+
+ /**
* DeleteSelectionAsSubAction() removes selection content or content around
* caret with transactions. This should be used for handling it as an
* edit sub-action.
*
* @param aDirection How much range should be removed.
* @param aStripWrappers Whether the parent blocks should be removed
* when they become empty.
*/
--- a/editor/libeditor/TextEditorDataTransfer.cpp
+++ b/editor/libeditor/TextEditorDataTransfer.cpp
@@ -90,17 +90,17 @@ TextEditor::InsertTextAt(const nsAString
ErrorResult error;
selection->Collapse(RawRangeBoundary(targetNode, targetOffset), error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
}
}
- nsresult rv = InsertTextAsAction(aStringToInsert);
+ nsresult rv = InsertTextAsSubAction(aStringToInsert);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
return NS_OK;
}
nsresult
TextEditor::InsertTextFromTransferable(nsITransferable* aTransferable)
--- a/extensions/spellcheck/tests/mochitest/test_bug1170484.html
+++ b/extensions/spellcheck/tests/mochitest/test_bug1170484.html
@@ -8,16 +8,21 @@ https://bugzilla.mozilla.org/show_bug.cg
<title>Test for Bug 1170484</title>
<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
<script type="application/javascript">
/** Test for Bug 1170484 **/
SimpleTest.waitForExplicitFinish();
+
+ // An assertion is recorded due to nested replacing words from spellchecker but not a problem.
+ // It is necessary to detect invalid method call without event loop.
+ SimpleTest.expectAssertions(1, 1);
+
SpecialPowers.Cu.import(
"resource://testing-common/AsyncSpellCheckTestHelper.jsm", window);
SimpleTest.waitForFocus(doTest, window);
function doTest() {
var mutations = 0;
var observer = new MutationObserver(() => { mutations++; });
observer.observe($('area'), { childList: true, characterData: true, characterDataOldValue: true, subtree: true });
--- a/extensions/spellcheck/tests/mochitest/test_bug1272623.html
+++ b/extensions/spellcheck/tests/mochitest/test_bug1272623.html
@@ -21,16 +21,20 @@ https://bugzilla.mozilla.org/show_bug.cg
<pre id="test">
<div id="area" contenteditable="true"><b style="font-weight:normal;">testing <span id="misspelled">spellechek</span></b></div>
<div id="area2" contenteditable="true">testing <span id="misspelled2" style="font-weight:bold;">spellechek</span></div>
</pre>
<script>
/** Test for Bug 1272623 **/
+ // 2 assertions are recorded due to nested replacing words from spellchecker but not a problem.
+ // They are necessary to detect invalid method call without event loop.
+ SimpleTest.expectAssertions(2, 2);
+
async function performCorrection(misspelled, area) {
synthesizeMouseAtCenter($(misspelled), {}, window);
await new Promise(resolve => onSpellCheck($(area), resolve));
synthesizeMouseAtCenter($(misspelled), {type: 'contextmenu'}, window);
// Perform the spelling correction
let mm = SpecialPowers.loadChromeScript(function() {
ChromeUtils.import("resource://gre/modules/Services.jsm");
--- a/toolkit/content/tests/chrome/test_autocomplete4.xul
+++ b/toolkit/content/tests/chrome/test_autocomplete4.xul
@@ -20,16 +20,18 @@
<script class="testbody" type="application/javascript">
<![CDATA[
ChromeUtils.import("resource://gre/modules/Services.jsm");
// Set to indicate whether or not we want autoCompleteSimple to return a result
var returnResult = true;
+const IS_MAC = navigator.platform.includes("Mac");
+
const ACR = Ci.nsIAutoCompleteResult;
// This result can't be constructed in-line, because otherwise we leak memory.
function nsAutoCompleteSimpleResult(aString)
{
this.searchString = aString;
if (returnResult) {
this.searchResult = ACR.RESULT_SUCCESS;
@@ -83,16 +85,24 @@ var componentManager = Components.manage
.QueryInterface(Ci.nsIComponentRegistrar);
componentManager.registerFactory(autoCompleteSimpleID, "Test Simple Autocomplete",
autoCompleteSimpleName, autoCompleteSimple);
// Test Bug 325842 - completeDefaultIndex
SimpleTest.waitForExplicitFinish();
+
+// 8 or 6 assertions are recorded due to nested setting <input>.value but not a problem.
+// They are necessary to detect invalid method call without event loop.
+if (IS_MAC)
+ SimpleTest.expectAssertions(6, 6);
+else
+ SimpleTest.expectAssertions(8, 8);
+
setTimeout(nextTest, 0);
var currentTest = null;
// Note the entries for these tests (key) are incremental.
const tests = [
{
desc: "HOME key remove selection",
@@ -182,17 +192,17 @@ function nextTest() {
return;
}
var autocomplete = $("autocomplete");
autocomplete.value = "";
currentTest = tests.shift();
// HOME key works differently on Mac, so we skip tests using it.
- if (currentTest.key == "KEY_Home" && navigator.platform.includes("Mac"))
+ if (currentTest.key == "KEY_Home" && IS_MAC)
nextTest();
else
setTimeout(runCurrentTest, 0);
}
function runCurrentTest() {
var autocomplete = $("autocomplete");
if ("minResultsForPopup" in currentTest)
--- a/toolkit/content/tests/chrome/test_autocomplete_placehold_last_complete.xul
+++ b/toolkit/content/tests/chrome/test_autocomplete_placehold_last_complete.xul
@@ -102,16 +102,20 @@ let autoCompleteSimple = {
},
stopSearch: function () {
clearTimeout(this.pendingSearch);
}
};
SimpleTest.waitForExplicitFinish();
+// 6 assertions are recorded due to nested setting <input>.value but not a problem.
+// They are necessary to detect invalid method call without mutation event listers.
+SimpleTest.expectAssertions(6, 6);
+
let gACTimer;
let gAutoComplete;
let asyncTest;
let searchCompleteTimeoutId = null;
function finishTest() {
// Unregister the factory so that we don't get in the way of other tests