Right aligned backspace bug (#25229)

* Fix bug in calculation of downstream text distance when aligned: right

* After having affinity explained to me, match getOffset and tests to
expected behavior

* Clean up test formatting and fix analyze errors

* Improve test comments and reorganize a bit

* Specify logical pixels

packages/flutter/lib/src/painting/text_painter.dart

@@ -417,6 +417,8 @@ class TextPainter {
   // Unicode value for a zero width joiner character.
   static const int _zwjUtf16 = 0x200d;
 
+  // Get the Offset of the cursor (in logical pixels) based off the near edge
+  // of the character upstream from the given string offset.
   // TODO(garyq): Use actual extended grapheme cluster length instead of
   // an increasing cluster length amount to achieve deterministic performance.
   Offset _getOffsetFromUpstream(int offset, Rect caretPrototype) {
@@ -424,6 +426,7 @@ class TextPainter {
     final int prevCodeUnit = _text.codeUnitAt(max(0, offset - 1));
     if (prevCodeUnit == null)
       return null;
+
     // Check for multi-code-unit glyphs such as emojis or zero width joiner
     final bool needsSearch = _isUtf16Surrogate(prevCodeUnit) || _text.codeUnitAt(offset) == _zwjUtf16;
     int graphemeClusterLength = needsSearch ? 2 : 1;
@@ -447,6 +450,13 @@ class TextPainter {
         continue;
       }
       final TextBox box = boxes.first;
+
+      // If the upstream character is a newline, cursor is at start of next line
+      const int NEWLINE_CODE_UNIT = 10;
+      if (prevCodeUnit == NEWLINE_CODE_UNIT) {
+        return Offset(_emptyOffset.dx, box.bottom);
+      }
+
       final double caretEnd = box.end;
       final double dx = box.direction == TextDirection.rtl ? caretEnd - caretPrototype.width : caretEnd;
       return Offset(dx, box.top);
@@ -454,6 +464,8 @@ class TextPainter {
     return null;
   }
 
+  // Get the Offset of the cursor (in logical pixels) based off the near edge
+  // of the character downstream from the given string offset.
   // TODO(garyq): Use actual extended grapheme cluster length instead of
   // an increasing cluster length amount to achieve deterministic performance.
   Offset _getOffsetFromDownstream(int offset, Rect caretPrototype) {

packages/flutter/test/painting/text_painter_test.dart

@@ -16,7 +16,10 @@ void main() {
     painter.text = TextSpan(text: text);
     painter.layout();
 
-    Offset caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 0), ui.Rect.zero);
+    Offset caretOffset = painter.getOffsetForCaret(
+      const ui.TextPosition(offset: 0),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, 0);
     caretOffset = painter.getOffsetForCaret(ui.TextPosition(offset: text.length), ui.Rect.zero);
     expect(caretOffset.dx, painter.width);
@@ -62,7 +65,7 @@ void main() {
     // One three-person family, one four person family, one US flag.
     const String text = '👩‍👩‍👦👩‍👩‍👧‍👧🇺🇸';
     painter.text = const TextSpan(text: text);
-    painter.layout();
+    painter.layout(maxWidth: 10000);
 
     expect(text.length, 23);
 
@@ -293,112 +296,340 @@ void main() {
     final TextPainter painter = TextPainter()
       ..textDirection = TextDirection.ltr;
 
+    const double SIZE_OF_A = 14.0; // square size of "a" character
     String text = 'aaa';
     painter.text = TextSpan(text: text);
     painter.layout();
 
-    Offset caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 0), ui.Rect.zero);
-    expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    caretOffset = painter.getOffsetForCaret(ui.TextPosition(offset: text.length), ui.Rect.zero);
-    expect(caretOffset.dx, painter.width);
+    // getOffsetForCaret in a plain one-line string is the same for either affinity.
+    int offset = 0;
+    painter.text = TextSpan(text: text);
+    painter.layout();
+    Offset caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * offset, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * offset, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    offset = 1;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * offset, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * offset, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    offset = 2;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * offset, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * offset, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    offset = 3;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * offset, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * offset, 0.0001));
     expect(caretOffset.dy, closeTo(0.0, 0.0001));
 
-    // Check that getOffsetForCaret handles a trailing newline when affinity is downstream.
-    text = 'aaa\n';
+    // For explicit newlines, getOffsetForCaret places the caret at the location
+    // indicated by offset regardless of affinity.
+    text = '\n\n';
     painter.text = TextSpan(text: text);
     painter.layout();
-    caretOffset = painter.getOffsetForCaret(ui.TextPosition(offset: text.length, affinity: TextAffinity.downstream), ui.Rect.zero);
+    offset = 0;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    offset = 1;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
+    offset = 2;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 2, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(14.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 2, 0.0001));
 
-    // Check that getOffsetForCaret handles a trailing newline when affinity is upstream.
-    text = 'aaa\n';
+    // getOffsetForCaret in an unwrapped string with explicit newlines is the
+    // same for either affinity.
+    text = '\naaa';
     painter.text = TextSpan(text: text);
     painter.layout();
-    caretOffset = painter.getOffsetForCaret(ui.TextPosition(offset: text.length, affinity: TextAffinity.upstream), ui.Rect.zero);
-    expect(caretOffset.dx, painter.width);
+    offset = 0;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
     expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    offset = 1;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
 
-    // Correctly moves through second line with downstream affinity.
-    text = 'aaa\naaa';
+    // When text wraps on its own, getOffsetForCaret disambiguates between the
+    // end of one line and start of next using affinity.
+    text = 'aaaaaaaa'; // Just enough to wrap one character down to second line
     painter.text = TextSpan(text: text);
-    painter.layout();
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 4), ui.Rect.zero);
+    painter.layout(maxWidth: 100); // SIZE_OF_A * text.length > 100, so it wraps
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: text.length - 1),
+      ui.Rect.zero,
+    );
+    // When affinity is downstream, cursor is at beginning of second line
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(14.0, 0.0001));
-
-    // Correctly moves through second line with upstream affinity.
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 4, affinity: TextAffinity.upstream), ui.Rect.zero);
-    expect(caretOffset.dx, closeTo(42.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: text.length - 1, affinity: ui.TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    // When affinity is upstream, cursor is at end of first line
+    expect(caretOffset.dx, closeTo(98.0, 0.0001));
     expect(caretOffset.dy, closeTo(0.0, 0.0001));
 
-    // Correctly handles multiple trailing newlines.
-    text = 'aaa\n\n\n';
+    // When given a string with a newline at the end, getOffsetForCaret puts
+    // the cursor at the start of the next line regardless of affinity
+    text = 'aaa\n';
     painter.text = TextSpan(text: text);
     painter.layout();
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 4), ui.Rect.zero);
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: text.length),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(14.0, 0.001));
-
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 5), ui.Rect.zero);
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
+    offset = text.length;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(28.0, 0.001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 6), ui.Rect.zero);
+    // Given a one-line right aligned string, positioning the cursor at offset 0
+    // means that it appears at the "end" of the string, after the character
+    // that was typed first, at x=0.
+    painter.textAlign = TextAlign.right;
+    text = 'aaa';
+    painter.text = TextSpan(text: text);
+    painter.layout();
+    offset = 0;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(42.0, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    painter.textAlign = TextAlign.left;
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 6, affinity: TextAffinity.upstream), ui.Rect.zero);
+    // When given an offset after a newline in the middle of a string,
+    // getOffsetForCaret returns the start of the next line regardless of
+    // affinity.
+    text = 'aaa\naaa';
+    painter.text = TextSpan(text: text);
+    painter.layout();
+    offset = 4;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(28.0, 0.0001));
-
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 5, affinity: TextAffinity.upstream), ui.Rect.zero);
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(14.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 4, affinity: TextAffinity.upstream), ui.Rect.zero);
-    expect(caretOffset.dx, closeTo(42.0, 0.0001));
+    // When given a string with multiple trailing newlines, places the caret
+    // in the position given by offset regardless of affinity.
+    text = 'aaa\n\n\n';
+    offset = 3;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * 3, 0.0001));
     expect(caretOffset.dy, closeTo(0.0, 0.0001));
-
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 3, affinity: TextAffinity.upstream), ui.Rect.zero);
-    expect(caretOffset.dx, closeTo(42.0, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(SIZE_OF_A * 3, 0.0001));
     expect(caretOffset.dy, closeTo(0.0, 0.0001));
 
-    // Correctly handles multiple leading newlines
-    text = '\n\n\naaa';
+    offset = 4;
     painter.text = TextSpan(text: text);
     painter.layout();
-
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 3), ui.Rect.zero);
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(42.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 2), ui.Rect.zero);
+    offset = 5;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 2, 0.001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(28.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 2, 0.0001));
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 1), ui.Rect.zero);
+    offset = 6;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy,closeTo(14.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 3, 0.0001));
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 0), ui.Rect.zero);
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 3, 0.0001));
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 0, affinity: TextAffinity.upstream), ui.Rect.zero);
+    // When given a string with multiple leading newlines, places the caret in
+    // the position given by offset regardless of affinity.
+    text = '\n\n\naaa';
+    offset = 3;
+    painter.text = TextSpan(text: text);
+    painter.layout();
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 3, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 3, 0.0001));
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 1, affinity: TextAffinity.upstream), ui.Rect.zero);
+    offset = 2;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 2, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A * 2, 0.0001));
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 2, affinity: TextAffinity.upstream), ui.Rect.zero);
+    offset = 1;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(14.0, 0.0001));
+    expect(caretOffset.dy,closeTo(SIZE_OF_A, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(SIZE_OF_A, 0.0001));
 
-    caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 3, affinity: TextAffinity.upstream), ui.Rect.zero);
+    offset = 0;
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset),
+      ui.Rect.zero,
+    );
+    expect(caretOffset.dx, closeTo(0.0, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
+    caretOffset = painter.getOffsetForCaret(
+      ui.TextPosition(offset: offset, affinity: TextAffinity.upstream),
+      ui.Rect.zero,
+    );
     expect(caretOffset.dx, closeTo(0.0, 0.0001));
-    expect(caretOffset.dy, closeTo(28.0, 0.0001));
+    expect(caretOffset.dy, closeTo(0.0, 0.0001));
   });
 }