[editing] Normalize caret position in the flat tree

VisiblePosition::Create canonicalizes positions using
MostBackwardCaretPosition and MostForwardCaretPosition.

For positions based on the DOM tree, this happened in the DOM tree too,
so they might consider two positions to be visually the same, even
though in the flat tree there was some content between them.

To solve the problem, this patch makes them convert the Position into a
PositionInFlatTree, canonicalize in the flat tree, and then convert the
result back into a Position, but clamping to avoid crossing shadow
boundaries.

Bug: 1145595

TEST=EditingUtilitiesTest.firstEditablePositionAfterPositionInRoot
TEST=EditingUtilitiesTest.lastEditablePositionBeforePositionInRoot
TEST=EditingUtilitiesTest.NextVisuallyDistinctCandidate
TEST=VisiblePositionTest.ShadowV0DistributedNodes
TEST=VisibleUnitsLineTest.endOfLine
TEST=VisibleUnitsLineTest.isEndOfLine
TEST=VisibleUnitsLineTest.isLogicalEndOfLine
TEST=VisibleUnitsLineTest.isStartOfLine
TEST=VisibleUnitsLineTest.logicalEndOfLine
TEST=VisibleUnitsLineTest.logicalStartOfLine
TEST=VisibleUnitsLineTest.startOfLine
TEST=VisibleUnitsTest.characterAfter
TEST=VisibleUnitsTest.characterBefore
TEST=VisibleUnitsTest.mostForwardCaretPositionAfterAnchor
TEST=VisibleUnitsTest.nextPositionOf
TEST=VisibleUnitsTest.previousPositionOf
TEST=All/ParameterizedVisibleUnitsLineTest.inSameLine/*
TEST=All/ParameterizedVisibleUnitsWordTest.EndOfWordShadowDOM/*
TEST=All/ParameterizedVisibleUnitsWordTest.StartOfWordShadowDOM/*

TEST=third_party/blink/web_tests/editing/execCommand/insert-list-with-progress-crash.html
TEST=third_party/blink/web_tests/editing/selection/collapse/collapse_before_select.html

Change-Id: I793c07c7ffbecdaf6cc619c6043f90475c922fda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517686
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832706}

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

@@ -59,7 +59,7 @@ TEST_F(EditingUtilitiesTest, firstEditablePositionAfterPositionInRoot) {
 
   EXPECT_EQ(Position(one, 0),
             FirstEditablePositionAfterPositionInRoot(Position(one, 0), *host));
-  EXPECT_EQ(Position(one->firstChild(), 0),
+  EXPECT_EQ(Position(two->firstChild(), 2),
             CreateVisiblePosition(FirstEditablePositionAfterPositionInRoot(
                                       Position(one, 0), *host))
                 .DeepEquivalent());
@@ -75,7 +75,7 @@ TEST_F(EditingUtilitiesTest, firstEditablePositionAfterPositionInRoot) {
   EXPECT_EQ(
       Position::FirstPositionInNode(*host),
       FirstEditablePositionAfterPositionInRoot(Position(three, 0), *host));
-  EXPECT_EQ(Position(one->firstChild(), 0),
+  EXPECT_EQ(Position(two->firstChild(), 0),
             CreateVisiblePosition(FirstEditablePositionAfterPositionInRoot(
                                       Position(three, 0), *host))
                 .DeepEquivalent());
@@ -211,7 +211,7 @@ TEST_F(EditingUtilitiesTest, lastEditablePositionBeforePositionInRoot) {
 
   EXPECT_EQ(Position(one, 0),
             LastEditablePositionBeforePositionInRoot(Position(one, 0), *host));
-  EXPECT_EQ(Position(one->firstChild(), 0),
+  EXPECT_EQ(Position(two->firstChild(), 2),
             CreateVisiblePosition(LastEditablePositionBeforePositionInRoot(
                                       Position(one, 0), *host))
                 .DeepEquivalent());
@@ -227,7 +227,7 @@ TEST_F(EditingUtilitiesTest, lastEditablePositionBeforePositionInRoot) {
   EXPECT_EQ(
       Position::FirstPositionInNode(*host),
       LastEditablePositionBeforePositionInRoot(Position(three, 0), *host));
-  EXPECT_EQ(Position(one->firstChild(), 0),
+  EXPECT_EQ(Position(two->firstChild(), 0),
             CreateVisiblePosition(LastEditablePositionBeforePositionInRoot(
                                       Position(three, 0), *host))
                 .DeepEquivalent());
@@ -271,7 +271,7 @@ TEST_F(EditingUtilitiesTest, NextVisuallyDistinctCandidate) {
   Node* two = GetDocument().getElementById("two");
   Node* three = GetDocument().getElementById("three");
 
-  EXPECT_EQ(Position(two->firstChild(), 1),
+  EXPECT_EQ(Position(two->firstChild(), 0),
             NextVisuallyDistinctCandidate(Position(one, 1)));
   EXPECT_EQ(PositionInFlatTree(three->firstChild(), 1),
             NextVisuallyDistinctCandidate(PositionInFlatTree(one, 1)));

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

@@ -594,13 +594,14 @@ PositionInFlatTree ToPositionInFlatTree(const Position& pos) {
     }
     DCHECK(!anchor->IsElementNode() || anchor->CanParticipateInFlatTree());
     int offset = pos.ComputeOffsetInContainerNode();
+    if (!offset) {
+      Node* node = anchor->IsShadowRoot() ? anchor->OwnerShadowHost() : anchor;
+      return PositionInFlatTree::FirstPositionInNode(*node);
+    }
     Node* child = NodeTraversal::ChildAt(*anchor, offset);
     if (!child) {
-      if (anchor->IsShadowRoot()) {
-        return PositionInFlatTree::LastPositionInNode(
-            *anchor->OwnerShadowHost());
-      }
-      return PositionInFlatTree::LastPositionInNode(*anchor);
+      Node* node = anchor->IsShadowRoot() ? anchor->OwnerShadowHost() : anchor;
+      return PositionInFlatTree::LastPositionInNode(*node);
     }
     child->UpdateDistributionForFlatTreeTraversal();
     if (!child->CanParticipateInFlatTree()) {

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

@@ -180,7 +180,7 @@ TEST_F(PositionTest, ToPositionInFlatTreeWithNotDistributed) {
   SetBodyContent("<progress id=sample>foo</progress>");
   Element* sample = GetDocument().getElementById("sample");
 
-  EXPECT_EQ(PositionInFlatTree::LastPositionInNode(*sample),
+  EXPECT_EQ(PositionInFlatTree::FirstPositionInNode(*sample),
             ToPositionInFlatTree(Position(sample, 0)));
 }
 
@@ -222,7 +222,7 @@ TEST_F(PositionTest, ToPositionInFlatTreeWithEmptyShadowRoot) {
   ShadowRoot* shadow_root = SetShadowContent(shadow_content, "host");
   Element* host = GetDocument().getElementById("host");
 
-  EXPECT_EQ(PositionInFlatTree::LastPositionInNode(*host),
+  EXPECT_EQ(PositionInFlatTree::FirstPositionInNode(*host),
             ToPositionInFlatTree(Position(shadow_root, 0)));
 }
 

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

@@ -162,13 +162,13 @@ TEST_F(VisiblePositionTest, ShadowV0DistributedNodes) {
             SnapForward(Position(one, 0)).GetPosition());
   EXPECT_EQ(Position(one->firstChild(), 0),
             CreateVisiblePosition(Position(one, 0)).DeepEquivalent());
-  EXPECT_EQ(Position(one->firstChild(), 2),
+  EXPECT_EQ(Position(two->firstChild(), 0),
             CanonicalPositionOf(Position(two, 0)));
-  EXPECT_EQ(Position(one->firstChild(), 2),
+  EXPECT_EQ(Position(two->firstChild(), 0),
             SnapBackward(Position(two, 0)).GetPosition());
   EXPECT_EQ(Position(two->firstChild(), 0),
             SnapForward(Position(two, 0)).GetPosition());
-  EXPECT_EQ(Position(one->firstChild(), 2),
+  EXPECT_EQ(Position(two->firstChild(), 0),
             CreateVisiblePosition(Position(two, 0)).DeepEquivalent());
 
   EXPECT_EQ(PositionInFlatTree(five->firstChild(), 2),

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

@@ -40,6 +40,8 @@
 #include "third_party/blink/renderer/core/editing/position.h"
 #include "third_party/blink/renderer/core/editing/position_iterator.h"
 #include "third_party/blink/renderer/core/editing/position_with_affinity.h"
+#include "third_party/blink/renderer/core/editing/selection_adjuster.h"
+#include "third_party/blink/renderer/core/editing/selection_template.h"
 #include "third_party/blink/renderer/core/editing/text_affinity.h"
 #include "third_party/blink/renderer/core/editing/visible_position.h"
 #include "third_party/blink/renderer/core/editing/visible_selection.h"
@@ -614,6 +616,44 @@ static bool IsStreamer(const PositionIteratorAlgorithm<Strategy>& pos) {
   return pos.AtStartOfNode();
 }
 
+template <typename F>
+static Position MostBackwardOrForwardCaretPosition(
+    const Position& position,
+    EditingBoundaryCrossingRule rule,
+    F AlgorithmInFlatTree) {
+  if (position.IsNull())
+    return Position();
+  DCHECK(position.IsValidFor(*position.GetDocument()));
+
+  // Find the most backward or forward caret position in the flat tree.
+  const Position& candidate = ToPositionInDOMTree(
+      AlgorithmInFlatTree(ToPositionInFlatTree(position), rule));
+  if (candidate.IsNull())
+    return candidate;
+
+  // Adjust the candidate to avoid crossing shadow boundaries.
+  const SelectionInDOMTree& selection =
+      SelectionInDOMTree::Builder()
+          .SetBaseAndExtent(position, candidate)
+          .Build();
+  if (selection.IsCaret())
+    return candidate;
+  const SelectionInDOMTree& shadow_adjusted_selection =
+      SelectionAdjuster::AdjustSelectionToAvoidCrossingShadowBoundaries(
+          selection);
+
+  // If we have to adjust the position, the editability may change, so avoid
+  // crossing editing boundaries if it's not allowed.
+  if (rule == kCannotCrossEditingBoundary &&
+      selection != shadow_adjusted_selection) {
+    const SelectionInDOMTree& editing_adjusted_selection =
+        SelectionAdjuster::AdjustSelectionToAvoidCrossingEditingBoundaries(
+            shadow_adjusted_selection);
+    return editing_adjusted_selection.Extent();
+  }
+  return shadow_adjusted_selection.Extent();
+}
+
 template <typename Strategy>
 static PositionTemplate<Strategy> AdjustPositionForBackwardIteration(
     const PositionTemplate<Strategy>& position) {
@@ -747,7 +787,8 @@ static PositionTemplate<Strategy> MostBackwardCaretPosition(
 
 Position MostBackwardCaretPosition(const Position& position,
                                    EditingBoundaryCrossingRule rule) {
-  return MostBackwardCaretPosition<EditingStrategy>(position, rule);
+  return MostBackwardOrForwardCaretPosition(
+      position, rule, MostBackwardCaretPosition<EditingInFlatTreeStrategy>);
 }
 
 PositionInFlatTree MostBackwardCaretPosition(const PositionInFlatTree& position,
@@ -885,7 +926,8 @@ PositionTemplate<Strategy> MostForwardCaretPosition(
 
 Position MostForwardCaretPosition(const Position& position,
                                   EditingBoundaryCrossingRule rule) {
-  return MostForwardCaretPosition<EditingStrategy>(position, rule);
+  return MostBackwardOrForwardCaretPosition(
+      position, rule, MostForwardCaretPosition<EditingInFlatTreeStrategy>);
 }
 
 PositionInFlatTree MostForwardCaretPosition(const PositionInFlatTree& position,

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

@@ -80,13 +80,13 @@ TEST_F(VisibleUnitsTest, characterAfter) {
   Element* one = GetDocument().getElementById("one");
   Element* two = GetDocument().getElementById("two");
 
-  EXPECT_EQ('2', CharacterAfter(
-                     CreateVisiblePositionInDOMTree(*one->firstChild(), 1)));
+  EXPECT_EQ(
+      0, CharacterAfter(CreateVisiblePositionInDOMTree(*one->firstChild(), 1)));
   EXPECT_EQ('5', CharacterAfter(
                      CreateVisiblePositionInFlatTree(*one->firstChild(), 1)));
 
-  EXPECT_EQ(
-      0, CharacterAfter(CreateVisiblePositionInDOMTree(*two->firstChild(), 2)));
+  EXPECT_EQ('1', CharacterAfter(
+                     CreateVisiblePositionInDOMTree(*two->firstChild(), 2)));
   EXPECT_EQ('1', CharacterAfter(
                      CreateVisiblePositionInFlatTree(*two->firstChild(), 2)));
 }
@@ -209,16 +209,16 @@ TEST_F(VisibleUnitsTest, characterBefore) {
   Node* two = GetDocument().getElementById("two")->firstChild();
   Node* five = shadow_root->getElementById("five")->firstChild();
 
-  EXPECT_EQ(0, CharacterBefore(CreateVisiblePositionInDOMTree(*one, 0)));
+  EXPECT_EQ('2', CharacterBefore(CreateVisiblePositionInDOMTree(*one, 0)));
   EXPECT_EQ('2', CharacterBefore(CreateVisiblePositionInFlatTree(*one, 0)));
 
   EXPECT_EQ('1', CharacterBefore(CreateVisiblePositionInDOMTree(*one, 1)));
   EXPECT_EQ('1', CharacterBefore(CreateVisiblePositionInFlatTree(*one, 1)));
 
-  EXPECT_EQ('1', CharacterBefore(CreateVisiblePositionInDOMTree(*two, 0)));
+  EXPECT_EQ(0, CharacterBefore(CreateVisiblePositionInDOMTree(*two, 0)));
   EXPECT_EQ('4', CharacterBefore(CreateVisiblePositionInFlatTree(*two, 0)));
 
-  EXPECT_EQ('4', CharacterBefore(CreateVisiblePositionInDOMTree(*five, 0)));
+  EXPECT_EQ(0, CharacterBefore(CreateVisiblePositionInDOMTree(*five, 0)));
   EXPECT_EQ('1', CharacterBefore(CreateVisiblePositionInFlatTree(*five, 0)));
 }
 
@@ -496,10 +496,9 @@ TEST_F(VisibleUnitsTest, mostForwardCaretPositionAfterAnchor) {
   UpdateAllLifecyclePhasesForTest();
 
   Element* host = GetDocument().getElementById("host");
-  Element* one = GetDocument().getElementById("one");
   Element* three = shadow_root->getElementById("three");
 
-  EXPECT_EQ(Position(one->firstChild(), 1),
+  EXPECT_EQ(Position::AfterNode(*host),
             MostBackwardCaretPosition(Position::AfterNode(*host)));
   EXPECT_EQ(PositionInFlatTree(three->firstChild(), 3),
             MostBackwardCaretPosition(PositionInFlatTree::AfterNode(*host)));
@@ -549,23 +548,25 @@ TEST_F(VisibleUnitsTest, nextPositionOf) {
   Element* four = shadow_root->getElementById("four");
   Element* five = shadow_root->getElementById("five");
 
-  EXPECT_EQ(Position(one->firstChild(), 0),
+  EXPECT_EQ(Position(two->firstChild(), 2),
             NextPositionOf(CreateVisiblePosition(Position(zero, 1)))
                 .DeepEquivalent());
   EXPECT_EQ(PositionInFlatTree(four->firstChild(), 0),
             NextPositionOf(CreateVisiblePosition(PositionInFlatTree(zero, 1)))
                 .DeepEquivalent());
 
-  EXPECT_EQ(
-      Position(one->firstChild(), 1),
-      NextPositionOf(CreateVisiblePosition(Position(one, 0))).DeepEquivalent());
+  EXPECT_EQ(Position(three->firstChild(), 0),
+            NextPositionOf(CreateVisiblePosition(Position(one, 0),
+                                                 TextAffinity::kUpstream))
+                .DeepEquivalent());
   EXPECT_EQ(PositionInFlatTree(one->firstChild(), 1),
             NextPositionOf(CreateVisiblePosition(PositionInFlatTree(one, 0)))
                 .DeepEquivalent());
 
-  EXPECT_EQ(
-      Position(two->firstChild(), 1),
-      NextPositionOf(CreateVisiblePosition(Position(one, 1))).DeepEquivalent());
+  EXPECT_EQ(Position(two->firstChild(), 0),
+            NextPositionOf(CreateVisiblePosition(Position(one, 1),
+                                                 TextAffinity::kUpstream))
+                .DeepEquivalent());
   EXPECT_EQ(PositionInFlatTree(five->firstChild(), 1),
             NextPositionOf(CreateVisiblePosition(PositionInFlatTree(one, 1)))
                 .DeepEquivalent());
@@ -620,7 +621,7 @@ TEST_F(VisibleUnitsTest, previousPositionOf) {
       PreviousPositionOf(CreateVisiblePosition(PositionInFlatTree(zero, 1)))
           .DeepEquivalent());
 
-  EXPECT_EQ(Position(zero, 1),
+  EXPECT_EQ(Position(two, 1),
             PreviousPositionOf(CreateVisiblePosition(Position(one, 0)))
                 .DeepEquivalent());
   EXPECT_EQ(
@@ -628,7 +629,7 @@ TEST_F(VisibleUnitsTest, previousPositionOf) {
       PreviousPositionOf(CreateVisiblePosition(PositionInFlatTree(one, 0)))
           .DeepEquivalent());
 
-  EXPECT_EQ(Position(one, 0),
+  EXPECT_EQ(Position(two, 2),
             PreviousPositionOf(CreateVisiblePosition(Position(one, 1)))
                 .DeepEquivalent());
   EXPECT_EQ(
@@ -636,7 +637,7 @@ TEST_F(VisibleUnitsTest, previousPositionOf) {
       PreviousPositionOf(CreateVisiblePosition(PositionInFlatTree(one, 1)))
           .DeepEquivalent());
 
-  EXPECT_EQ(Position(one, 0),
+  EXPECT_EQ(Position(one, 1),
             PreviousPositionOf(CreateVisiblePosition(Position(two, 0)))
                 .DeepEquivalent());
   EXPECT_EQ(
@@ -662,9 +663,9 @@ TEST_F(VisibleUnitsTest, previousPositionOf) {
       PreviousPositionOf(CreateVisiblePosition(PositionInFlatTree(four, 0)))
           .DeepEquivalent());
 
-  // Note: Canonicalization maps (five, 0) to (four, 4) in DOM tree and
+  // Note: Canonicalization maps (five, 0) to (five, 0) in DOM tree and
   // (one, 1) in flat tree.
-  EXPECT_EQ(Position(four, 4),
+  EXPECT_EQ(Position(five, 0),
             PreviousPositionOf(CreateVisiblePosition(Position(five, 1)))
                 .DeepEquivalent());
   EXPECT_EQ(

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

@@ -213,7 +213,7 @@ TEST_P(ParameterizedVisibleUnitsWordTest, StartOfWordShadowDOM) {
                 StartOfWordPosition(
                     CreateVisiblePositionInFlatTree(*one, 1).DeepEquivalent()))
                 .DeepEquivalent());
-  EXPECT_EQ(Position(one, 0),
+  EXPECT_EQ(Position(four, 0),
             CreateVisiblePosition(
                 StartOfWordPosition(
                     CreateVisiblePositionInDOMTree(*two, 0).DeepEquivalent()))
@@ -428,7 +428,7 @@ TEST_P(ParameterizedVisibleUnitsWordTest, EndOfWordShadowDOM) {
       EndOfWord(CreateVisiblePositionInFlatTree(*one, 1)).DeepEquivalent());
 
   EXPECT_EQ(
-      Position(five, 5),
+      Position(two, 2),
       EndOfWord(CreateVisiblePositionInDOMTree(*two, 0)).DeepEquivalent());
   EXPECT_EQ(
       PositionInFlatTree(two, 2),

third_party/blink/web_tests/editing/editability/ignored-content.html

@@ -31,10 +31,7 @@ for (var i = 0; i < test.childNodes.length; i++) {
 
 Markup.dump(test, '"world" and only "world" should be bolded in each element below');
 
-window.getSelection().collapse(test, 0);
-
-for (var i = 0; i < (text.length + 1) * test.childNodes.length - 1; i++)
-    window.getSelection().modify('extend', 'forward', 'character');
+window.getSelection().setBaseAndExtent(test, 0, test, 3);
 
 var range = window.getSelection().getRangeAt(0).cloneContents();
 

third_party/blink/web_tests/editing/execCommand/insert-list-with-progress-crash.html

@@ -14,14 +14,14 @@ test(()=> assert_selection(
   'InsertOrderedList',
   [
   '<div contenteditable>',
+    '<progress hidden>',
+      '<textarea></textarea>',
+    '</progress>',
     '<ol>',
       '<li>',
         '^<select></select>|',
       '</li>',
     '</ol>',
-    '<progress hidden>',
-      '<textarea></textarea>',
-    '</progress>',
     '<div></div>',
     '</div>']),
   "This InsertOrderedList in contenteditable div with progress.");