Bug 1572375 - part 2: Split `TextEditRules::WillInsert()` r=m_kato

`TextEditRules::WillInsert()` is not used with initial purpose since
`HTMLEditor` always works with `HTMLEditRules` and its `WillDoAction()`
always handles `EditSubAction::eInsertElement`.

Additionally, its name is too generic since it does non-related 3 things.

One is checking whether the editor is readonly or disabled.  However, this
may not be necessary since its callers may have already checked it or
just ignored the result.  So, this should be check by each caller.

Next one is masking password if auto-masking is enabled.  This is `TextEditor`
specific feature so that this patch moves the code into
`TextEditor::MaybeDoAutoPasswordMasking()`.

Final one is removing empty `<br>` element for empty editor if there is.
This is common feature so that this patch moves this code into
`EditorBase::EnsureNoPaddingBRElementForEmptyEditor()`.

Differential Revision: https://phabricator.services.mozilla.com/D41156

--HG--
extra : moz-landing-system : lando

editor/libeditor/EditAction.h

@@ -449,6 +449,9 @@ enum class EditSubAction : int32_t {
   // eInsertQuotation indicates to insert an element and make it "quoted text".
   eInsertQuotation,
 
+  // eInsertQuotedText indicates to insert text which has already been quoted.
+  eInsertQuotedText,
+
   // ePasteHTMLContent indicates to paste HTML content in clipboard.
   ePasteHTMLContent,
 

editor/libeditor/EditorBase.cpp

@@ -4033,6 +4033,25 @@ EditorDOMPoint EditorBase::JoinNodesDeepWithTransaction(
   return ret;
 }
 
+nsresult EditorBase::EnsureNoPaddingBRElementForEmptyEditor() {
+  MOZ_ASSERT(IsEditActionDataAvailable());
+
+  if (!mPaddingBRElementForEmptyEditor) {
+    return NS_OK;
+  }
+
+  // If we're an HTML editor, a mutation event listener may recreate padding
+  // <br> element for empty editor again during the call of
+  // DeleteNodeWithTransaction().  So, move it first.
+  RefPtr<HTMLBRElement> paddingBRElement(
+      std::move(mPaddingBRElementForEmptyEditor));
+  nsresult rv = DeleteNodeWithTransaction(*paddingBRElement);
+  if (NS_WARN_IF(Destroyed())) {
+    return NS_ERROR_EDITOR_DESTROYED;
+  }
+  return rv;
+}
+
 void EditorBase::BeginUpdateViewBatch() {
   MOZ_ASSERT(IsEditActionDataAvailable());
   MOZ_ASSERT(mUpdateCount >= 0, "bad state");

editor/libeditor/EditorBase.h

@@ -693,6 +693,7 @@ class EditorBase : public nsIEditor,
         case EditSubAction::eCreateOrChangeDefinitionList:
         case EditSubAction::eInsertElement:
         case EditSubAction::eInsertQuotation:
+        case EditSubAction::eInsertQuotedText:
         case EditSubAction::ePasteHTMLContent:
         case EditSubAction::eInsertHTMLSource:
         case EditSubAction::eSetPositionToAbsolute:
@@ -1466,6 +1467,13 @@ class EditorBase : public nsIEditor,
   EditorDOMPoint JoinNodesDeepWithTransaction(nsIContent& aLeftNode,
                                               nsIContent& aRightNode);
 
+  /**
+   * EnsureNoPaddingBRElementForEmptyEditor() removes padding <br> element
+   * for empty editor if there is.
+   */
+  MOZ_CAN_RUN_SCRIPT MOZ_MUST_USE nsresult
+  EnsureNoPaddingBRElementForEmptyEditor();
+
   MOZ_CAN_RUN_SCRIPT nsresult DoTransactionInternal(nsITransaction* aTxn);
 
   virtual bool IsBlockNode(nsINode* aNode) const;

editor/libeditor/HTMLEditRules.cpp

@@ -77,18 +77,24 @@ enum { kLonely = 0, kPrevSib = 1, kNextSib = 2, kBothSibs = 3 };
  ********************************************************/
 
 static bool IsStyleCachePreservingSubAction(EditSubAction aEditSubAction) {
-  return aEditSubAction == EditSubAction::eDeleteSelectedContent ||
-         aEditSubAction == EditSubAction::eInsertLineBreak ||
-         aEditSubAction == EditSubAction::eInsertParagraphSeparator ||
-         aEditSubAction == EditSubAction::eCreateOrChangeList ||
-         aEditSubAction == EditSubAction::eIndent ||
-         aEditSubAction == EditSubAction::eOutdent ||
-         aEditSubAction == EditSubAction::eSetOrClearAlignment ||
-         aEditSubAction == EditSubAction::eCreateOrRemoveBlock ||
-         aEditSubAction == EditSubAction::eRemoveList ||
-         aEditSubAction == EditSubAction::eCreateOrChangeDefinitionList ||
-         aEditSubAction == EditSubAction::eInsertElement ||
-         aEditSubAction == EditSubAction::eInsertQuotation;
+  switch (aEditSubAction) {
+    case EditSubAction::eDeleteSelectedContent:
+    case EditSubAction::eInsertLineBreak:
+    case EditSubAction::eInsertParagraphSeparator:
+    case EditSubAction::eCreateOrChangeList:
+    case EditSubAction::eIndent:
+    case EditSubAction::eOutdent:
+    case EditSubAction::eSetOrClearAlignment:
+    case EditSubAction::eCreateOrRemoveBlock:
+    case EditSubAction::eRemoveList:
+    case EditSubAction::eCreateOrChangeDefinitionList:
+    case EditSubAction::eInsertElement:
+    case EditSubAction::eInsertQuotation:
+    case EditSubAction::eInsertQuotedText:
+      return true;
+    default:
+      return false;
+  }
 }
 
 static nsAtom& ParagraphSeparatorElement(ParagraphSeparator separator) {
@@ -755,7 +761,8 @@ nsresult HTMLEditRules::WillDoAction(EditSubActionInfo& aInfo, bool* aCancel,
     case EditSubAction::eCreateOrChangeDefinitionList:
       return WillMakeDefListItem(aInfo.blockType, aInfo.entireList, aCancel,
                                  aHandled);
-    case EditSubAction::eInsertElement: {
+    case EditSubAction::eInsertElement:
+    case EditSubAction::eInsertQuotedText: {
       nsresult rv = WillInsert(aCancel);
       if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
         return NS_ERROR_EDITOR_DESTROYED;
@@ -1235,7 +1242,13 @@ nsresult HTMLEditRules::GetFormatString(nsINode* aNode, nsAString& outFormat) {
 nsresult HTMLEditRules::WillInsert(bool* aCancel) {
   MOZ_ASSERT(IsEditorDataAvailable());
 
-  nsresult rv = TextEditRules::WillInsert(aCancel);
+  // XXX Why don't we stop handling this call if we're readonly or disabled?
+  if (aCancel && (IsReadonly() || IsDisabled())) {
+    *aCancel = true;
+  }
+
+  nsresult rv =
+      MOZ_KnownLive(HTMLEditorRef()).EnsureNoPaddingBRElementForEmptyEditor();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }

editor/libeditor/TextEditRules.cpp

@@ -329,10 +329,22 @@ nsresult TextEditRules::WillDoAction(EditSubActionInfo& aInfo, bool* aCancel,
     case EditSubAction::eComputeTextToOutput:
       return WillOutputText(aInfo.outputFormat, aInfo.outString, aInfo.flags,
                             aCancel, aHandled);
+    case EditSubAction::eInsertQuotedText: {
+      CANCEL_OPERATION_IF_READONLY_OR_DISABLED
+
+      // XXX Do we need to support paste-as-quotation in password editor (and
+      //     also in single line editor)?
+      TextEditorRef().MaybeDoAutoPasswordMasking();
+
+      nsresult rv = MOZ_KnownLive(TextEditorRef())
+                        .EnsureNoPaddingBRElementForEmptyEditor();
+      NS_WARNING_ASSERTION(NS_FAILED(rv),
+                           "Failed to remove padding <br> element");
+      return rv;
+    }
     case EditSubAction::eInsertElement:
-      // i had thought this would be html rules only.  but we put pre elements
-      // into plaintext mail when doing quoting for reply!  doh!
-      return WillInsert(aCancel);
+      MOZ_ASSERT_UNREACHABLE("This path should've been dead code");
+      return NS_ERROR_UNEXPECTED;
     default:
       return NS_ERROR_FAILURE;
   }
@@ -372,45 +384,6 @@ bool TextEditRules::DocumentIsEmpty() const {
   return retVal;
 }
 
-nsresult TextEditRules::WillInsert(bool* aCancel) {
-  MOZ_ASSERT(IsEditorDataAvailable());
-
-  if (IsReadonly() || IsDisabled()) {
-    if (aCancel) {
-      *aCancel = true;
-    }
-    return NS_OK;
-  }
-
-  // initialize out param
-  if (aCancel) {
-    *aCancel = false;
-  }
-
-  if (IsPasswordEditor() && IsMaskingPassword()) {
-    TextEditorRef().MaskAllCharacters();
-  }
-
-  // check for the magic content node and delete it if it exists
-  if (!TextEditorRef().mPaddingBRElementForEmptyEditor) {
-    return NS_OK;
-  }
-
-  // A mutation event listener may recreate padding <br> element for empty
-  // editor again during the call of DeleteNodeWithTransaction().  So, move
-  // it first.
-  RefPtr<HTMLBRElement> paddingBRElement(
-      std::move(TextEditorRef().mPaddingBRElementForEmptyEditor));
-  DebugOnly<nsresult> rv = MOZ_KnownLive(TextEditorRef())
-                               .DeleteNodeWithTransaction(*paddingBRElement);
-  if (NS_WARN_IF(!CanHandleEditAction())) {
-    return NS_ERROR_EDITOR_DESTROYED;
-  }
-  NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
-                       "Failed to remove the padding <br> element");
-  return NS_OK;
-}
-
 EditActionResult TextEditRules::WillInsertLineBreak(int32_t aMaxLength) {
   MOZ_ASSERT(IsEditorDataAvailable());
   MOZ_ASSERT(!IsSingleLineEditor());
@@ -443,7 +416,7 @@ EditActionResult TextEditRules::WillInsertLineBreak(int32_t aMaxLength) {
     }
   }
 
-  rv = WillInsert();
+  rv = MOZ_KnownLive(TextEditorRef()).EnsureNoPaddingBRElementForEmptyEditor();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return EditActionIgnored(rv);
   }
@@ -745,7 +718,12 @@ nsresult TextEditRules::WillInsertText(EditSubAction aEditSubAction,
     }
   }
 
-  rv = WillInsert(aCancel);
+  // XXX Why do we set `aCancel` here, but ignore it?
+  CANCEL_OPERATION_IF_READONLY_OR_DISABLED
+
+  TextEditorRef().MaybeDoAutoPasswordMasking();
+
+  rv = MOZ_KnownLive(TextEditorRef()).EnsureNoPaddingBRElementForEmptyEditor();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
@@ -877,6 +855,7 @@ nsresult TextEditRules::WillSetText(bool* aCancel, bool* aHandled,
   MOZ_ASSERT(aString);
   MOZ_ASSERT(aString->FindChar(static_cast<char16_t>('\r')) == kNotFound);
 
+  // XXX If we're setting value, shouldn't we keep setting the new value here?
   CANCEL_OPERATION_IF_READONLY_OR_DISABLED
 
   *aHandled = false;
@@ -891,7 +870,10 @@ nsresult TextEditRules::WillSetText(bool* aCancel, bool* aHandled,
     return NS_OK;
   }
 
-  nsresult rv = WillInsert(aCancel);
+  TextEditorRef().MaybeDoAutoPasswordMasking();
+
+  nsresult rv =
+      MOZ_KnownLive(TextEditorRef()).EnsureNoPaddingBRElementForEmptyEditor();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
@@ -908,8 +890,8 @@ nsresult TextEditRules::WillSetText(bool* aCancel, bool* aHandled,
     // If we're a single line text editor, i.e., <input>, there is only padding
     // <br> element.  Otherwise, there should be only one text node.  But note
     // that even if there is a padding <br> element for empty editor, it's
-    // already been removed by WillInsert().  So, at here, there should be only
-    // one text node or no children.
+    // already been removed by `EnsureNoPaddingBRElementForEmptyEditor()`.  So,
+    // at here, there should be only one text node or no children.
     if (firstChild &&
         (!EditorBase::IsTextNode(firstChild) || firstChild->GetNextSibling())) {
       return NS_OK;

editor/libeditor/TextEditRules.h

@@ -184,16 +184,6 @@ class TextEditRules {
                                     const nsAString* inString,
                                     int32_t aMaxLength);
 
-  /**
-   * Called before inserting something into the editor.
-   * This method may removes mBougsNode if there is.  Therefore, this method
-   * might cause destroying the editor.
-   *
-   * @param aCancel             Returns true if the operation is canceled.
-   *                            This can be nullptr.
-   */
-  MOZ_CAN_RUN_SCRIPT MOZ_MUST_USE nsresult WillInsert(bool* aCancel = nullptr);
-
   /**
    * Called before deleting selected content.
    * This method may actually remove the selected content with

editor/libeditor/TextEditor.cpp

@@ -1980,10 +1980,7 @@ nsresult TextEditor::InsertWithQuotationsAsSubAction(
   AutoTopLevelEditSubActionNotifier maybeTopLevelEditSubAction(
       *this, EditSubAction::eInsertText, nsIEditor::eNext);
 
-  // XXX This WillDoAction() usage is hacky.  If it returns as handled,
-  //     this method cannot work as expected.  So, this should have specific
-  //     sub-action rather than using eInsertElement.
-  EditSubActionInfo subActionInfo(EditSubAction::eInsertElement);
+  EditSubActionInfo subActionInfo(EditSubAction::eInsertQuotedText);
   bool cancel, handled;
   rv = rules->WillDoAction(subActionInfo, &cancel, &handled);
   if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -1993,11 +1990,9 @@ nsresult TextEditor::InsertWithQuotationsAsSubAction(
     return NS_OK;  // Rules canceled the operation.
   }
   MOZ_ASSERT(!handled, "WillDoAction() shouldn't handle in this case");
-  if (!handled) {
-    rv = InsertTextAsSubAction(quotedStuff);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
+  rv = InsertTextAsSubAction(quotedStuff);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
   }
   // XXX Why don't we call TextEditRules::DidDoAction()?
   return NS_OK;

editor/libeditor/TextEditor.h

@@ -476,6 +476,15 @@ class TextEditor : public EditorBase,
   static void GetDefaultEditorPrefs(int32_t& aNewLineHandling,
                                     int32_t& aCaretStyle);
 
+  /**
+   * MaybeDoAutoPasswordMasking() may mask password if we're doing auto-masking.
+   */
+  void MaybeDoAutoPasswordMasking() {
+    if (IsPasswordEditor() && IsMaskingPassword()) {
+      MaskAllCharacters();
+    }
+  }
+
   /**
    * SetUnmaskRange() is available only when the instance is a password
    * editor.  This just updates unmask range.  I.e., caller needs to