Bug 1467693 - Merge EditorBase::SwitchTextDirection() and EditorBase::SwitchTextDirectionTo() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 07 Jun 2018 23:26:59 +0900
changeset 422025 7fb30739a8a01e09e5ba632932acfda269baad1e
parent 422024 f077e2d6e30ead21169af463ba77382ec8f58c46
child 422091 b3cc87c3a7e82f3de0171b8618022dd43fe321aa
push id65083
push usermasayuki@d-toybox.com
push dateSat, 09 Jun 2018 12:59:03 +0000
treeherderautoland@7fb30739a8a0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1467693
milestone62.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 1467693 - Merge EditorBase::SwitchTextDirection() and EditorBase::SwitchTextDirectionTo() r=m_kato There are two similar methods, but they are created with copy & paste. So, the common code should be merged into new method, SetTextDirection(). Additionally, EditorBase::SwitchTextDirection() is a scriptable method but nobody uses this from JS. So, we can make it from nsIEditor and this patch renames it to ToggleTextDirection() since current name is too similar to SwitchTextDirectionTo(). MozReview-Commit-ID: BX3M1OKxiD5
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/EditorCommands.cpp
editor/libeditor/EditorEventListener.cpp
editor/nsIEditor.idl
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -4814,76 +4814,102 @@ EditorBase::DetermineCurrentDirection()
     } else {
       mFlags |= nsIPlaintextEditor::eEditorLeftToRight;
     }
   }
 
   return NS_OK;
 }
 
-NS_IMETHODIMP
-EditorBase::SwitchTextDirection()
+nsresult
+EditorBase::ToggleTextDirection()
 {
-  // Get the current root direction from its frame
-  Element* rootElement = GetExposedRoot();
+  // XXX Oddly, Chrome does not dispatch beforeinput event in this case but
+  //     dispatches input event.
 
   nsresult rv = DetermineCurrentDirection();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // Apply the opposite direction
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
   if (IsRightToLeft()) {
-    NS_ASSERTION(!IsLeftToRight(),
-                 "Unexpected mutually exclusive flag");
-    mFlags &= ~nsIPlaintextEditor::eEditorRightToLeft;
-    mFlags |= nsIPlaintextEditor::eEditorLeftToRight;
-    rv = rootElement->SetAttr(kNameSpaceID_None, nsGkAtoms::dir, NS_LITERAL_STRING("ltr"), true);
+    nsresult rv = SetTextDirectionTo(TextDirection::eLTR);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   } else if (IsLeftToRight()) {
-    NS_ASSERTION(!IsRightToLeft(),
-                 "Unexpected mutually exclusive flag");
-    mFlags |= nsIPlaintextEditor::eEditorRightToLeft;
-    mFlags &= ~nsIPlaintextEditor::eEditorLeftToRight;
-    rv = rootElement->SetAttr(kNameSpaceID_None, nsGkAtoms::dir, NS_LITERAL_STRING("rtl"), true);
-  }
-
-  if (NS_SUCCEEDED(rv)) {
-    FireInputEvent();
-  }
-
-  return rv;
+    nsresult rv = SetTextDirectionTo(TextDirection::eRTL);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+  }
+
+  // XXX When we don't change the text direction, do we really need to
+  //     dispatch input event?
+  FireInputEvent();
+
+  return NS_OK;
 }
 
 void
-EditorBase::SwitchTextDirectionTo(uint32_t aDirection)
+EditorBase::SwitchTextDirectionTo(TextDirection aTextDirection)
 {
-  // Get the current root direction from its frame
-  Element* rootElement = GetExposedRoot();
+  // XXX Oddly, Chrome does not dispatch beforeinput event in this case but
+  //     dispatches input event.
 
   nsresult rv = DetermineCurrentDirection();
-  NS_ENSURE_SUCCESS_VOID(rv);
-
-  // Apply the requested direction
-  if (aDirection == nsIPlaintextEditor::eEditorLeftToRight &&
-      IsRightToLeft()) {
-    NS_ASSERTION(!(mFlags & nsIPlaintextEditor::eEditorLeftToRight),
-                 "Unexpected mutually exclusive flag");
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+
+  if (aTextDirection == TextDirection::eLTR && IsRightToLeft()) {
+    if (NS_WARN_IF(NS_FAILED(SetTextDirectionTo(aTextDirection)))) {
+      return;
+    }
+  } else if (aTextDirection == TextDirection::eRTL && IsLeftToRight()) {
+    if (NS_WARN_IF(NS_FAILED(SetTextDirectionTo(aTextDirection)))) {
+      return;
+    }
+  }
+
+  // XXX When we don't change the text direction, do we really need to
+  //     dispatch input event?
+  FireInputEvent();
+}
+
+nsresult
+EditorBase::SetTextDirectionTo(TextDirection aTextDirection)
+{
+  Element* rootElement = GetExposedRoot();
+
+  if (aTextDirection == TextDirection::eLTR) {
+    NS_ASSERTION(!IsLeftToRight(), "Unexpected mutually exclusive flag");
     mFlags &= ~nsIPlaintextEditor::eEditorRightToLeft;
     mFlags |= nsIPlaintextEditor::eEditorLeftToRight;
-    rv = rootElement->SetAttr(kNameSpaceID_None, nsGkAtoms::dir, NS_LITERAL_STRING("ltr"), true);
-  } else if (aDirection == nsIPlaintextEditor::eEditorRightToLeft &&
-             IsLeftToRight()) {
-    NS_ASSERTION(!(mFlags & nsIPlaintextEditor::eEditorRightToLeft),
-                 "Unexpected mutually exclusive flag");
+    nsresult rv = rootElement->SetAttr(kNameSpaceID_None, nsGkAtoms::dir,
+                                       NS_LITERAL_STRING("ltr"), true);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+    return NS_OK;
+  }
+
+  if (aTextDirection == TextDirection::eRTL) {
+    NS_ASSERTION(!IsRightToLeft(), "Unexpected mutually exclusive flag");
     mFlags |= nsIPlaintextEditor::eEditorRightToLeft;
     mFlags &= ~nsIPlaintextEditor::eEditorLeftToRight;
-    rv = rootElement->SetAttr(kNameSpaceID_None, nsGkAtoms::dir, NS_LITERAL_STRING("rtl"), true);
-  }
-
-  if (NS_SUCCEEDED(rv)) {
-    FireInputEvent();
-  }
+    nsresult rv = rootElement->SetAttr(kNameSpaceID_None, nsGkAtoms::dir,
+                                       NS_LITERAL_STRING("rtl"), true);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+    return NS_OK;
+  }
+
+  return NS_OK;
 }
 
 bool
 EditorBase::IsModifiableNode(nsINode* aNode)
 {
   return true;
 }
 
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -343,17 +343,31 @@ public:
    * Commit composition if there is.
    * Note that when there is a composition, this requests to commit composition
    * to native IME.  Therefore, when there is composition, this can do anything.
    * For example, the editor instance, the widget or the process itself may
    * be destroyed.
    */
   nsresult CommitComposition();
 
-  void SwitchTextDirectionTo(uint32_t aDirection);
+  /**
+   * ToggleTextDirection() toggles text-direction of the root element.
+   */
+  nsresult ToggleTextDirection();
+
+  /**
+   * SwitchTextDirectionTo() sets the text-direction of the root element to
+   * LTR or RTL.
+   */
+  enum class TextDirection
+  {
+    eLTR,
+    eRTL,
+  };
+  void SwitchTextDirectionTo(TextDirection aTextDirection);
 
   /**
    * Finalizes selection and caret for the editor.
    */
   nsresult FinalizeSelection();
 
   /**
    * Returns true if selection is in an editable element and both the range
@@ -1819,16 +1833,23 @@ protected: // Shouldn't be used by frien
     eNotifyEditorObserversOfCancel
   };
   void NotifyEditorObservers(NotificationForEditorObservers aNotification);
 
 private:
   nsCOMPtr<nsISelectionController> mSelectionController;
   nsCOMPtr<nsIDocument> mDocument;
 
+
+  /**
+   * SetTextDirectionTo() sets text-direction of the root element.
+   * Should use SwitchTextDirectionTo() or ToggleTextDirection() instead.
+   * This is a helper class of them.
+   */
+  nsresult SetTextDirectionTo(TextDirection aTextDirection);
 protected:
   enum Tristate
   {
     eTriUnset,
     eTriFalse,
     eTriTrue
   };
 
--- a/editor/libeditor/EditorCommands.cpp
+++ b/editor/libeditor/EditorCommands.cpp
@@ -700,17 +700,17 @@ SwitchTextDirectionCommand::DoCommand(co
                                       nsISupports* aCommandRefCon)
 {
   nsCOMPtr<nsIEditor> editor = do_QueryInterface(aCommandRefCon);
   if (NS_WARN_IF(!editor)) {
     return NS_ERROR_FAILURE;
   }
   TextEditor* textEditor = editor->AsTextEditor();
   MOZ_ASSERT(textEditor);
-  return textEditor->SwitchTextDirection();
+  return textEditor->ToggleTextDirection();
 }
 
 NS_IMETHODIMP
 SwitchTextDirectionCommand::DoCommandParams(const char* aCommandName,
                                             nsICommandParams* aParams,
                                             nsISupports* aCommandRefCon)
 {
   return DoCommand(aCommandName, aCommandRefCon);
--- a/editor/libeditor/EditorEventListener.cpp
+++ b/editor/libeditor/EditorEventListener.cpp
@@ -542,19 +542,19 @@ EditorEventListener::KeyUp(const WidgetK
     return NS_OK;
   }
 
   // XXX Why doesn't this method check if it's consumed?
   RefPtr<EditorBase> editorBase(mEditorBase);
   if ((aKeyboardEvent->mKeyCode == NS_VK_SHIFT ||
        aKeyboardEvent->mKeyCode == NS_VK_CONTROL) &&
       mShouldSwitchTextDirection && editorBase->IsPlaintextEditor()) {
-    editorBase->SwitchTextDirectionTo(mSwitchToRTL ?
-      nsIPlaintextEditor::eEditorRightToLeft :
-      nsIPlaintextEditor::eEditorLeftToRight);
+    editorBase->SwitchTextDirectionTo(
+                  mSwitchToRTL ? EditorBase::TextDirection::eRTL :
+                                 EditorBase::TextDirection::eLTR);
     mShouldSwitchTextDirection = false;
   }
   return NS_OK;
 }
 
 nsresult
 EditorEventListener::KeyDown(const WidgetKeyboardEvent* aKeyboardEvent)
 {
--- a/editor/nsIEditor.idl
+++ b/editor/nsIEditor.idl
@@ -427,24 +427,16 @@ interface nsIEditor  : nsISupports
 
   /**
    * markNodeDirty() sets a special dirty attribute on the node.
    * Usually this will be called immediately after creating a new node.
    * @param aNode      The node for which to insert formatting.
    */
   void markNodeDirty(in Node node);
 
-/* ---------- direction controller ---------- */
-
-  /**
-   * Switches the editor element direction; from "Left-to-Right" to
-   * "Right-to-Left", and vice versa.
-   */
-  void switchTextDirection();
-
 /* ------------ Output methods -------------- */
 
   /**
    * Output methods:
    * aFormatType is a mime type, like text/plain.
    */
   AString outputToString(in AString formatType,
                          in unsigned long flags);