Bug 1422234 - Part 1: HTMLEditRules::ReturnInParagraph() should adjust split point if caret position is positioned at edge of anchor element. r=m_kato, a=lizzard
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 23 Jan 2018 11:14:37 +0900
changeset 454633 1d6350b5091323b87ba8ae42b440ae8c99fa7a56
parent 454632 e82e444a853a8f3ea5315614ed33b90764e0edc4
child 454634 0fe1f0b63facaac51367d071018d8c22612bbb80
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, lizzard
bugs1422234
milestone59.0
Bug 1422234 - Part 1: HTMLEditRules::ReturnInParagraph() should adjust split point if caret position is positioned at edge of anchor element. r=m_kato, a=lizzard When Enter key is pressed at start or end of <a href="foo"> element, we shouldn't split it (in other words, we shouldn't create empty <a href="foo"> element in new paragraph) because users must not want to keep editing *same* link in new paragraph in most cases. This patch adjusts HTMLEditRules::ReturnInParagraph() selection start point locally when it gets selection. If caret is at start of an <a href="foo"> element, moves caret to before the element. If caret is at end of an <a href="foo"> element, moves to after the element. MozReview-Commit-ID: 3L3eDzc6Dk
editor/libeditor/EditorDOMPoint.h
editor/libeditor/HTMLEditRules.cpp
testing/web-platform/meta/editing/run/insertparagraph.html.ini
--- a/editor/libeditor/EditorDOMPoint.h
+++ b/editor/libeditor/EditorDOMPoint.h
@@ -9,16 +9,17 @@
 
 #include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/RangeBoundary.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/Text.h"
 #include "nsAtom.h"
 #include "nsCOMPtr.h"
+#include "nsGkAtoms.h"
 #include "nsIContent.h"
 #include "nsIDOMNode.h"
 #include "nsINode.h"
 
 namespace mozilla {
 
 template<typename ParentType, typename ChildType>
 class EditorDOMPointBase;
@@ -395,16 +396,36 @@ public:
     MOZ_ASSERT(aContainer);
     mParent = const_cast<nsINode*>(aContainer);
     mChild = nullptr;
     mOffset = mozilla::Some(mParent->Length());
     mIsChildInitialized = true;
   }
 
   /**
+   * SetAfter() sets mChild to next sibling of aChild.
+   */
+  void
+  SetAfter(const nsINode* aChild)
+  {
+    MOZ_ASSERT(aChild);
+    nsIContent* nextSibling = aChild->GetNextSibling();
+    if (nextSibling) {
+      Set(nextSibling);
+      return;
+    }
+    nsINode* parentNode = aChild->GetParentNode();
+    if (NS_WARN_IF(!parentNode)) {
+      Clear();
+      return;
+    }
+    SetToEndOf(parentNode);
+  }
+
+  /**
    * Clear() makes the instance not point anywhere.
    */
   void
   Clear()
   {
     mParent = nullptr;
     mChild = nullptr;
     mOffset.reset();
@@ -581,16 +602,33 @@ public:
                            mParent->GetChildAt_Deprecated(mOffset.value()) == mChild,
         "If mOffset and mChild are mismatched");
       return false;
     }
     MOZ_ASSERT(mOffset.isSome());
     return mOffset.value() == mParent->Length();
   }
 
+  bool
+  IsBRElementAtEndOfContainer() const
+  {
+    if (NS_WARN_IF(!mParent)) {
+      return false;
+    }
+    if (!mParent->IsContainerNode()) {
+      return false;
+    }
+    const_cast<SelfType*>(this)->EnsureChild();
+    if (!mChild ||
+        mChild->GetNextSibling()) {
+      return false;
+    }
+    return mChild->IsHTMLElement(nsGkAtoms::br);
+  }
+
   // Convenience methods for switching between the two types
   // of EditorDOMPointBase.
   EditorDOMPointBase<nsINode*, nsIContent*>
   AsRaw() const
   {
     return EditorRawDOMPoint(*this);
   }
 
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -6889,16 +6889,81 @@ HTMLEditRules::ReturnInParagraph(Selecti
   }
 
   EditorDOMPoint atStartOfSelection(firstRange->StartRef());
   if (NS_WARN_IF(!atStartOfSelection.IsSet())) {
     return EditActionResult(NS_ERROR_FAILURE);
   }
   MOZ_ASSERT(atStartOfSelection.IsSetAndValid());
 
+  // We shouldn't create new anchor element which has non-empty href unless
+  // splitting middle of it because we assume that users don't want to create
+  // *same* anchor element across two or more paragraphs in most cases.
+  // So, adjust selection start if it's edge of anchor element(s).
+  // XXX We don't support whitespace collapsing in these cases since it needs
+  //     some additional work with WSRunObject but it's not usual case.
+  //     E.g., |<a href="foo"><b>foo []</b> </a>|
+  if (atStartOfSelection.IsStartOfContainer()) {
+    for (nsIContent* container = atStartOfSelection.GetContainerAsContent();
+         container && container != &aParentDivOrP;
+         container = container->GetParent()) {
+      if (HTMLEditUtils::IsLink(container)) {
+        // Found link should be only in right node.  So, we shouldn't split it.
+        atStartOfSelection.Set(container);
+        // Even if we found an anchor element, don't break because DOM API
+        // allows to nest anchor elements.
+      }
+      // If the container is middle of its parent, stop adjusting split point.
+      if (container->GetPreviousSibling()) {
+        // XXX Should we check if previous sibling is visible content?
+        //     E.g., should we ignore comment node, invisible <br> element?
+        break;
+      }
+    }
+  }
+  // We also need to check if selection is at invisible <br> element at end
+  // of an <a href="foo"> element because editor inserts a <br> element when
+  // user types Enter key after a whitespace which is at middle of
+  // <a href="foo"> element and when setting selection at end of the element,
+  // selection becomes referring the <br> element.  We may need to change this
+  // behavior later if it'd be standardized.
+  else if (atStartOfSelection.IsEndOfContainer() ||
+           atStartOfSelection.IsBRElementAtEndOfContainer()) {
+    // If there are 2 <br> elements, the first <br> element is visible.  E.g.,
+    // |<a href="foo"><b>boo[]<br></b><br></a>|, we should split the <a>
+    // element.  Otherwise, E.g., |<a href="foo"><b>boo[]<br></b></a>|,
+    // we should not split the <a> element and ignore inline elements in it.
+    bool foundBRElement = atStartOfSelection.IsBRElementAtEndOfContainer();
+    for (nsIContent* container = atStartOfSelection.GetContainerAsContent();
+         container && container != &aParentDivOrP;
+         container = container->GetParent()) {
+      if (HTMLEditUtils::IsLink(container)) {
+        // Found link should be only in left node.  So, we shouldn't split it.
+        atStartOfSelection.SetAfter(container);
+        // Even if we found an anchor element, don't break because DOM API
+        // allows to nest anchor elements.
+      }
+      // If the container is middle of its parent, stop adjusting split point.
+      if (nsIContent* nextSibling = container->GetNextSibling()) {
+        if (foundBRElement) {
+          // If we've already found a <br> element, we assume found node is
+          // visible <br> or something other node.
+          // XXX Should we check if non-text data node like comment?
+          break;
+        }
+
+        // XXX Should we check if non-text data node like comment?
+        if (!nextSibling->IsHTMLElement(nsGkAtoms::br)) {
+          break;
+        }
+        foundBRElement = true;
+      }
+    }
+  }
+
   bool doesCRCreateNewP = htmlEditor->GetReturnInParagraphCreatesNewParagraph();
 
   bool splitAfterNewBR = false;
   nsCOMPtr<nsIContent> brNode;
 
   EditorDOMPoint pointToSplitParentDivOrP(atStartOfSelection);
 
   EditorRawDOMPoint pointToInsertBR;
--- a/testing/web-platform/meta/editing/run/insertparagraph.html.ini
+++ b/testing/web-platform/meta/editing/run/insertparagraph.html.ini
@@ -115,22 +115,16 @@
     expected: FAIL
 
   [[["stylewithcss","true"\],["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<p><b id=x class=y>foo[\]bar</b></p>" compare innerHTML]
     expected: FAIL
 
   [[["stylewithcss","false"\],["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<p><b id=x class=y>foo[\]bar</b></p>" compare innerHTML]
     expected: FAIL
 
-  [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "foo<a href=foo>[\]bar</a>" compare innerHTML]
-    expected: FAIL
-
-  [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "foo<a href=foo>[\]bar</a>" compare innerHTML]
-    expected: FAIL
-
   [[["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<p>foo[\]<!--bar-->" compare innerHTML]
     expected: FAIL
 
   [[["defaultparagraphseparator","p"\],["insertparagraph",""\]\] "<p>foo[\]<!--bar-->" compare innerHTML]
     expected: FAIL
 
   [[["stylewithcss","true"\],["defaultparagraphseparator","div"\],["insertparagraph",""\]\] "<p>[foo<span style=color:#aBcDeF>bar\]</span>baz" compare innerHTML]
     expected: FAIL