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

UltraBlame original commit: f8505320d2165dd86f8fbb212ffe87397a814aaf

editor/libeditor/WSRunObject.cpp

@@ -654,30 +654,47 @@ nsresult WSRunObject::AdjustWhitespace() {
 
 
 
-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");
 
 
 
-  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() {
 
 
 
   EditorDOMPoint start(mScanStartPoint), end(mScanStartPoint);
-  nsCOMPtr<nsINode> wsBoundingParent = GetWSBoundingParent();
+  nsIContent* scanStartContent = mScanStartPoint.GetContainerAsContent();
+  if (NS_WARN_IF(!scanStartContent)) {
+
+
+
+
+    return NS_ERROR_FAILURE;
+  }
+  nsIContent* editableBlockParentOrTopmotEditableInlineContent =
+      GetEditableBlockParentOrTopmotEditableInlineContent(scanStartContent);
+  if (NS_WARN_IF(!editableBlockParentOrTopmotEditableInlineContent)) {
+
+    editableBlockParentOrTopmotEditableInlineContent = scanStartContent;
+  }
 
 
   if (Text* textNode = mScanStartPoint.GetContainerAsText()) {
@@ -715,7 +732,8 @@ nsresult WSRunScanner::GetWSNodes() {
 
   while (!mStartNode) {
 
-    nsCOMPtr<nsIContent> priorNode = GetPreviousWSNode(start, wsBoundingParent);
+    nsCOMPtr<nsIContent> priorNode = GetPreviousWSNode(
+        start, editableBlockParentOrTopmotEditableInlineContent);
     if (priorNode) {
       if (IsBlockNode(priorNode)) {
         mStartNode = start.GetContainer();
@@ -777,10 +795,13 @@ nsresult WSRunScanner::GetWSNodes() {
       }
     } else {
 
+
       mStartNode = start.GetContainer();
       mStartOffset = start.Offset();
       mStartReason = WSType::thisBlock;
-      mStartReasonNode = wsBoundingParent;
+
+
+      mStartReasonNode = editableBlockParentOrTopmotEditableInlineContent;
     }
   }
 
@@ -820,7 +841,8 @@ nsresult WSRunScanner::GetWSNodes() {
 
   while (!mEndNode) {
 
-    nsCOMPtr<nsIContent> nextNode = GetNextWSNode(end, wsBoundingParent);
+    nsCOMPtr<nsIContent> nextNode =
+        GetNextWSNode(end, editableBlockParentOrTopmotEditableInlineContent);
     if (nextNode) {
       if (IsBlockNode(nextNode)) {
 
@@ -884,10 +906,13 @@ nsresult WSRunScanner::GetWSNodes() {
       }
     } else {
 
+
       mEndNode = end.GetContainer();
       mEndOffset = end.Offset();
       mEndReason = WSType::thisBlock;
-      mEndReasonNode = wsBoundingParent;
+
+
+      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

@@ -295,8 +295,8 @@ class MOZ_STACK_CLASS WSRunScanner {
 
 
 
-
-  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 @@
   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