Partial Revert "[Editing] Fix canonical caret position at line break"

This partially reverts commit d46bdb4f.

Reason for revert: the change causes a regression where inserting text
at end of current block and next block is an inline block, then the new
selection will be end up at the beginning of next block. Per suggestion
in crbug.com/1161370#c15, we only need to partially revert the change(
change made in VisiblePositionTemplate::Create.

Original change's description:
> [Editing] Fix canonical caret position at line break
>
> Consider this testcase:
>     <style>div { width: min-content; padding: 5px; }</style>
>     <div contenteditable>line1<wbr>line2</div>
> which is rendered as
>     line1
>     line2
>
> Before this patch, when clicking at the beginning of the 2nd line, the
> caret would appear at the end of the 1st one, because CanonicalPosition
> would search backwards even with a downstream affinity.
>
> Also, when clicking at the beginning of the 1st line and pressing the
> down arrow key, the caret would move to the end of the 1st line instead
> of to the beginning of the 2nd one. And pressing the key again would
> have no effect, the caret would refuse to go down.
>
> This patch fixes these problems by making CanonicalPosition take a
> TextAffinity parameter which affects whether the canonical position is
> first searched backwards or forwards. If no suitable candidate is found,
> it will still search in the other direction.
>
> And then, VisiblePosition::Create takes care of deciding between the
> upstream and the downstream canonical positions, depending on the
> affinity and whether there is a line break.
>
> Bug: 1002937
>
> Web tests:
> TEST=external/wpt/editing/run/caret-navigation-around-line-break.html
>
> Unit tests:
> TEST=All/ParameterizedVisibleUnitsLineTest.inSameLine/0
> TEST=All/ParameterizedVisibleUnitsLineTest.inSameLine/1
> TEST=All/ParameterizedVisibleUnitsWordTest.StartOfWordShadowDOM/0
> TEST=All/ParameterizedVisibleUnitsWordTest.StartOfWordShadowDOM/1
> 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.canonicalPositionOfWithHTMLHtmlElement
> TEST=VisibleUnitsTest.canonicalPositionOfWithInputElement
> TEST=VisibleUnitsTest.canonicalizationWithCollapsedSpaceAndIsolatedCombiningCharacter
>
> Change-Id: I82d86d40a87513b2e92c024735957e9f71154094
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410404
> Commit-Queue: Oriol Brufau <obrufau@igalia.com>
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#813744}

Bug: 1002937
Bug: 1161370
Change-Id: I65dbc8b442ae308f42582095e4693d9f146b8b32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2616734Reviewed-by: Siye Liu <siliu@microsoft.com>
Reviewed-by: Oriol Brufau <obrufau@igalia.com>
Commit-Queue: Siye Liu <siliu@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#841598}

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

@@ -82,55 +82,36 @@ VisiblePositionTemplate<Strategy> VisiblePositionTemplate<Strategy>::Create(
   DocumentLifecycle::DisallowTransitionScope disallow_transition(
       document.Lifecycle());
 
-  // Find the canonical position with a backward preference.
-  const PositionWithAffinityTemplate<Strategy> backward_position =
-      SnapBackward(position_with_affinity.GetPosition());
-  if (backward_position.IsNull())
+  const PositionTemplate<Strategy> deep_position =
+      CanonicalPositionOf(position_with_affinity.GetPosition());
+  if (deep_position.IsNull())
     return VisiblePositionTemplate<Strategy>();
-  const PositionWithAffinityTemplate<Strategy> backward_position_upstream(
-      backward_position.GetPosition(), TextAffinity::kUpstream);
-  const PositionWithAffinityTemplate<Strategy> backward_position_downstream(
-      backward_position.GetPosition(), TextAffinity::kDownstream);
+  const PositionWithAffinityTemplate<Strategy> downstream_position(
+      deep_position);
+  if (position_with_affinity.Affinity() == TextAffinity::kDownstream)
+    return VisiblePositionTemplate<Strategy>(downstream_position);
 
   if (RuntimeEnabledFeatures::BidiCaretAffinityEnabled() &&
-      NGInlineFormattingContextOf(backward_position.GetPosition())) {
-    if (position_with_affinity.Affinity() == TextAffinity::kDownstream)
-      return VisiblePositionTemplate<Strategy>(backward_position_downstream);
+      NGInlineFormattingContextOf(deep_position)) {
     // When not at a line wrap or bidi boundary, make sure to end up with
     // |TextAffinity::Downstream| affinity.
-    if (AbsoluteCaretBoundsOf(backward_position_upstream) ==
-        AbsoluteCaretBoundsOf(backward_position_downstream)) {
-      return VisiblePositionTemplate<Strategy>(backward_position_downstream);
-    }
-    return VisiblePositionTemplate<Strategy>(backward_position_upstream);
-  }
+    const PositionWithAffinityTemplate<Strategy> upstream_position(
+        deep_position, TextAffinity::kUpstream);
 
-  // Find the canonical position with a forward preference. If backward_position
-  // has a downstream affinity, it means that we couldn't find any backward
-  // candidate, so they must be equal and we can avoid calling SnapForward().
-  // The forward canonical position can't be null because we already checked
-  // that the backward one is not null.
-  const PositionWithAffinityTemplate<Strategy> forward_position =
-      backward_position.Affinity() == TextAffinity::kDownstream
-          ? backward_position
-          : SnapForward(position_with_affinity.GetPosition());
-  DCHECK(forward_position.IsNotNull());
-
-  // Fast path to avoid slow InSameLine() below in common cases.
-  if (position_with_affinity.Affinity() == TextAffinity::kDownstream &&
-      backward_position_downstream == forward_position) {
-    return VisiblePositionTemplate<Strategy>(backward_position_downstream);
+    if (AbsoluteCaretBoundsOf(downstream_position) !=
+        AbsoluteCaretBoundsOf(upstream_position)) {
+      return VisiblePositionTemplate<Strategy>(upstream_position);
+    }
+    return VisiblePositionTemplate<Strategy>(downstream_position);
   }
 
-  // When not at a line wrap, make sure to end up with the backward canonical
-  // position with |TextAffinity::Downstream| affinity.
-  if (InSameLine(backward_position_upstream, forward_position))
-    return VisiblePositionTemplate<Strategy>(backward_position_downstream);
-  if (position_with_affinity.Affinity() == TextAffinity::kUpstream)
-    return VisiblePositionTemplate<Strategy>(backward_position_upstream);
-  if (StartOfLine(forward_position).IsNull())
-    return VisiblePositionTemplate<Strategy>(backward_position_downstream);
-  return VisiblePositionTemplate<Strategy>(forward_position);
+  // When not at a line wrap, make sure to end up with
+  // |TextAffinity::Downstream| affinity.
+  const PositionWithAffinityTemplate<Strategy> upstream_position(
+      deep_position, TextAffinity::kUpstream);
+  if (InSameLine(downstream_position, upstream_position))
+    return VisiblePositionTemplate<Strategy>(downstream_position);
+  return VisiblePositionTemplate<Strategy>(upstream_position);
 }
 
 template <typename Strategy>

third_party/blink/web_tests/TestExpectations

@@ -1431,6 +1431,8 @@ crbug.com/1143074 [ Mac ] external/wpt/html/semantics/forms/the-input-element/se
 
 crbug.com/860211 [ Mac ] external/wpt/editing/run/delete.html [ Failure ]
 
+crbug.com/1002937 external/wpt/editing/run/caret-navigation-around-line-break.html [ Failure ]
+
 crbug.com/821455 editing/pasteboard/drag-files-to-editable-element.html [ Failure ]
 
 crbug.com/688613 external/wpt/mediacapture-streams/MediaStreamTrack-MediaElement-disabled-audio-is-silence.https.html [ Skip ]

third_party/blink/web_tests/editing/execCommand/indent/indent_blockquote_with_inline_nested_paragraphs.html

@@ -151,9 +151,7 @@ selection_test(
             '<blockquote style="margin: 0 0 0 40px; border: none; padding: 0px;">^hello',
             '<svg viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">',
             '<foreignobject height="80" width="80" x="20" y="20">Test|</foreignobject>',
-            '</svg>',
-            '<svg viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">',
-            '<foreignobject height="80" width="80" x="20" y="20">Test</foreignobject></svg></blockquote>',
+            '</svg></blockquote>',
         '</pre>',
     ],
     'Indent content spanning across different selection contexts');