Bug 796839 - Part 4: Don't pretend that empty text nodes are not editable; r=roc
authorEhsan Akhgari <ehsan@mozilla.com>
Wed, 03 Oct 2012 21:25:00 -0400
changeset 109317 e92287b62a547f0ae7cac5d8487e1159f4b13557
parent 109316 7297e7f7b7481785eb2645178718648aa856b5da
child 109318 5b5981c2106bf4354446fa3bb65f7e4606315924
push id15965
push usereakhgari@mozilla.com
push dateFri, 05 Oct 2012 03:54:10 +0000
treeherdermozilla-inbound@e92287b62a54 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs796839
milestone18.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 796839 - Part 4: Don't pretend that empty text nodes are not editable; r=roc This is the real fix for this bug. Previously we mistakenly thought that if a text node is empty (or has only whitespace content), it is not editable. This patch removes that check completely and makes us treat text nodes the same way that we treat element nodes.
editor/libeditor/base/nsEditor.cpp
editor/libeditor/base/nsEditor.h
editor/libeditor/html/nsHTMLEditor.cpp
editor/libeditor/html/nsHTMLEditor.h
editor/libeditor/html/tests/Makefile.in
editor/libeditor/html/tests/test_bug796839.html
--- a/editor/libeditor/base/nsEditor.cpp
+++ b/editor/libeditor/base/nsEditor.cpp
@@ -3636,30 +3636,16 @@ nsEditor::IsDescendantOfEditorRoot(nsINo
 }
 
 bool 
 nsEditor::IsContainer(nsIDOMNode *aNode)
 {
   return aNode ? true : false;
 }
 
-bool
-nsEditor::IsTextInDirtyFrameVisible(nsIContent *aNode)
-{
-  MOZ_ASSERT(aNode);
-  MOZ_ASSERT(aNode->NodeType() == nsIDOMNode::TEXT_NODE);
-
-  // virtual method
-  //
-  // If this is a simple non-html editor,
-  // the best we can do is to assume it's visible.
-
-  return true;
-}
-
 static inline bool
 IsElementVisible(dom::Element* aElement)
 {
   if (aElement->GetPrimaryFrame()) {
     // It's visible, for our purposes
     return true;
   }
 
@@ -3732,19 +3718,18 @@ nsEditor::IsEditable(nsIContent *aNode)
   if (aNode->IsElement() && !IsElementVisible(aNode->AsElement())) {
     // If the element has no frame, it's not editable.  Note that we
     // need to check IsElement() here, because some of our tests
     // rely on frameless textnodes being visible.
     return false;
   }
   switch (aNode->NodeType()) {
     case nsIDOMNode::ELEMENT_NODE:
-      return true; // not a text node; not invisible
     case nsIDOMNode::TEXT_NODE:
-      return IsTextInDirtyFrameVisible(aNode);
+      return true; // element or text node; not invisible
     default:
       return false;
   }
 }
 
 bool
 nsEditor::IsMozEditorBogusNode(nsIContent *element)
 {
--- a/editor/libeditor/base/nsEditor.h
+++ b/editor/libeditor/base/nsEditor.h
@@ -581,21 +581,16 @@ public:
 
   /** returns true if aNode is a container */
   virtual bool IsContainer(nsIDOMNode *aNode);
 
   /** returns true if aNode is an editable node */
   bool IsEditable(nsIDOMNode *aNode);
   virtual bool IsEditable(nsIContent *aNode);
 
-  /**
-   * aNode must be a non-null text node.
-   */
-  virtual bool IsTextInDirtyFrameVisible(nsIContent *aNode);
-
   /** returns true if aNode is a MozEditorBogus node */
   bool IsMozEditorBogusNode(nsIContent *aNode);
 
   /** counts number of editable child nodes */
   uint32_t CountEditableChildren(nsINode* aNode);
   
   /** Find the deep first and last children. */
   nsINode* GetFirstEditableNode(nsINode* aRoot);
--- a/editor/libeditor/html/nsHTMLEditor.cpp
+++ b/editor/libeditor/html/nsHTMLEditor.cpp
@@ -4333,33 +4333,16 @@ nsHTMLEditor::GetLastEditableLeaf(nsIDOM
       child = nullptr;
     }
   }
   
   *aOutLastLeaf = child;
   return res;
 }
 
-bool
-nsHTMLEditor::IsTextInDirtyFrameVisible(nsIContent *aNode)
-{
-  MOZ_ASSERT(aNode);
-  MOZ_ASSERT(aNode->NodeType() == nsIDOMNode::TEXT_NODE);
-
-  bool isEmptyTextNode;
-  nsresult rv = IsVisTextNode(aNode, &isEmptyTextNode, false);
-  if (NS_FAILED(rv)) {
-    // We are following the historical decision:
-    //   if we don't know, we say it's visible...
-    return true;
-  }
-
-  return !isEmptyTextNode;
-}
-
 
 ///////////////////////////////////////////////////////////////////////////
 // IsVisTextNode: figure out if textnode aTextNode has any visible content.
 //                  
 nsresult
 nsHTMLEditor::IsVisTextNode(nsIContent* aNode,
                             bool* outIsEmptyNode,
                             bool aSafeToAskFrames)
--- a/editor/libeditor/html/nsHTMLEditor.h
+++ b/editor/libeditor/html/nsHTMLEditor.h
@@ -339,21 +339,16 @@ public:
   //  e.g., when setting at beginning of a table cell
   // This will stop at a table, however, since we don't want to
   //  "drill down" into nested tables.
   // aSelection is optional -- if null, we get current seletion
   nsresult CollapseSelectionToDeepestNonTableFirstChild(nsISelection *aSelection, nsIDOMNode *aNode);
 
   /**
    * aNode must be a non-null text node.
-   */
-  virtual bool IsTextInDirtyFrameVisible(nsIContent *aNode);
-
-  /**
-   * aNode must be a non-null text node.
    * outIsEmptyNode must be non-null.
    */
   nsresult IsVisTextNode(nsIContent* aNode,
                          bool* outIsEmptyNode,
                          bool aSafeToAskFrames);
   nsresult IsEmptyNode(nsIDOMNode *aNode, bool *outIsEmptyBlock, 
                        bool aMozBRDoesntCount = false,
                        bool aListOrCellNotEmpty = false,
--- a/editor/libeditor/html/tests/Makefile.in
+++ b/editor/libeditor/html/tests/Makefile.in
@@ -76,16 +76,17 @@ MOCHITEST_FILES = \
 		test_select_all_without_body.html \
 		file_select_all_without_body.html \
 		test_root_element_replacement.html \
 		test_bug738366.html \
 		test_bug757371.html \
 		test_bug767684.html \
 		test_bug780035.html \
 		test_bug787432.html \
+		test_bug796839.html \
 		$(NULL)
 
 ifneq (mobile,$(MOZ_BUILD_APP))
 MOCHITEST_FILES +=  test_spellcheck_pref.html \
 		$(NULL)
 endif
 
 _DATA_FILES = \
new file mode 100644
--- /dev/null
+++ b/editor/libeditor/html/tests/test_bug796839.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=796839
+-->
+<title>Test for Bug 796839</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" href="/tests/SimpleTest/test.css">
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=796839">Mozilla Bug 796839</a>
+<div id="test" contenteditable><br></div>
+<script>
+var div = document.getElementById("test");
+var text = document.createTextNode("");
+div.insertBefore(text, div.firstChild);
+getSelection().collapse(text, 0);
+document.execCommand("inserthtml", false, "x");
+is(div.textContent, 'x', "Empty textnodes should be editable");
+</script>