Bug 1555227 - Make GetInternalCommand() return new commands when the command is "cmd_align" without params r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 04 Jun 2019 10:01:31 +0000
changeset 476759 c487de34b03810777f966844b6b8d0b24bb02903
parent 476758 f99b54a6da932c5e88a6932d5d6f27629ba89a4e
child 476760 47613941fb5d9eefa1e37489a77f62d5031ccb29
push id36106
push userbtara@mozilla.com
push dateTue, 04 Jun 2019 16:04:27 +0000
treeherdermozilla-central@d17dc040eec7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1555227
milestone69.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 1555227 - Make GetInternalCommand() return new commands when the command is "cmd_align" without params r=m_kato `cmd_align` is always with `nsCommandParams` when it's executed. However, when somebody checks whether the command is enabled or not, or retrieves the state, `GetInternalCommand()` is called without `nsCommandParams`. Therefore, even when `nsCommandParmas` is nullptr for `cmd_align`, `GetInternalCommand()` shouldn't warn it. Additionally, internal command supports to set `align` to empty string. Therefore, `GetInternalCommand()` also needs to support it. This patch adds `Command::FormatJustify` for the former case and `Command::FormatJustifyNone` for the latter case. Note that this does not affect to actual behavior since `AlignCommand` does not refer the result of `GetInternalCommand()`. Differential Revision: https://phabricator.services.mozilla.com/D33604
editor/libeditor/EditorCommands.h
editor/libeditor/HTMLEditorCommands.cpp
widget/CommandList.h
widget/WidgetEventImpl.cpp
--- a/editor/libeditor/EditorCommands.h
+++ b/editor/libeditor/EditorCommands.h
@@ -262,16 +262,17 @@ class EditorCommand : public nsIControll
         return EditorCommandParamType::CString |
                EditorCommandParamType::String |
                EditorCommandParamType::StateAttribute;
       // AlignCommand:
       case Command::FormatJustifyLeft:
       case Command::FormatJustifyRight:
       case Command::FormatJustifyCenter:
       case Command::FormatJustifyFull:
+      case Command::FormatJustifyNone:
         return EditorCommandParamType::CString |
                EditorCommandParamType::String |
                EditorCommandParamType::StateAttribute;
       // RemoveStylesCommand
       case Command::FormatRemove:
         return EditorCommandParamType::None;
       // IncreaseFontSizeCommand
       case Command::FormatIncreaseFontSize:
--- a/editor/libeditor/HTMLEditorCommands.cpp
+++ b/editor/libeditor/HTMLEditorCommands.cpp
@@ -497,16 +497,19 @@ nsresult MultiStateCommandBase::DoComman
   NS_WARNING(
       "who is calling MultiStateCommandBase::DoCommand (no implementation)?");
   return NS_OK;
 }
 
 nsresult MultiStateCommandBase::DoCommandParam(Command aCommand,
                                                const nsAString& aStringParam,
                                                TextEditor& aTextEditor) const {
+  NS_WARNING_ASSERTION(aCommand != Command::FormatJustify,
+                       "Command::FormatJustify should be used only for "
+                       "IsCommandEnabled() and GetCommandStateParams()");
   HTMLEditor* htmlEditor = aTextEditor.AsHTMLEditor();
   if (NS_WARN_IF(!htmlEditor)) {
     return NS_ERROR_FAILURE;
   }
   nsresult rv = SetState(MOZ_KnownLive(htmlEditor), aStringParam);
   NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "SetState() failed");
   return rv;
 }
--- a/widget/CommandList.h
+++ b/widget/CommandList.h
@@ -129,16 +129,18 @@ NS_DEFINE_COMMAND(FormatAbbreviation, cm
 NS_DEFINE_COMMAND(FormatAbsolutePosition, cmd_absPos)
 NS_DEFINE_COMMAND(FormatAcronym, cmd_acronym)
 NS_DEFINE_COMMAND(FormatCitation, cmd_cite)
 NS_DEFINE_COMMAND(FormatCode, cmd_code)
 NS_DEFINE_COMMAND(FormatDecreaseZIndex, cmd_decreaseZIndex)
 NS_DEFINE_COMMAND(FormatDocumentBackgroundColor, cmd_backgroundColor)
 NS_DEFINE_COMMAND(FormatEmphasis, cmd_em)
 NS_DEFINE_COMMAND(FormatIncreaseZIndex, cmd_increaseZIndex)
+NS_DEFINE_COMMAND(FormatJustify, cmd_align)  // Only for getting enabled/state
+NS_DEFINE_COMMAND(FormatJustifyNone, cmd_align)  // with empty string or params
 NS_DEFINE_COMMAND(FormatNoBreak, cmd_nobreak)
 NS_DEFINE_COMMAND(FormatRemoveList, cmd_removeList)
 NS_DEFINE_COMMAND(FormatSample, cmd_samp)
 NS_DEFINE_COMMAND(FormatSetBlockTextDirection, cmd_switchTextDirection)
 NS_DEFINE_COMMAND(FormatStrong, cmd_strong)
 NS_DEFINE_COMMAND(FormatTeletypeText, cmd_tt)
 NS_DEFINE_COMMAND(FormatVariable, cmd_var)
 NS_DEFINE_COMMAND(InsertDefinitionDetails, cmd_dd)
--- a/widget/WidgetEventImpl.cpp
+++ b/widget/WidgetEventImpl.cpp
@@ -188,41 +188,47 @@ Command GetInternalCommand(const char* a
                            const nsCommandParams* aCommandParams) {
   if (!aCommandName) {
     return Command::DoNothing;
   }
 
   // Special cases for "cmd_align".  It's mapped to multiple internal commands
   // with additional param.  Therefore, we cannot handle it with the hashtable.
   if (!strcmp(aCommandName, "cmd_align")) {
-    if (NS_WARN_IF(!aCommandParams)) {
-      return Command::DoNothing;
+    if (!aCommandParams) {
+      // Note that if this is called by EditorCommand::IsCommandEnabled(),
+      // it cannot set aCommandParams.  So, don't warn in this case even though
+      // this is illegal case for DoCommandParams().
+      return Command::FormatJustify;
     }
     nsAutoCString cValue;
     nsresult rv = aCommandParams->GetCString("state_attribute", cValue);
     if (NS_FAILED(rv)) {
       nsString value;  // Avoid copying the string buffer with using nsString.
       rv = aCommandParams->GetString("state_attribute", value);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return Command::DoNothing;
+      if (NS_FAILED(rv)) {
+        return Command::FormatJustifyNone;
       }
       cValue = NS_ConvertUTF16toUTF8(value);
     }
     if (cValue.LowerCaseEqualsASCII("left")) {
       return Command::FormatJustifyLeft;
     }
     if (cValue.LowerCaseEqualsASCII("right")) {
       return Command::FormatJustifyRight;
     }
     if (cValue.LowerCaseEqualsASCII("center")) {
       return Command::FormatJustifyCenter;
     }
     if (cValue.LowerCaseEqualsASCII("justify")) {
       return Command::FormatJustifyFull;
     }
+    if (cValue.IsEmpty()) {
+      return Command::FormatJustifyNone;
+    }
     return Command::DoNothing;
   }
 
   if (!sCommandHashtable) {
     sCommandHashtable = new nsDataHashtable<nsDepCharHashKey, Command>();
 #define NS_DEFINE_COMMAND(aName, aCommandStr) \
   sCommandHashtable->Put(#aCommandStr, Command::aName);