Bug 1411687 - part 2: Rewrite the check to insert a <br> element in HTMLEditRules::WillInsertBreak() r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 27 Oct 2017 01:27:44 +0900
changeset 688513 2cc74de2adfeb2531fef963f5ba287f09f3e4a2f
parent 688512 93c2eec593eb00f9e17631ac97b94dad97245b88
child 688514 bac3bfbccf4897192fb1271ae3893e60ba60380c
push id86761
push usermasayuki@d-toybox.com
push dateMon, 30 Oct 2017 05:49:37 +0000
reviewersm_kato
bugs1411687
milestone58.0a1
Bug 1411687 - part 2: Rewrite the check to insert a <br> element in HTMLEditRules::WillInsertBreak() r?m_kato Currently, HTMLEditRules::WillInsertBreak() checks if the editing host can contain a <p> element as a child or a descendant. However, this is not enough. If an inline element has a block element which can contain a <p> element, current implementation considers to insert a <br>. This is possible when * The editing host is an unknown element including user defined element. * The editing host is an inline element and its children and/or descendants were added by JS. E.g., <span contenteditable> element can have <div> element. I think that we should consider to insert a <br> element when: - There is no block ancestors in the editing host. - The editing host is the only block element and it cannot contain <p> element or the default paragraph separator is <br> element. - The nearest block ancestor isn't a single-line container declared in the execCommand spec and there are no block elements which can contain <p> element. Note that Chromium checks if CSS box of ancestors is block too. However, it must be out of scope of this bug. MozReview-Commit-ID: HdjU9t83Nd1
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
editor/libeditor/HTMLEditUtils.cpp
editor/libeditor/HTMLEditUtils.h
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLEditor.h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -3538,54 +3538,54 @@ EditorBase::IsBlockNode(nsINode* aNode)
   // screwing around with the class hierarchy here in order
   // to not duplicate the code in GetNextNode/GetPrevNode
   // across both EditorBase/HTMLEditor.
   return false;
 }
 
 bool
 EditorBase::CanContain(nsINode& aParent,
-                       nsIContent& aChild)
+                       nsIContent& aChild) const
 {
   switch (aParent.NodeType()) {
     case nsIDOMNode::ELEMENT_NODE:
     case nsIDOMNode::DOCUMENT_FRAGMENT_NODE:
       return TagCanContain(*aParent.NodeInfo()->NameAtom(), aChild);
   }
   return false;
 }
 
 bool
 EditorBase::CanContainTag(nsINode& aParent,
-                          nsAtom& aChildTag)
+                          nsAtom& aChildTag) const
 {
   switch (aParent.NodeType()) {
     case nsIDOMNode::ELEMENT_NODE:
     case nsIDOMNode::DOCUMENT_FRAGMENT_NODE:
       return TagCanContainTag(*aParent.NodeInfo()->NameAtom(), aChildTag);
   }
   return false;
 }
 
 bool
 EditorBase::TagCanContain(nsAtom& aParentTag,
-                          nsIContent& aChild)
+                          nsIContent& aChild) const
 {
   switch (aChild.NodeType()) {
     case nsIDOMNode::TEXT_NODE:
     case nsIDOMNode::ELEMENT_NODE:
     case nsIDOMNode::DOCUMENT_FRAGMENT_NODE:
       return TagCanContainTag(aParentTag, *aChild.NodeInfo()->NameAtom());
   }
   return false;
 }
 
 bool
 EditorBase::TagCanContainTag(nsAtom& aParentTag,
-                             nsAtom& aChildTag)
+                             nsAtom& aChildTag) const
 {
   return true;
 }
 
 bool
 EditorBase::IsRoot(nsIDOMNode* inNode)
 {
   NS_ENSURE_TRUE(inNode, false);
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -765,20 +765,20 @@ public:
   static inline bool NodeIsType(nsIDOMNode* aNode, nsAtom* aTag)
   {
     return GetTag(aNode) == aTag;
   }
 
   /**
    * Returns true if aParent can contain a child of type aTag.
    */
-  bool CanContain(nsINode& aParent, nsIContent& aChild);
-  bool CanContainTag(nsINode& aParent, nsAtom& aTag);
-  bool TagCanContain(nsAtom& aParentTag, nsIContent& aChild);
-  virtual bool TagCanContainTag(nsAtom& aParentTag, nsAtom& aChildTag);
+  bool CanContain(nsINode& aParent, nsIContent& aChild) const;
+  bool CanContainTag(nsINode& aParent, nsAtom& aTag) const;
+  bool TagCanContain(nsAtom& aParentTag, nsIContent& aChild) const;
+  virtual bool TagCanContainTag(nsAtom& aParentTag, nsAtom& aChildTag) const;
 
   /**
    * Returns true if aNode is our root node.
    */
   bool IsRoot(nsIDOMNode* inNode);
   bool IsRoot(nsINode* inNode);
   bool IsEditorRoot(nsINode* aNode);
 
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -1532,16 +1532,45 @@ HTMLEditRules::WillLoadHTML(Selection* a
     }
     mHTMLEditor->DeleteNode(mBogusNode);
     mBogusNode = nullptr;
   }
 
   return NS_OK;
 }
 
+bool
+HTMLEditRules::CanContainParagraph(Element& aElement) const
+{
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return false;
+  }
+
+  if (mHTMLEditor->CanContainTag(aElement, *nsGkAtoms::p)) {
+    return true;
+  }
+
+  // Even if the element cannot have a <p> element as a child, it can contain
+  // <p> element as a descendant if it's one of the following elements.
+  if (aElement.IsAnyOfHTMLElements(nsGkAtoms::ol,
+                                   nsGkAtoms::ul,
+                                   nsGkAtoms::dl,
+                                   nsGkAtoms::table,
+                                   nsGkAtoms::thead,
+                                   nsGkAtoms::tbody,
+                                   nsGkAtoms::tfoot,
+                                   nsGkAtoms::tr)) {
+    return true;
+  }
+
+  // XXX Otherwise, Chromium checks the CSS box is a block, but we don't do it
+  //     for now.
+  return false;
+}
+
 nsresult
 HTMLEditRules::WillInsertBreak(Selection& aSelection,
                                bool* aCancel,
                                bool* aHandled)
 {
   MOZ_ASSERT(aCancel && aHandled);
   *aCancel = false;
   *aHandled = false;
@@ -1580,64 +1609,91 @@ HTMLEditRules::WillInsertBreak(Selection
   int32_t offset = aSelection.GetRangeAt(0)->StartOffset();
 
   // Do nothing if the node is read-only
   if (!htmlEditor->IsModifiableNode(node)) {
     *aCancel = true;
     return NS_OK;
   }
 
-  // Identify the block
-  nsCOMPtr<Element> blockParent = htmlEditor->GetBlock(node);
-  NS_ENSURE_TRUE(blockParent, NS_ERROR_FAILURE);
-
   // If the active editing host is an inline element, or if the active editing
   // host is the block parent itself and we're configured to use <br> as a
   // paragraph separator, just append a <br>.
   nsCOMPtr<Element> host = htmlEditor->GetActiveEditingHost();
   if (NS_WARN_IF(!host)) {
     return NS_ERROR_FAILURE;
   }
-  ParagraphSeparator separator = mHTMLEditor->GetDefaultParagraphSeparator();
-  if (!IsBlockNode(*host) ||
-      // The nodes that can contain p and div are the same.  If the editing
-      // host is a <p> or similar, we have to just insert a newline.
-      (!mHTMLEditor->CanContainTag(*host, *nsGkAtoms::p) &&
-       // These can't contain <p> as a child, but can as a descendant, so we
-       // don't have to fall back to inserting a newline.
-       !host->IsAnyOfHTMLElements(nsGkAtoms::ol, nsGkAtoms::ul, nsGkAtoms::dl,
-                                  nsGkAtoms::table, nsGkAtoms::thead,
-                                  nsGkAtoms::tbody, nsGkAtoms::tfoot,
-                                  nsGkAtoms::tr)) ||
-      (host == blockParent && separator == ParagraphSeparator::br)) {
+
+  // Look for the nearest parent block.  However, don't return error even if
+  // there is no block parent here because in such case, i.e., editing host
+  // is an inline element, we should insert <br> simply.
+  RefPtr<Element> blockParent = HTMLEditor::GetBlock(node, host);
+
+  ParagraphSeparator separator = htmlEditor->GetDefaultParagraphSeparator();
+  bool insertBRElement;
+  // If there is no block parent in the editing host, i.e., the editing host
+  // itself is also a non-block element, we should insert a <br> element.
+  if (!blockParent) {
+    // XXX Chromium checks if the CSS box of the editing host is block.
+    insertBRElement = true;
+  }
+  // If only the editing host is block, and the default paragraph separator
+  // is <br> or the editing host cannot contain a <p> element, we should
+  // insert a <br> element.
+  else if (host == blockParent) {
+    insertBRElement =
+      separator == ParagraphSeparator::br || !CanContainParagraph(*host);
+  }
+  // If the nearest block parent is a single-line container declared in
+  // the execCommand spec and not the editing host, we should separate the
+  // block even if the default paragraph separator is <br> element.
+  else if (HTMLEditUtils::IsSingleLineContainer(*blockParent)) {
+    insertBRElement = false;
+  }
+  // Otherwise, unless there is no block ancestor which can contain <p>
+  // element, we shouldn't insert a <br> element here.
+  else {
+    insertBRElement = true;
+    for (Element* blockAncestor = blockParent;
+         blockAncestor && insertBRElement;
+         blockAncestor = HTMLEditor::GetBlockNodeParent(blockAncestor, host)) {
+      insertBRElement = !CanContainParagraph(*blockAncestor);
+    }
+  }
+
+  // If we cannot insert a <p>/<div> element at the selection, we should insert
+  // a <br> element instead.
+  if (insertBRElement) {
     nsresult rv = StandardBreakImpl(node, offset, aSelection);
     NS_ENSURE_SUCCESS(rv, rv);
     *aHandled = true;
     return NS_OK;
   }
+
   if (host == blockParent && separator != ParagraphSeparator::br) {
     // Insert a new block first
     MOZ_ASSERT(separator == ParagraphSeparator::div ||
                separator == ParagraphSeparator::p);
     nsresult rv = MakeBasicBlock(aSelection,
                                  ParagraphSeparatorElement(separator));
     // We warn on failure, but don't handle it, because it might be harmless.
     // Instead we just check that a new block was actually created.
-    Unused << NS_WARN_IF(NS_FAILED(rv));
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+                         "HTMLEditRules::MakeBasicBlock() failed");
 
     // Reinitialize node/offset in case they're not inside the new block
     if (NS_WARN_IF(!aSelection.GetRangeAt(0) ||
                    !aSelection.GetRangeAt(0)->GetStartContainer())) {
       return NS_ERROR_FAILURE;
     }
     node = *aSelection.GetRangeAt(0)->GetStartContainer();
     child = aSelection.GetRangeAt(0)->GetChildAtStartOffset();
     offset = aSelection.GetRangeAt(0)->StartOffset();
 
-    blockParent = mHTMLEditor->GetBlock(node);
+    blockParent = mHTMLEditor->GetBlock(node, host);
     if (NS_WARN_IF(!blockParent)) {
       return NS_ERROR_UNEXPECTED;
     }
     if (NS_WARN_IF(blockParent == host)) {
       // Didn't create a new block for some reason, fall back to <br>
       rv = StandardBreakImpl(node, offset, aSelection);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -168,16 +168,22 @@ protected:
                                nsIEditor::EStripWrappers aStripWrappers,
                                bool* aCancel, bool* aHandled);
   nsresult DidDeleteSelection(Selection* aSelection,
                               nsIEditor::EDirection aDir,
                               nsresult aResult);
   nsresult InsertBRIfNeeded(Selection* aSelection);
 
   /**
+   * CanContainParagraph() returns true if aElement can have a <p> element as
+   * its child or its descendant.
+   */
+  bool CanContainParagraph(Element& aElement) const;
+
+  /**
    * Insert a normal <br> element or a moz-<br> element to aNode when
    * aNode is a block and it has no children.
    *
    * @param aNode           Reference to a block parent.
    * @param aInsertMozBR    true if this should insert a moz-<br> element.
    *                        Otherwise, i.e., this should insert a normal <br>
    *                        element, false.
    */
--- a/editor/libeditor/HTMLEditUtils.cpp
+++ b/editor/libeditor/HTMLEditUtils.cpp
@@ -829,9 +829,35 @@ bool
 HTMLEditUtils::IsContainer(int32_t aTag)
 {
   NS_ASSERTION(aTag > eHTMLTag_unknown && aTag <= eHTMLTag_userdefined,
                "aTag out of range!");
 
   return kElements[aTag - 1].mIsContainer;
 }
 
+bool
+HTMLEditUtils::IsNonListSingleLineContainer(nsINode& aNode)
+{
+  return aNode.IsAnyOfHTMLElements(nsGkAtoms::address,
+                                   nsGkAtoms::div,
+                                   nsGkAtoms::h1,
+                                   nsGkAtoms::h2,
+                                   nsGkAtoms::h3,
+                                   nsGkAtoms::h4,
+                                   nsGkAtoms::h5,
+                                   nsGkAtoms::h6,
+                                   nsGkAtoms::listing,
+                                   nsGkAtoms::p,
+                                   nsGkAtoms::pre,
+                                   nsGkAtoms::xmp);
+}
+
+bool
+HTMLEditUtils::IsSingleLineContainer(nsINode& aNode)
+{
+  return IsNonListSingleLineContainer(aNode) ||
+         aNode.IsAnyOfHTMLElements(nsGkAtoms::li,
+                                   nsGkAtoms::dt,
+                                   nsGkAtoms::dd);
+}
+
 } // namespace mozilla
--- a/editor/libeditor/HTMLEditUtils.h
+++ b/editor/libeditor/HTMLEditUtils.h
@@ -11,17 +11,16 @@
 class nsIDOMNode;
 class nsINode;
 
 namespace mozilla {
 
 class HTMLEditUtils final
 {
 public:
-  // from nsHTMLEditRules:
   static bool IsInlineStyle(nsINode* aNode);
   static bool IsInlineStyle(nsIDOMNode *aNode);
   static bool IsFormatNode(nsINode* aNode);
   static bool IsFormatNode(nsIDOMNode* aNode);
   static bool IsNodeThatCanOutdent(nsIDOMNode* aNode);
   static bool IsHeader(nsINode& aNode);
   static bool IsHeader(nsIDOMNode* aNode);
   static bool IsParagraph(nsIDOMNode* aNode);
@@ -55,14 +54,22 @@ public:
   static bool IsMozDiv(nsIDOMNode* aNode);
   static bool IsMailCite(nsINode* aNode);
   static bool IsMailCite(nsIDOMNode* aNode);
   static bool IsFormWidget(nsINode* aNode);
   static bool IsFormWidget(nsIDOMNode* aNode);
   static bool SupportsAlignAttr(nsINode& aNode);
   static bool CanContain(int32_t aParent, int32_t aChild);
   static bool IsContainer(int32_t aTag);
+
+  /**
+   * See execCommand spec:
+   * https://w3c.github.io/editing/execCommand.html#non-list-single-line-container
+   * https://w3c.github.io/editing/execCommand.html#single-line-container
+   */
+  static bool IsNonListSingleLineContainer(nsINode& aNode);
+  static bool IsSingleLineContainer(nsINode& aNode);
 };
 
 } // namespace mozilla
 
 #endif // #ifndef HTMLEditUtils_h
 
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -3466,17 +3466,17 @@ HTMLEditor::EndOperation()
   // post processing
   nsresult rv = rules ? rules->AfterEdit(mAction, mDirection) : NS_OK;
   EditorBase::EndOperation();  // will clear mAction, mDirection
   return rv;
 }
 
 bool
 HTMLEditor::TagCanContainTag(nsAtom& aParentTag,
-                             nsAtom& aChildTag)
+                             nsAtom& aChildTag) const
 {
   int32_t childTagEnum;
   // XXX Should this handle #cdata-section too?
   if (&aChildTag == nsGkAtoms::textTagName) {
     childTagEnum = eHTMLTag_text;
   } else {
     childTagEnum = nsHTMLTags::AtomTagToId(&aChildTag);
   }
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -289,17 +289,17 @@ public:
    * with a call to EndOperation.
    */
   NS_IMETHOD EndOperation() override;
 
   /**
    * returns true if aParentTag can contain a child of type aChildTag.
    */
   virtual bool TagCanContainTag(nsAtom& aParentTag,
-                                nsAtom& aChildTag) override;
+                                nsAtom& aChildTag) const override;
 
   /**
    * Returns true if aNode is a container.
    */
   virtual bool IsContainer(nsINode* aNode) override;
   virtual bool IsContainer(nsIDOMNode* aNode) override;
 
   /**