Bug 1616257 - part 1: Redesign `WSRunScanner::GetWSBoundingParent()` r=m_kato

Its result has 4 meanings:

1. an editable block element for container of `mScanStartPoint`.
2. a topmost inline editable content for container of `mScanStartPoint` if there
is no editable block parent.
3. container of `mScanStartPoint` if it's a block (either editable or
non-editable).
4. container of `mScanStartPoint` if its parent is not editable and a inline
content.

#1, #2 and editable case of #3 make sense because the results are topmost
editable content in current context.  On the other hand, non-editable case
of #3 and #4 are caused by unexpected wrong fallback code.

So, let's make it always returns the content in the former meaning and if
the caller needs the latter one, they should use the container by themselves.
Therefore, for making what's the start of the search, this patch also makes
new method take start content instead of hiding `mScanStartPoint` from the
callers.

Differential Revision: https://phabricator.services.mozilla.com/D63610

--HG--
extra : moz-landing-system : lando

editor/libeditor/WSRunObject.cpp

@@ -654,22 +654,26 @@ nsresult WSRunObject::AdjustWhitespace() {
 //   protected methods
 //--------------------------------------------------------------------------------------------
 
-nsINode* WSRunScanner::GetWSBoundingParent() const {
-  if (NS_WARN_IF(!mScanStartPoint.IsSet())) {
+nsIContent* WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent(
+    nsIContent* aContent) const {
+  if (NS_WARN_IF(!aContent)) {
     return nullptr;
   }
+  NS_ASSERTION(mHTMLEditor->IsEditable(aContent),
+               "Given content is not editable");
   // XXX What should we do if scan range crosses block boundary?  Currently,
   //     it's not collapsed only when inserting composition string so that
   //     it's possible but shouldn't occur actually.
-  OwningNonNull<nsINode> wsBoundingParent = *mScanStartPoint.GetContainer();
-  while (!IsBlockNode(wsBoundingParent)) {
-    nsCOMPtr<nsINode> parent = wsBoundingParent->GetParentNode();
-    if (!parent || !mHTMLEditor->IsEditable(parent)) {
+  nsIContent* editableBlockParentOrTopmotEditableInlineContent = nullptr;
+  for (nsIContent* content = aContent;
+       content && mHTMLEditor->IsEditable(content);
+       content = content->GetParent()) {
+    editableBlockParentOrTopmotEditableInlineContent = content;
+    if (IsBlockNode(editableBlockParentOrTopmotEditableInlineContent)) {
       break;
     }
-    wsBoundingParent = parent;
   }
-  return wsBoundingParent;
+  return editableBlockParentOrTopmotEditableInlineContent;
 }
 
 nsresult WSRunScanner::GetWSNodes() {
@@ -677,7 +681,20 @@ nsresult WSRunScanner::GetWSNodes() {
   // and which contain only whitespace.  Stop if you reach non-ws text or a new
   // block boundary.
   EditorDOMPoint start(mScanStartPoint), end(mScanStartPoint);
-  nsCOMPtr<nsINode> wsBoundingParent = GetWSBoundingParent();
+  nsIContent* scanStartContent = mScanStartPoint.GetContainerAsContent();
+  if (NS_WARN_IF(!scanStartContent)) {
+    // Meaning container of mScanStartPoint is a Document or DocumentFragment.
+    // I.e., we're try to modify outside of root element.  We don't need to
+    // support such odd case because web apps cannot append text nodes as
+    // direct child of Document node.
+    return NS_ERROR_FAILURE;
+  }
+  nsIContent* editableBlockParentOrTopmotEditableInlineContent =
+      GetEditableBlockParentOrTopmotEditableInlineContent(scanStartContent);
+  if (NS_WARN_IF(!editableBlockParentOrTopmotEditableInlineContent)) {
+    // Meaning that the container of `mScanStartPoint` is not editable.
+    editableBlockParentOrTopmotEditableInlineContent = scanStartContent;
+  }
 
   // first look backwards to find preceding ws nodes
   if (Text* textNode = mScanStartPoint.GetContainerAsText()) {
@@ -715,7 +732,8 @@ nsresult WSRunScanner::GetWSNodes() {
 
   while (!mStartNode) {
     // we haven't found the start of ws yet.  Keep looking
-    nsCOMPtr<nsIContent> priorNode = GetPreviousWSNode(start, wsBoundingParent);
+    nsCOMPtr<nsIContent> priorNode = GetPreviousWSNode(
+        start, editableBlockParentOrTopmotEditableInlineContent);
     if (priorNode) {
       if (IsBlockNode(priorNode)) {
         mStartNode = start.GetContainer();
@@ -776,11 +794,14 @@ nsresult WSRunScanner::GetWSNodes() {
         mStartReasonNode = priorNode;
       }
     } else {
-      // no prior node means we exhausted wsBoundingParent
+      // no prior node means we exhausted
+      // editableBlockParentOrTopmotEditableInlineContent
       mStartNode = start.GetContainer();
       mStartOffset = start.Offset();
       mStartReason = WSType::thisBlock;
-      mStartReasonNode = wsBoundingParent;
+      // mStartReasonNode can be either a block element or any non-editable
+      // content in this case.
+      mStartReasonNode = editableBlockParentOrTopmotEditableInlineContent;
     }
   }
 
@@ -820,7 +841,8 @@ nsresult WSRunScanner::GetWSNodes() {
 
   while (!mEndNode) {
     // we haven't found the end of ws yet.  Keep looking
-    nsCOMPtr<nsIContent> nextNode = GetNextWSNode(end, wsBoundingParent);
+    nsCOMPtr<nsIContent> nextNode =
+        GetNextWSNode(end, editableBlockParentOrTopmotEditableInlineContent);
     if (nextNode) {
       if (IsBlockNode(nextNode)) {
         // we encountered a new block.  therefore no more ws.
@@ -883,11 +905,14 @@ nsresult WSRunScanner::GetWSNodes() {
         mEndReasonNode = nextNode;
       }
     } else {
-      // no next node means we exhausted wsBoundingParent
+      // no next node means we exhausted
+      // editableBlockParentOrTopmotEditableInlineContent
       mEndNode = end.GetContainer();
       mEndOffset = end.Offset();
       mEndReason = WSType::thisBlock;
-      mEndReasonNode = wsBoundingParent;
+      // mEndReasonNode can be either a block element or any non-editable
+      // content in this case.
+      mEndReasonNode = editableBlockParentOrTopmotEditableInlineContent;
     }
   }
 
@@ -1839,7 +1864,9 @@ nsresult WSRunObject::CheckTrailingNBSPOfRun(WSFragment* aRun) {
         rightCheck = true;
       }
       if ((aRun->mRightType & WSType::block) &&
-          IsBlockNode(GetWSBoundingParent())) {
+          (IsBlockNode(GetEditableBlockParentOrTopmotEditableInlineContent(
+               mScanStartPoint.GetContainerAsContent())) ||
+           IsBlockNode(mScanStartPoint.GetContainerAsContent()))) {
         RefPtr<Selection> selection = htmlEditor->GetSelection();
         if (NS_WARN_IF(!selection)) {
           return NS_ERROR_FAILURE;

editor/libeditor/WSRunObject.h

@@ -292,11 +292,11 @@ class MOZ_STACK_CLASS WSRunScanner {
   nsresult GetWSNodes();
 
   /**
-   * Return the node which we will handle white-space under. This is the
-   * closest block within the DOM subtree we're editing, or if none is
-   * found, the (inline) root of the editable subtree.
+   * Return a current block element for aContent or a topmost editable inline
+   * element if aContent is not in editable block element.
    */
-  nsINode* GetWSBoundingParent() const;
+  nsIContent* GetEditableBlockParentOrTopmotEditableInlineContent(
+      nsIContent* aContent) const;
 
   static bool IsBlockNode(nsINode* aNode);
 

editor/libeditor/crashtests/crashtests.list

@@ -80,7 +80,7 @@ load 1348851.html
 load 1350772.html
 load 1364133.html
 load 1366176.html
-load 1375131.html
+asserts(3) load 1375131.html # assertion in WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent()
 load 1381541.html
 load 1383747.html
 load 1383755.html
@@ -94,7 +94,7 @@ load 1402526.html
 pref(dom.document.exec_command.nested_calls_allowed,true) asserts(1) load 1402904.html # assertion is that mutation event listener caused by execCommand calls another execCommand
 pref(dom.document.exec_command.nested_calls_allowed,true) asserts(1) load 1405747.html # assertion is that mutation event listener caused by execCommand calls another execCommand
 load 1405897.html
-load 1408170.html
+asserts(3) load 1408170.html # assertion in WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent()
 asserts(0-1) load 1414581.html
 load 1415231.html
 load 1423767.html
@@ -111,7 +111,7 @@ pref(layout.accessiblecaret.enabled,true) load 1470926.html
 pref(dom.document.exec_command.nested_calls_allowed,true) asserts(1) load 1474978.html # assertion is that mutation event listener caused by execCommand calls another execCommand
 load 1525481.html
 load 1533913.html
-load 1534394.html
+asserts(4) load 1534394.html # assertion in WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent()
 load 1547897.html
 load 1547898.html
 load 1556799.html

editor/libeditor/tests/test_bug430392.html

@@ -110,6 +110,11 @@ function test() {
   SimpleTest.finish();
 }
 
+// In WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent().
+// Before it, HTMLEditor must be able to stop handling edit action at
+// non-editable content.
+SimpleTest.expectAssertions(18);
+
 SimpleTest.waitForExplicitFinish();
 SimpleTest.waitForFocus(test);
 

testing/web-platform/meta/editing/run/inserthtml.html.ini

@@ -1,4 +1,6 @@
 [inserthtml.html]
+  min-asserts: 14
+  max-asserts: 14
   prefs: [editor.use_div_for_default_newlines:true]
   [[["stylewithcss","true"\],["inserthtml","ab<b>c</b>d"\]\] "[foo<span style=color:#aBcDeF>bar\]</span>baz" compare innerHTML]
     expected: FAIL