Bug 1572375 - part 2: Split `TextEditRules::WillInsert()` r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 09 Aug 2019 08:25:37 +0000
changeset 487547 94b4e63c71df3c07e8a2ac8dab406284782b73e5
parent 487546 235c10b6af849e1658fefd1dfdc6e21a02c023b2
child 487548 e7d953d0be6cf385c848d80039a1cb737b47606b
push id36425
push userbtara@mozilla.com
push dateTue, 13 Aug 2019 09:54:32 +0000
treeherdermozilla-central@e29ba984dad2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1572375
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
editor/libeditor/EditAction.h
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/TextEditRules.cpp
editor/libeditor/TextEditRules.h
editor/libeditor/TextEditor.cpp
editor/libeditor/TextEditor.h
--- a/editor/libeditor/EditAction.h
+++ b/editor/libeditor/EditAction.h
@@ -444,16 +444,19 @@ enum class EditSubAction : int32_t {
   eCreateOrChangeDefinitionList,
 
   // eInsertElement indicates to insert an element.
   eInsertElement,
 
   // 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,
 
   // eInsertHTMLSource indicates to create a document fragment from given HTML
   // source and insert into the DOM tree.  So, this is similar to innerHTML.
   eInsertHTMLSource,
 
   // eReplaceHeadWithHTMLSource indicates to create a document fragment from
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -4028,16 +4028,35 @@ EditorDOMPoint EditorBase::JoinNodesDeep
 
   if (NS_WARN_IF(!ret.IsSet())) {
     return EditorDOMPoint();
   }
 
   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");
 
   if (!mUpdateCount) {
     // Turn off selection updates and notifications.
     SelectionRefPtr()->StartBatchChanges();
   }
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -688,16 +688,17 @@ class EditorBase : public nsIEditor,
         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:
         case EditSubAction::ePasteHTMLContent:
         case EditSubAction::eInsertHTMLSource:
         case EditSubAction::eSetPositionToAbsolute:
         case EditSubAction::eSetPositionToStatic:
         case EditSubAction::eDecreaseZIndex:
         case EditSubAction::eIncreaseZIndex:
           MOZ_ASSERT(aDirection == eNext);
           mDirectionOfTopLevelEditSubAction = eNext;
@@ -1461,16 +1462,23 @@ class EditorBase : public nsIEditor,
    * @param aRightNode  The node which will be inserted the contents of
    *                    aLeftNode.
    * @return            The point of the first child of the last right node.
    */
   MOZ_CAN_RUN_SCRIPT
   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;
 
   /**
    * Set outOffset to the offset of aChild in the parent.
    * Returns the parent of aChild.
    */
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -72,28 +72,34 @@ using namespace dom;
 
 enum { kLonely = 0, kPrevSib = 1, kNextSib = 2, kBothSibs = 3 };
 
 /********************************************************
  *  first some helpful functors we will use
  ********************************************************/
 
 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) {
   switch (separator) {
     default:
       MOZ_FALLTHROUGH_ASSERT("Unexpected paragraph separator!");
 
     case ParagraphSeparator::div:
@@ -750,17 +756,18 @@ nsresult HTMLEditRules::WillDoAction(Edi
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       return NS_OK;
     }
     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;
       }
       NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "WillInsert() failed");
       return NS_OK;
     }
     case EditSubAction::eDecreaseZIndex:
@@ -1230,17 +1237,23 @@ nsresult HTMLEditRules::GetFormatString(
     outFormat.Truncate();
   }
   return NS_OK;
 }
 
 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;
   }
 
   // Adjust selection to prevent insertion after a padding <br> element for
   // empty last line.  This next only works for collapsed selections right
   // now, because selection is a pain to work with when not collapsed.  (no
   // good way to extend start or end of selection), so we ignore those types
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -324,20 +324,32 @@ nsresult TextEditRules::WillDoAction(Edi
       return WillRedo(aCancel, aHandled);
     case EditSubAction::eSetTextProperty:
       return WillSetTextProperty(aCancel, aHandled);
     case EditSubAction::eRemoveTextProperty:
       return WillRemoveTextProperty(aCancel, aHandled);
     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;
   }
 }
 
 nsresult TextEditRules::DidDoAction(EditSubActionInfo& aInfo,
                                     nsresult aResult) {
   if (NS_WARN_IF(!CanHandleEditAction())) {
@@ -367,55 +379,16 @@ bool TextEditRules::DocumentIsEmpty() co
   bool retVal = false;
   if (!mTextEditor || NS_FAILED(mTextEditor->IsEmpty(&retVal))) {
     retVal = true;
   }
 
   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());
 
   CANCEL_OPERATION_AND_RETURN_EDIT_ACTION_RESULT_IF_READONLY_OF_DISABLED
 
   // handle docs with a max length
   // NOTE, this function copies inString into outString for us.
@@ -438,17 +411,17 @@ EditActionResult TextEditRules::WillInse
     if (NS_WARN_IF(!CanHandleEditAction())) {
       return EditActionIgnored(NS_ERROR_EDITOR_DESTROYED);
     }
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return EditActionIgnored(rv);
     }
   }
 
-  rv = WillInsert();
+  rv = MOZ_KnownLive(TextEditorRef()).EnsureNoPaddingBRElementForEmptyEditor();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return EditActionIgnored(rv);
   }
 
   // get the (collapsed) selection location
   nsRange* firstRange = SelectionRefPtr()->GetRangeAt(0);
   if (NS_WARN_IF(!firstRange)) {
     return EditActionIgnored(NS_ERROR_FAILURE);
@@ -740,17 +713,22 @@ nsresult TextEditRules::WillInsertText(E
     if (NS_WARN_IF(!CanHandleEditAction())) {
       return NS_ERROR_EDITOR_DESTROYED;
     }
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
-  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;
   }
 
   // People have lots of different ideas about what text fields
   // should do with multiline pastes.  See bugs 21032, 23485, 23485, 50935.
   // The six possible options are:
   // 0. paste newlines intact
@@ -872,49 +850,53 @@ nsresult TextEditRules::WillSetText(bool
                                     const nsAString* aString,
                                     int32_t aMaxLength) {
   MOZ_ASSERT(IsEditorDataAvailable());
   MOZ_ASSERT(aCancel);
   MOZ_ASSERT(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;
   *aCancel = false;
 
   if (!IsPlaintextEditor() || TextEditorRef().IsIMEComposing() ||
       TextEditorRef().IsUndoRedoEnabled() ||
       TextEditorRef().GetEditAction() == EditAction::eReplaceText ||
       aMaxLength != -1) {
     // SetTextImpl only supports plain text editor without IME and
     // when we don't need to make it undoable.
     return NS_OK;
   }
 
-  nsresult rv = WillInsert(aCancel);
+  TextEditorRef().MaybeDoAutoPasswordMasking();
+
+  nsresult rv =
+      MOZ_KnownLive(TextEditorRef()).EnsureNoPaddingBRElementForEmptyEditor();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   RefPtr<Element> rootElement = TextEditorRef().GetRoot();
   nsIContent* firstChild = rootElement->GetFirstChild();
 
   // We can use this fast path only when:
   //  - we need to insert a text node.
   //  - we need to replace content of existing text node.
   // Additionally, for avoiding odd result, we should check whether we're in
   // usual condition.
   if (IsSingleLineEditor()) {
     // 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;
     }
   } else {
     // If we're a multiline text editor, i.e., <textarea>, there is a padding
     // <br> element for empty last line followed by scrollbar/resizer elements.
     // Otherwise, a text node is followed by them.
--- a/editor/libeditor/TextEditRules.h
+++ b/editor/libeditor/TextEditRules.h
@@ -180,26 +180,16 @@ class TextEditRules {
    *                            allows to set.
    */
   MOZ_CAN_RUN_SCRIPT
   MOZ_MUST_USE nsresult WillSetText(bool* aCancel, bool* aHandled,
                                     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
    * DeleteSelectionWithTransaction().  So, this might cause destroying the
    * editor.
    *
    * @param aCaollapsedAction   Direction to extend the selection.
    * @param aCancel             Returns true if the operation is canceled.
    * @param aHandled            Returns true if the edit action is handled.
--- a/editor/libeditor/TextEditor.cpp
+++ b/editor/libeditor/TextEditor.cpp
@@ -1975,34 +1975,29 @@ nsresult TextEditor::InsertWithQuotation
   // written without thinking won't be so ugly.
   if (!aQuotedText.IsEmpty() && (aQuotedText.Last() != char16_t('\n'))) {
     quotedStuff.Append(char16_t('\n'));
   }
 
   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))) {
     return rv;
   }
   if (cancel) {
     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;
 }
 
 nsresult TextEditor::SharedOutputString(uint32_t aFlags, bool* aIsCollapsed,
                                         nsAString& aResult) {
   MOZ_ASSERT(IsEditActionDataAvailable());
--- a/editor/libeditor/TextEditor.h
+++ b/editor/libeditor/TextEditor.h
@@ -472,16 +472,25 @@ class TextEditor : public EditorBase,
    * extension.
    */
   nsresult ExtendSelectionForDelete(nsIEditor::EDirection* aAction);
 
   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
    * guarantee to update the layout.
    *
    * @param aStart      First index to show the character.
    *                    If aLength is 0, this value is ignored.
    * @param aLength     Optional, Length to show characters.
    *                    If UINT32_MAX, it means unmasking all characters after