Bug 612128 - Prevent the editor from modifying nodes which are not under an editing host; r=roc,bzbarsky

This patch ensures that the NODE_IS_EDITABLE flag is only set on nodes living
under an editing host.  Things like text controls which used to have that flag
previously will not have it any more.  The flag would be set on their anonymous
div node instead.  Note that if text controls actually fall under an editing
host, they will get the NODE_IS_EDITABLE flag.

This patch also makes nsHTMLEditor::IsEditable return sane results (text nodes
are always considered to be editable).

content/html/content/src/nsGenericHTMLElement.cpp

@@ -1789,37 +1789,6 @@ nsGenericHTMLElement::MapCommonAttributesInto(const nsMappedAttributes* aAttribu
   }
 }
 
-void
-nsGenericHTMLFormElement::UpdateEditableFormControlState(PRBool aNotify)
-{
-  // nsCSSFrameConstructor::MaybeConstructLazily is based on the logic of this
-  // function, so should be kept in sync with that.
-
-  ContentEditableTristate value = GetContentEditableValue();
-  if (value != eInherit) {
-    DoSetEditableFlag(!!value, aNotify);
-    return;
-  }
-
-  nsIContent *parent = GetParent();
-
-  if (parent && parent->HasFlag(NODE_IS_EDITABLE)) {
-    DoSetEditableFlag(PR_TRUE, aNotify);
-    return;
-  }
-
-  if (!IsTextControl(PR_FALSE)) {
-    DoSetEditableFlag(PR_FALSE, aNotify);
-    return;
-  }
-
-  // If not contentEditable we still need to check the readonly attribute.
-  PRBool roState;
-  GetBoolAttr(nsGkAtoms::readonly, &roState);
-
-  DoSetEditableFlag(!roState, aNotify);
-}
-
 
 /* static */ const nsGenericHTMLElement::MappedAttributeEntry
 nsGenericHTMLElement::sCommonAttributeMap[] = {
@@ -2912,6 +2881,18 @@ nsGenericHTMLFormElement::IntrinsicState() const
       state |= NS_EVENT_STATE_DEFAULT;
   }
 
+  // Make the text controls read-write
+  if (!state.HasState(NS_EVENT_STATE_MOZ_READWRITE) &&
+      IsTextControl(PR_FALSE)) {
+    PRBool roState;
+    GetBoolAttr(nsGkAtoms::readonly, &roState);
+
+    if (!roState) {
+      state |= NS_EVENT_STATE_MOZ_READWRITE;
+      state &= ~NS_EVENT_STATE_MOZ_READONLY;
+    }
+  }
+
   return state;
 }
 

content/html/content/src/nsGenericHTMLElement.h

@@ -928,8 +928,6 @@ protected:
   virtual nsresult AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                                 const nsAString* aValue, PRBool aNotify);
 
-  void UpdateEditableFormControlState(PRBool aNotify);
-
   /**
    * This method will update the form owner, using @form or looking to a parent.
    *

content/html/content/src/nsHTMLInputElement.cpp

@@ -848,7 +848,6 @@ nsHTMLInputElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
       UpdateTypeMismatchValidityState();
     }
 
-    UpdateEditableState(aNotify);
     UpdateState(aNotify);
   }
 

content/html/content/src/nsHTMLInputElement.h

@@ -235,11 +235,6 @@ public:
 
   NS_IMETHOD FireAsyncClickHandler();
 
-  virtual void UpdateEditableState(PRBool aNotify)
-  {
-    return UpdateEditableFormControlState(aNotify);
-  }
-
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsHTMLInputElement,
                                            nsGenericHTMLFormElement)
 

content/html/content/src/nsHTMLTextAreaElement.cpp

@@ -200,11 +200,6 @@ public:
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
 
-  virtual void UpdateEditableState(PRBool aNotify)
-  {
-    return UpdateEditableFormControlState(aNotify);
-  }
-
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsHTMLTextAreaElement,
                                            nsGenericHTMLFormElement)
 
@@ -1267,9 +1262,6 @@ nsHTMLTextAreaElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
       UpdateTooLongValidityState();
     }
 
-    if (aName == nsGkAtoms::readonly) {
-      UpdateEditableState(aNotify);
-    }
     UpdateState(aNotify);
   }
 

content/html/content/src/nsTextEditorState.cpp

@@ -1559,6 +1559,9 @@ nsTextEditorState::CreateRootNode()
 nsresult
 nsTextEditorState::InitializeRootNode()
 {
+  // Make our root node editable
+  mRootNode->SetFlags(NODE_IS_EDITABLE);
+
   // Set the necessary classes on the text control. We use class values
   // instead of a 'style' attribute so that the style comes from a user-agent
   // style sheet and is still applied even if author styles are disabled.

editor/libeditor/base/crashtests/382527-1.html

@@ -36,7 +36,9 @@ function init3()
   rng.setEnd(textNode, 1);
   targetWindow.getSelection().addRange(rng);
 
-  targetDocument.execCommand("inserthtml", false, "<p>");
+  try {
+    targetDocument.execCommand("inserthtml", false, "<p>");
+  } catch(e) {}
   
   document.documentElement.removeAttribute("class");
 }

editor/libeditor/base/nsEditor.h

@@ -548,7 +548,7 @@ public:
   virtual PRBool IsContainer(nsIDOMNode *aNode);
 
   /** returns PR_TRUE if aNode is an editable node */
-  PRBool IsEditable(nsIDOMNode *aNode);
+  virtual PRBool IsEditable(nsIDOMNode *aNode);
 
   virtual PRBool IsTextInDirtyFrameVisible(nsIDOMNode *aNode);
 

editor/libeditor/base/nsEditorCommands.cpp

@@ -661,13 +661,8 @@ nsSelectAllCommand::IsCommandEnabled(const char * aCommandName,
 {
   NS_ENSURE_ARG_POINTER(outCmdEnabled);
 
-  // you can select all if there is an editor (and potentially no contents)
-  // some day we may want to change this
-  nsCOMPtr<nsIEditor> editor = do_QueryInterface(aCommandRefCon);
-  if (editor)
-    return editor->GetIsSelectionEditable(outCmdEnabled);
-
-  *outCmdEnabled = PR_FALSE;
+  // You can always select all!
+  *outCmdEnabled = PR_TRUE;
   return NS_OK;
 }
 

editor/libeditor/html/nsHTMLDataTransfer.cpp

@@ -311,7 +311,9 @@ nsHTMLEditor::DoInsertHTMLWithContext(const nsAString & aInputString,
     // if caller didn't provide the destination/target node,
     // fetch the paste insertion point from our selection
     res = GetStartNodeAndOffset(selection, getter_AddRefs(targetNode), &targetOffset);
-    if (!targetNode) res = NS_ERROR_FAILURE;
+    if (!targetNode || !IsEditable(targetNode)) {
+      res = NS_ERROR_FAILURE;
+    }
     NS_ENSURE_SUCCESS(res, res);
   }
   else

editor/libeditor/html/nsHTMLEditor.cpp

@@ -431,6 +431,12 @@ nsHTMLEditor::FindSelectionRoot(nsINode *aNode)
   }
 
   if (!content->HasFlag(NODE_IS_EDITABLE)) {
+    // If the content is in read-write state but is not editable itself,
+    // return it as the selection root.
+    if (content->IsElement() &&
+        content->AsElement()->State().HasState(NS_EVENT_STATE_MOZ_READWRITE)) {
+      return content.forget();
+    }
     return nsnull;
   }
 
@@ -6028,3 +6034,22 @@ nsHTMLEditor::GetPreferredIMEState(PRUint32 *aState)
   *aState = nsIContent::IME_STATUS_ENABLE;
   return NS_OK;
 }
+
+PRBool
+nsHTMLEditor::IsEditable(nsIDOMNode* aNode) {
+  if (!nsPlaintextEditor::IsEditable(aNode)) {
+    return PR_FALSE;
+  }
+  nsCOMPtr<nsINode> node = do_QueryInterface(aNode);
+  if (!node) {
+    // If what we're dealing with is not a node, then it's not editable!
+    return PR_FALSE;
+  }
+  if (node->IsElement()) {
+    // If we're dealing with an element, then ask it whether it's editable.
+    return node->IsEditable();
+  }
+  // We might be dealing with a text node for example, which we always consider
+  // to be editable.
+  return PR_TRUE;
+}

editor/libeditor/html/nsHTMLEditor.h

@@ -154,6 +154,7 @@ public:
   virtual already_AddRefed<nsIDOMEventTarget> GetDOMEventTarget();
   virtual already_AddRefed<nsIContent> FindSelectionRoot(nsINode *aNode);
   virtual PRBool IsAcceptableInputEvent(nsIDOMEvent* aEvent);
+  virtual PRBool IsEditable(nsIDOMNode *aNode);
 
   /* ------------ nsStubMutationObserver overrides --------- */
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED

editor/libeditor/html/tests/Makefile.in

@@ -80,6 +80,7 @@ _TEST_FILES = \
 		test_bug599322.html \
 		test_bug607584.html \
 		test_bug611182.html \
+		test_bug612128.html \
 		test_bug612447.html \
 		test_bug620906.html \
 		test_bug622371.html \

editor/libeditor/html/tests/test_bug612128.html

@@ -0,0 +1,42 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=612128
+-->
+<head>
+  <title>Test for Bug 612128</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=612128">Mozilla Bug 612128</a>
+<p id="display"></p>
+<div id="content">
+<input>
+<div contenteditable></div>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 612128 **/
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(function() {
+  document.querySelector("input").focus();
+  var threw = false;
+  try {
+    document.execCommand("inserthtml", null, "<span>f" + "oo</span>");
+  } catch (e) {
+    threw = true;
+  }
+  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();
+});
+
+</script>
+</pre>
+</body>
+</html>

editor/libeditor/html/tests/test_bug676401.html

@@ -35,10 +35,17 @@ var gBlock1, gBlock2;
 function IsCommandEnabled(command) {
   var enabled;
 
+  var resultInNonEditableRegion = false;
+  if (command == "selectAll") {
+    // The select all command is sort of exceptional, as it needs to be enabled
+    // everywhere.
+    resultInNonEditableRegion = true;
+  }
+
   // non-editable div: should return false
   window.getSelection().selectAllChildren(gBlock1);
   enabled = document.queryCommandEnabled(command);
-  is(enabled, false, "'" + command + "' should not be enabled on a non-editable block.");
+  is(enabled, resultInNonEditableRegion, "'" + command + "' should not be enabled on a non-editable block.");
 
   // editable div: should return true
   window.getSelection().selectAllChildren(gBlock2);

layout/base/nsCSSFrameConstructor.cpp

@@ -3883,6 +3883,31 @@ nsCSSFrameConstructor::CreateAnonymousFrames(nsFrameConstructorState& aState,
   return NS_OK;
 }
 
+static void
+SetFlagsOnSubtree(nsIContent *aNode, PtrBits aFlagsToSet)
+{
+#ifdef DEBUG
+  // Make sure that the node passed to us doesn't have any XBL children
+  {
+    nsIDocument *doc = aNode->GetOwnerDoc();
+    NS_ASSERTION(doc, "The node must be in a document");
+    NS_ASSERTION(!doc->BindingManager()->GetXBLChildNodesFor(aNode),
+                 "The node should not have any XBL children");
+  }
+#endif
+
+  // Set the flag on the node itself
+  aNode->SetFlags(aFlagsToSet);
+
+  // Set the flag on all of its children recursively
+  PRUint32 count;
+  nsIContent * const *children = aNode->GetChildArray(&count);
+
+  for (PRUint32 index = 0; index < count; ++index) {
+    SetFlagsOnSubtree(children[index], aFlagsToSet);
+  }
+}
+
 nsresult
 nsCSSFrameConstructor::GetAnonymousContent(nsIContent* aParent,
                                            nsIFrame* aParentFrame,
@@ -3910,7 +3935,17 @@ nsCSSFrameConstructor::GetAnonymousContent(nsIContent* aParent,
       content->SetNativeAnonymous();
     }
 
+    PRBool anonContentIsEditable = content->HasFlag(NODE_IS_EDITABLE);
     rv = content->BindToTree(mDocument, aParent, aParent, PR_TRUE);
+    // If the anonymous content creator requested that the content should be
+    // editable, honor its request.
+    // We need to set the flag on the whole subtree, because existing
+    // children's flags have already been set as part of the BindToTree operation.
+    if (anonContentIsEditable) {
+      NS_ASSERTION(aParentFrame->GetType() == nsGkAtoms::textInputFrame,
+                   "We only expect this for anonymous content under a text control frame");
+      SetFlagsOnSubtree(content, NODE_IS_EDITABLE);
+    }
     if (NS_FAILED(rv)) {
       content->UnbindFromTree();
       return rv;
@@ -6137,25 +6172,6 @@ nsCSSFrameConstructor::ReframeTextIfNeeded(nsIContent* aParentContent,
   ContentInserted(aParentContent, aContent, nsnull, PR_FALSE);
 }
 
-// We want to disable lazy frame construction for nodes that are under an
-// editor. We use nsINode::IsEditable, but that includes inputs with type text
-// and password and textareas, which are common and aren't really editable (the
-// native anonymous content under them is what is actually editable) so we want
-// to construct frames for those lazily.
-// The logic for this check is based on
-// nsGenericHTMLFormElement::UpdateEditableFormControlState and so must be kept
-// in sync with that.  MayHaveContentEditableAttr() being true only indicates
-// a contenteditable attribute, it doesn't indicate whether it is true or false,
-// so we force eager construction in some cases when the node is not editable,
-// but that should be rare.
-static inline PRBool
-IsActuallyEditable(nsIContent* aContainer, nsIContent* aChild)
-{
-  return (aChild->IsEditable() &&
-          (aContainer->IsEditable() ||
-           aChild->MayHaveContentEditableAttr()));
-}
-
 // For inserts aChild should be valid, for appends it should be null.
 // Returns true if this operation can be lazy, false if not.
 PRBool
@@ -6170,7 +6186,7 @@ nsCSSFrameConstructor::MaybeConstructLazily(Operation aOperation,
 
   if (aOperation == CONTENTINSERT) {
     if (aChild->IsRootOfAnonymousSubtree() ||
-        aChild->IsXUL() || IsActuallyEditable(aContainer, aChild)) {
+        aChild->IsEditable() || aChild->IsXUL()) {
       return PR_FALSE;
     }
   } else { // CONTENTAPPEND
@@ -6179,7 +6195,7 @@ nsCSSFrameConstructor::MaybeConstructLazily(Operation aOperation,
     for (nsIContent* child = aChild; child; child = child->GetNextSibling()) {
       NS_ASSERTION(!child->IsRootOfAnonymousSubtree(),
                    "Should be coming through the CONTENTAPPEND case");
-      if (child->IsXUL() || IsActuallyEditable(aContainer, child)) {
+      if (child->IsXUL() || child->IsEditable()) {
         return PR_FALSE;
       }
     }

layout/generic/nsIAnonymousContentCreator.h

@@ -79,6 +79,10 @@ public:
    * Creates "native" anonymous content and adds the created content to
    * the aElements array. None of the returned elements can be nsnull.
    *
+   * If the anonymous content creator sets the editable flag on some
+   * of the elements that it creates, the flag will be applied to the node
+   * upon being bound to the document.
+   *
    * @note The returned elements are owned by this object. This object is
    *       responsible for calling UnbindFromTree on the elements it returned
    *       from CreateAnonymousContent when appropriate (i.e. before releasing

layout/reftests/editor/readonly-editable-ref.html

@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <input>
+    <input readonly>
+    <input type=password>
+    <input type=password readonly>
+    <input type=email>
+    <input type=email readonly>
+    <textarea></textarea>
+    <textarea readonly></textarea>
+  </body>
+</html>

layout/reftests/editor/readonly-editable.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      :-moz-read-write + span {
+        display: none;
+      }
+      span {
+        color: transparent; /* workaround for bug 617524 */
+        outline: 1px solid green;
+      }
+    </style>
+  </head>
+  <body contenteditable>
+    <input><span>hide me</span>
+    <input readonly><span>hide me</span>
+    <input type=password><span>hide me</span>
+    <input type=password readonly><span>hide me</span>
+    <input type=email><span>hide me</span>
+    <input type=email readonly><span>hide me</span>
+    <textarea></textarea><span>hide me</span>
+    <textarea readonly></textarea><span>hide me</span>
+  </body>
+</html>

layout/reftests/editor/readonly-non-editable-ref.html

@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      span {
+        color: transparent; /* workaround for bug 617524 */
+        outline: 1px solid green;
+      }
+    </style>
+  </head>
+  <body>
+    <input><span>hide me</span>
+    <input readonly>
+    <input type=password><span>hide me</span>
+    <input type=password readonly>
+    <input type=email><span>hide me</span>
+    <input type=email readonly>
+    <textarea></textarea><span>hide me</span>
+    <textarea readonly></textarea>
+  </body>
+</html>

layout/reftests/editor/readonly-non-editable.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      :-moz-read-only + span {
+        display: none;
+      }
+      span {
+        color: transparent; /* workaround for bug 617524 */
+        outline: 1px solid green;
+      }
+    </style>
+  </head>
+  <body>
+    <input><span>hide me</span>
+    <input readonly><span>hide me</span>
+    <input type=password><span>hide me</span>
+    <input type=password readonly><span>hide me</span>
+    <input type=email><span>hide me</span>
+    <input type=email readonly><span>hide me</span>
+    <textarea></textarea><span>hide me</span>
+    <textarea readonly></textarea><span>hide me</span>
+  </body>
+</html>

layout/reftests/editor/readwrite-editable-ref.html

@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <input>
+    <input readonly>
+    <input type=password>
+    <input type=password readonly>
+    <input type=email>
+    <input type=email readonly>
+    <textarea></textarea>
+    <textarea readonly></textarea>
+  </body>
+</html>

layout/reftests/editor/readwrite-editable.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      :-moz-read-write + span {
+        display: none;
+      }
+      span {
+        color: transparent; /* workaround for bug 617524 */
+        outline: 1px solid green;
+      }
+    </style>
+  </head>
+  <body contenteditable>
+    <input><span>hide me</span>
+    <input readonly><span>hide me</span>
+    <input type=password><span>hide me</span>
+    <input type=password readonly><span>hide me</span>
+    <input type=email><span>hide me</span>
+    <input type=email readonly><span>hide me</span>
+    <textarea></textarea><span>hide me</span>
+    <textarea readonly></textarea><span>hide me</span>
+  </body>
+</html>

layout/reftests/editor/readwrite-non-editable-ref.html

@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      span {
+        color: transparent; /* workaround for bug 617524 */
+        outline: 1px solid green;
+      }
+    </style>
+  </head>
+  <body>
+    <input>
+    <input readonly><span>hide me</span>
+    <input type=password>
+    <input type=password readonly><span>hide me</span>
+    <input type=email>
+    <input type=email readonly><span>hide me</span>
+    <textarea></textarea>
+    <textarea readonly></textarea><span>hide me</span>
+  </body>
+</html>

layout/reftests/editor/readwrite-non-editable.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      :-moz-read-write + span {
+        display: none;
+      }
+      span {
+        color: transparent; /* workaround for bug 617524 */
+        outline: 1px solid green;
+      }
+    </style>
+  </head>
+  <body>
+    <input><span>hide me</span>
+    <input readonly><span>hide me</span>
+    <input type=password><span>hide me</span>
+    <input type=password readonly><span>hide me</span>
+    <input type=email><span>hide me</span>
+    <input type=email readonly><span>hide me</span>
+    <textarea></textarea><span>hide me</span>
+    <textarea readonly></textarea><span>hide me</span>
+  </body>
+</html>

layout/reftests/editor/reftest.list

@@ -69,4 +69,8 @@ skip-if(Android) == 674212-spellcheck.html 674212-spellcheck-ref.html
 skip-if(Android) == 338427-2.html 338427-2-ref.html
 skip-if(Android) == 338427-3.html 338427-3-ref.html
 skip-if(Android) == 462758-grabbers-resizers.html 462758-grabbers-resizers-ref.html
+== readwrite-non-editable.html readwrite-non-editable-ref.html
+== readwrite-editable.html readwrite-editable-ref.html
+== readonly-non-editable.html readonly-non-editable-ref.html
+== readonly-editable.html readonly-editable-ref.html
 == dynamic-overflow-change.html dynamic-overflow-change-ref.html