Bug 760052 - execCommand() should abort and return false for disabled commands; r=ehsan

2025-02-10 17:24:29 +00:00 · 2012-06-21 15:05:24 +03:00 · 2012-06-21 15:05:24 +03:00 · 4b464cdfe8
commit 4b464cdfe8
parent 087be3ebe6
13 changed files with 87 additions and 94 deletions
--- a/content/html/document/src/nsHTMLDocument.cpp
+++ b/content/html/document/src/nsHTMLDocument.cpp
@ -2873,8 +2873,8 @@ static const struct MidasCommand gMidasCommandTable[] = {
  { "cut",           "cmd_cut",             "", true,  false },
  { "copy",          "cmd_copy",            "", true,  false },
  { "paste",         "cmd_paste",           "", true,  false },
-  { "delete",        "cmd_delete",          "", true,  false },
-  { "forwarddelete", "cmd_forwardDelete",   "", true,  false },
+  { "delete",        "cmd_deleteCharBackward", "", true,  false },
+  { "forwarddelete", "cmd_deleteCharForward", "", true,  false },
  { "selectall",     "cmd_selectAll",       "", true,  false },
  { "undo",          "cmd_undo",            "", true,  false },
  { "redo",          "cmd_redo",            "", true,  false },
@ -3189,6 +3189,13 @@ nsHTMLDocument::ExecCommand(const nsAString& commandID,
    return NS_OK;
  }

+  // Return false for disabled commands (bug 760052)
+  bool enabled = false;
+  cmdMgr->IsCommandEnabled(cmdToDispatch.get(), window, &enabled);
+  if (!enabled) {
+    return NS_OK;
+  }
+
  if (!isBool && paramStr.IsEmpty()) {
    rv = cmdMgr->DoCommand(cmdToDispatch.get(), nsnull, window);
  } else {
--- a/dom/imptests/failures/editing/conformancetest/test_event.html.json
+++ b/dom/imptests/failures/editing/conformancetest/test_event.html.json
@ -291,21 +291,5 @@
  "Command outdent, value \"\": input event, uncanceled":true,
  "Command outdent, value \"quasit\": beforeinput event, canceled":true,
  "Command outdent, value \"quasit\": beforeinput event, uncanceled":true,
-  "Command outdent, value \"quasit\": input event, uncanceled":true,
-  "Command redo, value \"\": beforeinput event, canceled":true,
-  "Command redo, value \"\": input event, canceled":true,
-  "Command redo, value \"\": beforeinput event, uncanceled":true,
-  "Command redo, value \"\": input event, uncanceled":true,
-  "Command redo, value \"quasit\": beforeinput event, canceled":true,
-  "Command redo, value \"quasit\": input event, canceled":true,
-  "Command redo, value \"quasit\": beforeinput event, uncanceled":true,
-  "Command redo, value \"quasit\": input event, uncanceled":true,
-  "Command undo, value \"\": beforeinput event, canceled":true,
-  "Command undo, value \"\": input event, canceled":true,
-  "Command undo, value \"\": beforeinput event, uncanceled":true,
-  "Command undo, value \"\": input event, uncanceled":true,
-  "Command undo, value \"quasit\": beforeinput event, canceled":true,
-  "Command undo, value \"quasit\": input event, canceled":true,
-  "Command undo, value \"quasit\": beforeinput event, uncanceled":true,
-  "Command undo, value \"quasit\": input event, uncanceled":true
+  "Command outdent, value \"quasit\": input event, uncanceled":true
 }
--- a/dom/imptests/failures/editing/conformancetest/test_runtest.html.json
+++ b/dom/imptests/failures/editing/conformancetest/test_runtest.html.json
@ -109,21 +109,14 @@
  "[[\"stylewithcss\",\"true\"],[\"bold\",\"\"]] \"<p>[foo</p><p> <span>bar</span> </p><p>baz]</p>\" queryCommandIndeterm(\"bold\") before":true,
  "[[\"stylewithcss\",\"false\"],[\"bold\",\"\"]] \"<p>[foo</p><p> <span>bar</span> </p><p>baz]</p>\" queryCommandIndeterm(\"bold\") before":true,
  "[[\"bold\",\"\"]] \"<span>foo[</span><span>]bar</span>\" queryCommandState(\"bold\") after":true,
-  "[[\"bold\",\"\"]] \"foo<span contenteditable=false>[bar]</span>baz\": execCommand(\"bold\", false, \"\") return value":true,
  "[[\"stylewithcss\",\"true\"],[\"bold\",\"\"]] \"fo[o<span contenteditable=false>bar</span>b]az\" compare innerHTML":true,
  "[[\"stylewithcss\",\"false\"],[\"bold\",\"\"]] \"fo[o<span contenteditable=false>bar</span>b]az\" compare innerHTML":true,
-  "[[\"bold\",\"\"]] \"foo<span contenteditable=false>ba[r</span>b]az\": execCommand(\"bold\", false, \"\") return value":true,
-  "[[\"bold\",\"\"]] \"fo[o<span contenteditable=false>b]ar</span>baz\": execCommand(\"bold\", false, \"\") return value":true,
  "[[\"stylewithcss\",\"true\"],[\"bold\",\"\"]] \"<span contenteditable=false>foo<span contenteditable=true>[bar]</span>baz</span>\": execCommand(\"bold\", false, \"\") return value":true,
  "[[\"stylewithcss\",\"true\"],[\"bold\",\"\"]] \"<span contenteditable=false>foo<span contenteditable=true>[bar]</span>baz</span>\" compare innerHTML":true,
  "[[\"stylewithcss\",\"true\"],[\"bold\",\"\"]] \"<span contenteditable=false>foo<span contenteditable=true>[bar]</span>baz</span>\" queryCommandState(\"bold\") after":true,
  "[[\"stylewithcss\",\"false\"],[\"bold\",\"\"]] \"<span contenteditable=false>foo<span contenteditable=true>[bar]</span>baz</span>\": execCommand(\"bold\", false, \"\") return value":true,
  "[[\"stylewithcss\",\"false\"],[\"bold\",\"\"]] \"<span contenteditable=false>foo<span contenteditable=true>[bar]</span>baz</span>\" compare innerHTML":true,
  "[[\"stylewithcss\",\"false\"],[\"bold\",\"\"]] \"<span contenteditable=false>foo<span contenteditable=true>[bar]</span>baz</span>\" queryCommandState(\"bold\") after":true,
-  "[[\"bold\",\"\"]] \"<span contenteditable=false>fo[o<span contenteditable=true>bar</span>b]az</span>\": execCommand(\"bold\", false, \"\") return value":true,
-  "[[\"bold\",\"\"]] \"<span contenteditable=false>foo<span contenteditable=true>ba[r</span>b]az</span>\": execCommand(\"bold\", false, \"\") return value":true,
-  "[[\"bold\",\"\"]] \"<span contenteditable=false>fo[o<span contenteditable=true>b]ar</span>baz</span>\": execCommand(\"bold\", false, \"\") return value":true,
-  "[[\"bold\",\"\"]] \"<span contenteditable=false>fo[<b>o<span contenteditable=true>bar</span>b</b>]az</span>\": execCommand(\"bold\", false, \"\") return value":true,
  "[[\"stylewithcss\",\"true\"],[\"bold\",\"\"]] \"<table><tbody data-start=0 data-end=1><tr><td>foo<td>bar<td>baz</table>\" queryCommandIndeterm(\"bold\") before":true,
  "[[\"stylewithcss\",\"false\"],[\"bold\",\"\"]] \"<table><tbody data-start=0 data-end=1><tr><td>foo<td>bar<td>baz</table>\" queryCommandIndeterm(\"bold\") before":true,
  "[[\"stylewithcss\",\"true\"],[\"bold\",\"\"]] \"<table data-start=0 data-end=1><tbody><tr><td>foo<td>bar<td>baz</table>\" queryCommandIndeterm(\"bold\") before":true,
@ -1339,10 +1332,8 @@
  "[[\"stylewithcss\",\"true\"],[\"fontname\",\"sans-serif\"]] \"foo<span style=\\\"font-family: monospace\\\">b[a]r</span>baz\" compare innerHTML":true,
  "[[\"stylewithcss\",\"true\"],[\"fontname\",\"sans-serif\"]] \"foo<span style=\\\"font-family: monospace\\\">b[a]r</span>baz\" queryCommandValue(\"fontname\") before":true,
  "[[\"stylewithcss\",\"false\"],[\"fontname\",\"sans-serif\"]] \"foo<span style=\\\"font-family: monospace\\\">b[a]r</span>baz\" queryCommandValue(\"fontname\") before":true,
-  "[[\"fontname\",\"sans-serif\"]] \"foo<tt contenteditable=false>ba[r</tt>b]az\": execCommand(\"fontname\", false, \"sans-serif\") return value":true,
  "[[\"fontname\",\"sans-serif\"]] \"foo<tt contenteditable=false>ba[r</tt>b]az\" queryCommandValue(\"fontname\") before":true,
  "[[\"fontname\",\"sans-serif\"]] \"foo<tt contenteditable=false>ba[r</tt>b]az\" queryCommandValue(\"fontname\") after":true,
-  "[[\"fontname\",\"sans-serif\"]] \"fo[o<tt contenteditable=false>b]ar</tt>baz\": execCommand(\"fontname\", false, \"sans-serif\") return value":true,
  "[[\"fontname\",\"sans-serif\"]] \"fo[o<tt contenteditable=false>b]ar</tt>baz\" queryCommandValue(\"fontname\") before":true,
  "[[\"fontname\",\"sans-serif\"]] \"fo[o<tt contenteditable=false>b]ar</tt>baz\" queryCommandValue(\"fontname\") after":true,
  "[[\"fontname\",\"sans-serif\"]] \"foo<tt>{}<br></tt>bar\" queryCommandValue(\"fontname\") before":true,
--- a/editor/composer/src/nsComposerCommands.cpp
+++ b/editor/composer/src/nsComposerCommands.cpp
@ -493,18 +493,14 @@ nsOutdentCommand::IsCommandEnabled(const char * aCommandName,
                                   nsISupports *refCon,
                                   bool *outCmdEnabled)
 {
+  *outCmdEnabled = false;
+
  nsCOMPtr<nsIEditor> editor = do_QueryInterface(refCon);
-  nsCOMPtr<nsIHTMLEditor> htmlEditor = do_QueryInterface(refCon);
-  if (editor && htmlEditor)
-  {
-    bool canIndent, isEditable = false;
-    nsresult rv = editor->GetIsSelectionEditable(&isEditable);
+  if (editor) {
+    nsresult rv = editor->GetIsSelectionEditable(outCmdEnabled);
    NS_ENSURE_SUCCESS(rv, rv);
-    if (isEditable)
-      return htmlEditor->GetIndentState(&canIndent, outCmdEnabled);
  }

-  *outCmdEnabled = false;
  return NS_OK;
 }

--- a/editor/composer/src/nsComposerDocumentCommands.cpp
+++ b/editor/composer/src/nsComposerDocumentCommands.cpp
@ -184,12 +184,9 @@ nsSetDocumentStateCommand::IsCommandEnabled(const char * aCommandName,
                                            nsISupports *refCon,
                                            bool *outCmdEnabled)
 {
+  // These commands are always enabled
  NS_ENSURE_ARG_POINTER(outCmdEnabled);
-  nsCOMPtr<nsIEditor> editor = do_QueryInterface(refCon);
-  if (editor)
-    return editor->GetIsSelectionEditable(outCmdEnabled);
-
-  *outCmdEnabled = false;
+  *outCmdEnabled = true;
  return NS_OK;
 }

--- a/editor/libeditor/base/crashtests/766360.html
+++ b/editor/libeditor/base/crashtests/766360.html
@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+
+function boom()
+{
+  var r = document.createRange();
+  r.setEnd(document.createTextNode("x"), 0);
+  window.getSelection().addRange(r);
+  document.execCommand("inserthtml", false, "y");
+}
+
+</script>
+</head>
+
+<body contenteditable="true" onload="boom();"></body>
+</html>
--- a/editor/libeditor/base/crashtests/crashtests.list
+++ b/editor/libeditor/base/crashtests/crashtests.list
@ -11,4 +11,5 @@ load 636074-1.html
 load 713427-1.html
 load 713427-2.xhtml
 load 762183.html
+load 766360.html
 load 766413.html
--- a/editor/libeditor/base/nsEditorCommands.cpp
+++ b/editor/libeditor/base/nsEditorCommands.cpp
@ -527,68 +527,60 @@ nsSwitchTextDirectionCommand::GetCommandStateParams(const char *aCommandName,
 }

 NS_IMETHODIMP
-nsDeleteCommand::IsCommandEnabled(const char * aCommandName,
-                                  nsISupports *aCommandRefCon,
-                                  bool *outCmdEnabled)
+nsDeleteCommand::IsCommandEnabled(const char* aCommandName,
+                                  nsISupports* aCommandRefCon,
+                                  bool* outCmdEnabled)
 {
  NS_ENSURE_ARG_POINTER(outCmdEnabled);
  nsCOMPtr<nsIEditor> editor = do_QueryInterface(aCommandRefCon);
  *outCmdEnabled = false;

-  // we can delete when we can cut
  NS_ENSURE_TRUE(editor, NS_OK);
-    
-  bool isEditable = false;
-  nsresult rv = editor->GetIsSelectionEditable(&isEditable);
+
+  // We can generally delete whenever the selection is editable.  However,
+  // cmd_delete doesn't make sense if the selection is collapsed because it's
+  // directionless, which is the same condition under which we can't cut.
+  nsresult rv = editor->GetIsSelectionEditable(outCmdEnabled);
  NS_ENSURE_SUCCESS(rv, rv);

-  if (!isEditable)
-    return NS_OK;
-  else if (!nsCRT::strcmp(aCommandName,"cmd_delete"))
-    return editor->CanCut(outCmdEnabled);
-  else if (!nsCRT::strcmp(aCommandName,"cmd_forwardDelete"))
-    return editor->CanCut(outCmdEnabled);
-  else if (!nsCRT::strcmp(aCommandName,"cmd_deleteCharBackward"))
-    *outCmdEnabled = true;
-  else if (!nsCRT::strcmp(aCommandName,"cmd_deleteCharForward"))
-    *outCmdEnabled = true;
-  else if (!nsCRT::strcmp(aCommandName,"cmd_deleteWordBackward"))
-    *outCmdEnabled = true;
-  else if (!nsCRT::strcmp(aCommandName,"cmd_deleteWordForward"))
-    *outCmdEnabled = true;
-  else if (!nsCRT::strcmp(aCommandName,"cmd_deleteToBeginningOfLine"))
-    *outCmdEnabled = true;
-  else if (!nsCRT::strcmp(aCommandName,"cmd_deleteToEndOfLine"))
-    *outCmdEnabled = true;  
+  if (!nsCRT::strcmp("cmd_delete", aCommandName) && *outCmdEnabled) {
+    rv = editor->CanCut(outCmdEnabled);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }

  return NS_OK;
 }


 NS_IMETHODIMP
-nsDeleteCommand::DoCommand(const char *aCommandName, nsISupports *aCommandRefCon)
+nsDeleteCommand::DoCommand(const char* aCommandName,
+                           nsISupports* aCommandRefCon)
 {
  nsCOMPtr<nsIEditor> editor = do_QueryInterface(aCommandRefCon);
  NS_ENSURE_TRUE(editor, NS_ERROR_FAILURE);
-    
+
  nsIEditor::EDirection deleteDir = nsIEditor::eNone;
-  
-  if (!nsCRT::strcmp("cmd_delete",aCommandName))
+
+  if (!nsCRT::strcmp("cmd_delete", aCommandName)) {
+    // Really this should probably be eNone, but it only makes a difference if
+    // the selection is collapsed, and then this command is disabled.  So let's
+    // keep it as it always was to avoid breaking things.
    deleteDir = nsIEditor::ePrevious;
-  else if (!nsCRT::strcmp("cmd_forwardDelete", aCommandName))
+  } else if (!nsCRT::strcmp("cmd_deleteCharForward", aCommandName)) {
    deleteDir = nsIEditor::eNext;
-  else if (!nsCRT::strcmp("cmd_deleteCharBackward",aCommandName))
+  } else if (!nsCRT::strcmp("cmd_deleteCharBackward", aCommandName)) {
    deleteDir = nsIEditor::ePrevious;
-  else if (!nsCRT::strcmp("cmd_deleteCharForward",aCommandName))
-    deleteDir = nsIEditor::eNext;
-  else if (!nsCRT::strcmp("cmd_deleteWordBackward",aCommandName))
+  } else if (!nsCRT::strcmp("cmd_deleteWordBackward", aCommandName)) {
    deleteDir = nsIEditor::ePreviousWord;
-  else if (!nsCRT::strcmp("cmd_deleteWordForward",aCommandName))
+  } else if (!nsCRT::strcmp("cmd_deleteWordForward", aCommandName)) {
    deleteDir = nsIEditor::eNextWord;
-  else if (!nsCRT::strcmp("cmd_deleteToBeginningOfLine",aCommandName))
+  } else if (!nsCRT::strcmp("cmd_deleteToBeginningOfLine", aCommandName)) {
    deleteDir = nsIEditor::eToBeginningOfLine;
-  else if (!nsCRT::strcmp("cmd_deleteToEndOfLine",aCommandName))
+  } else if (!nsCRT::strcmp("cmd_deleteToEndOfLine", aCommandName)) {
    deleteDir = nsIEditor::eToEndOfLine;
+  } else {
+    MOZ_NOT_REACHED("Unrecognized nsDeleteCommand");
+  }

  return editor->DeleteSelection(deleteDir, nsIEditor::eStrip);
 }
--- a/editor/libeditor/base/nsEditorController.cpp
+++ b/editor/libeditor/base/nsEditorController.cpp
@ -58,7 +58,6 @@ nsresult nsEditorController::RegisterEditingCommands(nsIControllerCommandTable *
  NS_REGISTER_ONE_COMMAND(nsSwitchTextDirectionCommand, "cmd_switchTextDirection");

  NS_REGISTER_FIRST_COMMAND(nsDeleteCommand, "cmd_delete");
-  NS_REGISTER_NEXT_COMMAND(nsDeleteCommand, "cmd_forwardDelete");
  NS_REGISTER_NEXT_COMMAND(nsDeleteCommand, "cmd_deleteCharBackward");
  NS_REGISTER_NEXT_COMMAND(nsDeleteCommand, "cmd_deleteCharForward");
  NS_REGISTER_NEXT_COMMAND(nsDeleteCommand, "cmd_deleteWordBackward");
--- a/editor/libeditor/base/tests/test_bug408231.html
+++ b/editor/libeditor/base/tests/test_bug408231.html
@ -25,7 +25,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=408231
      ["createlink", "true"],
      ["cut", "false"],
      ["decreasefontsize", "true"],
-      ["delete", "false"],
+      ["delete", "true"],
      ["fontname", "true"],
      ["fontsize", "true"],
      ["formatblock", "true"],
@ -44,7 +44,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=408231
      ["justifyfull", "true"],
      ["justifyleft", "true"],
      ["justifyright", "true"],
-      ["outdent", "false"],
+      ["outdent", "true"],
      //["paste", "true"],
      ["redo", "false"],
      ["removeformat", "true"],
--- a/editor/libeditor/html/tests/browserscope/lib/richtext2/currentStatus.js
+++ b/editor/libeditor/html/tests/browserscope/lib/richtext2/currentStatus.js
@ -283,9 +283,6 @@ const knownFailures = {
    "QE-Proposed-INSERTLINEBREAK_TEXT-1-dM": true,
    "QE-Proposed-INSERTLINEBREAK_TEXT-1-body": true,
    "QE-Proposed-INSERTLINEBREAK_TEXT-1-div": true,
-    "QE-Proposed-OUTDENT_TEXT-1-dM": true,
-    "QE-Proposed-OUTDENT_TEXT-1-body": true,
-    "QE-Proposed-OUTDENT_TEXT-1-div": true,
    "QE-Proposed-CREATEBOOKMARK_TEXT-1-dM": true,
    "QE-Proposed-CREATEBOOKMARK_TEXT-1-body": true,
    "QE-Proposed-CREATEBOOKMARK_TEXT-1-div": true,
--- a/editor/libeditor/html/tests/test_bug612128.html
+++ b/editor/libeditor/html/tests/test_bug612128.html
@ -26,11 +26,11 @@ addLoadEvent(function() {
  document.querySelector("input").focus();
  var threw = false;
  try {
-    document.execCommand("inserthtml", null, "<span>f" + "oo</span>");
+    is(document.execCommand("inserthtml", null, "<span>f" + "oo</span>"),
+       false, "The insertHTML command should return false");
  } catch (e) {
-    threw = true;
+    ok(false, "insertHTML should not throw here");
  }
-  ok(threw, "The inserthtml command should fail");
  is(document.querySelectorAll("span").length, 0, "No span element should be injected inside the page");
  is(document.body.innerHTML.indexOf("f" + "oo"), -1, "No text should be injected inside the page");
  SimpleTest.finish();
--- a/editor/libeditor/html/tests/test_bug676401.html
+++ b/editor/libeditor/html/tests/test_bug676401.html
@ -32,13 +32,24 @@ SimpleTest.waitForFocus(runTests);

 var gBlock1, gBlock2;

-function IsCommandEnabled(command, alwaysEnabled) {
+var alwaysEnabledCommands = [
+  "contentReadOnly",
+  "copy",
+  "enableInlineTableEditing",
+  "enableObjectResizing",
+  "insertBrOnReturn",
+  "selectAll",
+  "styleWithCSS",
+];
+
+function IsCommandEnabled(command) {
  var enabled;

  // non-editable div: should return false unless alwaysEnabled
  window.getSelection().selectAllChildren(gBlock1);
  enabled = document.queryCommandEnabled(command);
-  is(enabled, alwaysEnabled, "'" + command + "' should not be enabled on a non-editable block.");
+  is(enabled, alwaysEnabledCommands.indexOf(command) != -1,
+     "'" + command + "' should not be enabled on a non-editable block.");

  // editable div: should return true
  window.getSelection().selectAllChildren(gBlock2);
@ -67,15 +78,15 @@ function runTests() {
  ];
  document.execCommand("styleWithCSS", false, false);
  for (i = 0; i < commands.length; i++)
-    IsCommandEnabled(commands[i], commands[i] == "selectAll");
+    IsCommandEnabled(commands[i]);
  document.execCommand("styleWithCSS", false, true);
  for (i = 0; i < commands.length; i++)
-    IsCommandEnabled(commands[i], commands[i] == "selectAll");
+    IsCommandEnabled(commands[i]);

  // Mozilla-specific stuff
  commands = ["enableInlineTableEditing", "enableObjectResizing", "insertBrOnReturn"];
  for (i = 0; i < commands.length; i++)
-    IsCommandEnabled(commands[i], false);
+    IsCommandEnabled(commands[i]);

  // cut/copy/paste -- SpecialPowers required
  SpecialPowers.setCharPref("capability.policy.policynames",                      "allowclipboard");
@ -84,7 +95,7 @@ function runTests() {
  SpecialPowers.setCharPref("capability.policy.allowclipboard.Clipboard.paste",   "allAccess");
  commands = ["cut", "paste", "copy"];
  for (i = 0; i < commands.length; i++) {
-    IsCommandEnabled(commands[i], commands[i] == "copy");
+    IsCommandEnabled(commands[i]);
    document.execCommand(commands[i], false, false);
  }
  SpecialPowers.clearUserPref("capability.policy.policynames");
@ -97,7 +108,7 @@ function runTests() {
  //  * there's nothing to redo if we haven't undone something first
  commands = ["delete", "undo", "redo"];
  for (i = 0; i < commands.length; i++) {
-    IsCommandEnabled(commands[i], false);
+    IsCommandEnabled(commands[i]);
    document.execCommand(commands[i], false, false);
  }