Change TypingCommand::DeleteKeyPressed() to handle anonymous placeholder

This patch changes |TypingCommand::DeleteKeyPressed()| to handle
anonymous placeholder instead of |ComputeAdjustedSelection()| to limit
scope of special handling for improving code health.

* Anonymous placeholder is a <br> in anonymous block having only it.
* Placeholder is represented by <div><br></div>.

Example of special handling:
<div contenteditable><img style="display:block"><br></div>

Before this patch, |ComputeAdjustedSelection()| returns range selection:
After:<img>, Before:<br> even if both positions specify equivalent
position <div>@1. After this patch |ComputeAdjustedSelection()| returns
caret selection Before:<br>.

Note: |TypingCommand::DeleteKeyPressed()| basically does extend
selection backward in character then applies |DeleteSelectionCommand|.

Bug: 1143292
Change-Id: I33879d0fa8598b5c26b41ecbdaf134029b240cf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2564140
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832082}

third_party/blink/renderer/core/editing/commands/typing_command.cc

@@ -160,6 +160,25 @@ bool CanAppendNewLineFeedToSelection(const VisibleSelection& selection,
   return false;
 }
 
+// Example: <div><img style="display:block">|<br></p>
+// See "editing/deleting/delete_after_block_image.html"
+Position AfterBlockIfBeforeAnonymousPlaceholder(const Position& position) {
+  if (!position.IsBeforeAnchor())
+    return Position();
+  const LayoutObject* const layout_object =
+      position.AnchorNode()->GetLayoutObject();
+  if (!layout_object || !layout_object->IsBR() ||
+      layout_object->NextSibling() || layout_object->PreviousSibling())
+    return Position();
+  const LayoutObject* const parent = layout_object->Parent();
+  if (!parent || !parent->IsAnonymous())
+    return Position();
+  const LayoutObject* const previous = parent->PreviousSibling();
+  if (!previous || !previous->NonPseudoNode())
+    return Position();
+  return Position::AfterNode(*previous->NonPseudoNode());
+}
+
 }  // anonymous namespace
 
 TypingCommand::TypingCommand(Document& document,
@@ -763,19 +782,37 @@ bool TypingCommand::MakeEditableRootEmpty(EditingState* editing_state) {
 
 // If there are multiple Unicode code points to be deleted, adjust the
 // range to match platform conventions.
-static VisibleSelection AdjustSelectionForBackwardDelete(
-    const VisibleSelection& selection) {
-  if (selection.End().ComputeContainerNode() !=
-      selection.Start().ComputeContainerNode())
-    return selection;
-  if (selection.End().ComputeOffsetInContainerNode() -
-          selection.Start().ComputeOffsetInContainerNode() <=
+static SelectionForUndoStep AdjustSelectionForBackwardDelete(
+    const SelectionInDOMTree& selection) {
+  const Position& base = selection.Base();
+  if (selection.IsCaret()) {
+    // TODO(yosin): We should make |DeleteSelectionCommand| to work with
+    // anonymous placeholder.
+    if (Position after_block = AfterBlockIfBeforeAnonymousPlaceholder(base)) {
+      // We remove a anonymous placeholder <br> in <div> like <div><br></div>:
+      //   <div><img style="display:block"><br></div>
+      //   |selection_to_delete| is Before:<br>
+      // as
+      //   <div><img style="display:block"><div><br></div></div>.
+      //   |selection_to_delete| is <div>@0, After:<img>
+      // See "editing/deleting/delete_after_block_image.html"
+      return SelectionForUndoStep::Builder()
+          .SetBaseAndExtentAsBackwardSelection(base, after_block)
+          .Build();
+    }
+    return SelectionForUndoStep::From(selection);
+  }
+  if (base.ComputeContainerNode() != selection.Extent().ComputeContainerNode())
+    return SelectionForUndoStep::From(selection);
+  if (base.ComputeOffsetInContainerNode() -
+          selection.Extent().ComputeOffsetInContainerNode() <=
       1)
-    return selection;
-  return VisibleSelection::CreateWithoutValidationDeprecated(
-      selection.End(),
-      PreviousPositionOf(selection.End(), PositionMoveType::kBackwardDeletion),
-      selection.Affinity());
+    return SelectionForUndoStep::From(selection);
+  const Position& end = selection.ComputeEndPosition();
+  return SelectionForUndoStep::Builder()
+      .SetBaseAndExtentAsBackwardSelection(
+          end, PreviousPositionOf(end, PositionMoveType::kBackwardDeletion))
+      .Build();
 }
 
 void TypingCommand::DeleteKeyPressed(TextGranularity granularity,
@@ -887,17 +924,17 @@ void TypingCommand::DeleteKeyPressed(TextGranularity granularity,
     return;
   }
 
-  const VisibleSelection& selection_to_delete =
+  const SelectionForUndoStep& selection_to_delete =
       granularity == TextGranularity::kCharacter
-          ? AdjustSelectionForBackwardDelete(selection_modifier.Selection())
-          : selection_modifier.Selection();
+          ? AdjustSelectionForBackwardDelete(
+                selection_modifier.Selection().AsSelection())
+          : SelectionForUndoStep::From(
+                selection_modifier.Selection().AsSelection());
 
   if (!StartingSelection().IsRange() ||
       selection_to_delete.Base() != StartingSelection().Start()) {
-    DeleteKeyPressedInternal(
-        SelectionForUndoStep::From(selection_to_delete.AsSelection()),
-        SelectionForUndoStep::From(selection_to_delete.AsSelection()),
-        kill_ring, editing_state);
+    DeleteKeyPressedInternal(selection_to_delete, selection_to_delete,
+                             kill_ring, editing_state);
     return;
   }
   // Note: |StartingSelection().End()| can be disconnected.
@@ -909,9 +946,8 @@ void TypingCommand::DeleteKeyPressed(TextGranularity granularity,
               CreateVisiblePosition(selection_to_delete.Extent())
                   .DeepEquivalent())
           .Build();
-  DeleteKeyPressedInternal(
-      SelectionForUndoStep::From(selection_to_delete.AsSelection()),
-      selection_after_undo, kill_ring, editing_state);
+  DeleteKeyPressedInternal(selection_to_delete, selection_after_undo, kill_ring,
+                           editing_state);
 }
 
 void TypingCommand::DeleteKeyPressedInternal(

third_party/blink/renderer/core/editing/selection_adjuster.cc

@@ -43,11 +43,6 @@ template <typename Strategy>
 SelectionTemplate<Strategy> ComputeAdjustedSelection(
     const SelectionTemplate<Strategy> selection,
     const EphemeralRangeTemplate<Strategy>& range) {
-  if (selection.ComputeRange() == range) {
-    // To pass "editing/deleting/delete_after_block_image.html", we need to
-    // return original selection.
-    return selection;
-  }
   if (range.StartPosition().CompareTo(range.EndPosition()) == 0) {
     return typename SelectionTemplate<Strategy>::Builder()
         .Collapse(selection.IsBaseFirst() ? range.StartPosition()

third_party/blink/renderer/core/editing/visible_selection_test.cc

@@ -95,6 +95,24 @@ VisibleSelectionTemplate<Strategy> ExpandUsingGranularity(
       granularity);
 }
 
+// For "editing/deleting/delete_after_block_image.html"
+TEST_F(VisibleSelectionTest, AnonymousPlaceholder) {
+  InsertStyleElement("img { display:block; width: 10px; height: 10px;");
+  SetBodyContent("<div><img id=i><br id=b></div>");
+  Element& img = *GetElementById("i");
+  Element& br = *GetElementById("b");
+
+  // Note: After:<img>, Before:<br>, DIV@1 are equivalent.
+  const VisibleSelection& selection =
+      CreateVisibleSelection(SelectionInDOMTree::Builder()
+                                 .Collapse(Position::BeforeNode(br))
+                                 .Extend(Position::AfterNode(img))
+                                 .Build());
+  EXPECT_TRUE(selection.IsCaret());
+  EXPECT_EQ(Position::BeforeNode(br), selection.Base());
+  EXPECT_EQ(Position::BeforeNode(br), selection.Extent());
+}
+
 TEST_F(VisibleSelectionTest, expandUsingGranularity) {
   const char* body_content =
       "<span id=host><a id=one>1</a><a id=two>22</a></span>";