Bug 1355792 - Consider invisible nodes to be editable; r=masayuki

We can't depend on information from layout to be correct unless we've
flushed pending notifications, which we can't do at every editability
check.  So let's forget about layout.  Nobody knows why editability ever
depended on visibility in the first place.

This allows us to revert the additions from bug 795418 as well.

The original test-case submitted on the bug report was very big and
complicated, so I have a minimal test-case instead.  This might not
exactly correspond to the originally reported bug, but this fix works for
both the original and minimal test case.

MozReview-Commit-ID: LOKjlgiAEOT

--HG--
extra : rebase_source : 5aaeae87fdc0dd78cb6f1060399c09d2945f8e08

editor/libeditor/EditorBase.cpp

@@ -3491,63 +3491,6 @@ EditorBase::IsContainer(nsIDOMNode* aNode)
   return aNode ? true : false;
 }
 
-static inline bool
-IsElementVisible(Element* aElement)
-{
-  if (aElement->GetPrimaryFrame()) {
-    // It's visible, for our purposes
-    return true;
-  }
-
-  nsIContent *cur = aElement;
-  for (;;) {
-    // Walk up the tree looking for the nearest ancestor with a frame.
-    // The state of the child right below it will determine whether
-    // we might possibly have a frame or not.
-    bool haveLazyBitOnChild = cur->HasFlag(NODE_NEEDS_FRAME);
-    cur = cur->GetFlattenedTreeParent();
-    if (!cur) {
-      if (!haveLazyBitOnChild) {
-        // None of our ancestors have lazy bits set, so we shouldn't
-        // have a frame
-        return false;
-      }
-
-      // The root has a lazy frame construction bit.  We need to check
-      // our style.
-      break;
-    }
-
-    if (cur->GetPrimaryFrame()) {
-      if (!haveLazyBitOnChild) {
-        // Our ancestor directly under |cur| doesn't have lazy bits;
-        // that means we won't get a frame
-        return false;
-      }
-
-      if (cur->GetPrimaryFrame()->IsLeaf()) {
-        // Nothing under here will ever get frames
-        return false;
-      }
-
-      // Otherwise, we might end up with a frame when that lazy bit is
-      // processed.  Figure out our actual style.
-      break;
-    }
-  }
-
-  // Now it might be that we have no frame because we're in a
-  // display:none subtree, or it might be that we're just dealing with
-  // lazy frame construction and it hasn't happened yet.  Check which
-  // one it is.
-  RefPtr<nsStyleContext> styleContext =
-    nsComputedDOMStyle::GetStyleContextNoFlush(aElement, nullptr, nullptr);
-  if (styleContext) {
-    return styleContext->StyleDisplay()->mDisplay != StyleDisplay::None;
-  }
-  return false;
-}
-
 bool
 EditorBase::IsEditable(nsIDOMNode* aNode)
 {
@@ -3565,18 +3508,10 @@ EditorBase::IsEditable(nsINode* aNode)
     return false;
   }
 
-  // see if it has a frame.  If so, we'll edit it.
-  // special case for textnodes: frame must have width.
-  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:
     case nsIDOMNode::TEXT_NODE:
-      return true; // element or text node; not invisible
+      return true;
     default:
       return false;
   }

editor/libeditor/EditorBase.h

@@ -7,7 +7,6 @@
 #define mozilla_EditorBase_h
 
 #include "mozilla/Assertions.h"         // for MOZ_ASSERT, etc.
-#include "mozilla/FlushType.h"          // for FlushType enum
 #include "mozilla/OwningNonNull.h"      // for OwningNonNull
 #include "mozilla/SelectionState.h"     // for RangeUpdater, etc.
 #include "mozilla/StyleSheet.h"   // for StyleSheet
@@ -984,14 +983,6 @@ public:
    */
   void HideCaret(bool aHide);
 
-  void FlushFrames()
-  {
-    nsCOMPtr<nsIDocument> doc = GetDocument();
-    if (doc) {
-      doc->FlushPendingNotifications(FlushType::Frames);
-    }
-  }
-
 protected:
   enum Tristate
   {

editor/libeditor/HTMLEditorDataTransfer.cpp

@@ -1668,9 +1668,6 @@ HTMLEditor::PasteAsCitedQuotation(const nsAString& aCitation,
   rv = selection->Collapse(newNode, 0);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Ensure that the inserted <blockquote> has a frame to make it IsEditable.
-  FlushFrames();
-
   return Paste(aSelectionType);
 }
 
@@ -1727,7 +1724,6 @@ HTMLEditor::PasteAsPlaintextQuotation(int32_t aSelectionType)
 NS_IMETHODIMP
 HTMLEditor::InsertTextWithQuotations(const nsAString& aStringToInsert)
 {
-  AutoEditBatch beginBatching(this);
   // The whole operation should be undoable in one transaction:
   BeginTransaction();
 
@@ -1891,9 +1887,6 @@ HTMLEditor::InsertAsPlaintextQuotation(const nsAString& aQuotedText,
     selection->Collapse(newNode, 0);
   }
 
-  // Ensure that the inserted <span> has a frame to make it IsEditable.
-  FlushFrames();
-
   if (aAddCites) {
     rv = TextEditor::InsertAsQuotation(aQuotedText, aNodeInserted);
   } else {
@@ -1977,9 +1970,6 @@ HTMLEditor::InsertAsCitedQuotation(const nsAString& aQuotedText,
   // Set the selection inside the blockquote so aQuotedText will go there:
   selection->Collapse(newNode, 0);
 
-  // Ensure that the inserted <blockquote> has a frame to make it IsEditable.
-  FlushFrames();
-
   if (aInsertHTML) {
     rv = LoadHTML(aQuotedText);
   } else {

editor/libeditor/tests/mochitest.ini

@@ -232,6 +232,7 @@ skip-if = toolkit == 'android' # bug 1315898
 [test_bug1328023.html]
 [test_bug1330796.html]
 [test_bug1332876.html]
+[test_bug1355792.html]
 
 [test_CF_HTML_clipboard.html]
 subsuite = clipboard

editor/libeditor/tests/test_bug1355792.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<title>Test for Bug 1355792</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" href="/tests/SimpleTest/test.css">
+<div contenteditable><div><font><table><td>a</table><br><br><table><td>b</table></font></div></div>
+<script>
+var font = document.querySelector("font");
+getSelection().collapse(font, 1);
+document.execCommand("forwarddelete");
+is(document.body.firstChild.innerHTML,
+   "<div><font><table><tbody><tr><td>a</td></tr></tbody></table><br>"
+   + "<table><tbody><tr><td>b</td></tr></tbody></table></font></div>",
+   "No creating an extra <br>");
+is(getSelection().focusNode, font, "Selection node should not change");
+is(getSelection().focusOffset, 1, "Selection offset should not move");
+ok(getSelection().isCollapsed, "Selection should be collapsed");
+</script>