Simplify & improve DialerBidiFormatter

Bug: 72162627,78464687
Test: DialerBidiFormatterTest + Manual testing
PiperOrigin-RevId: 198950604
Change-Id: Ia3d4d29b7c6a96a7facfeb5c41b17a6e7cabebf2

2 files changed, 45 insertions, 111 deletions

diff --git a/java/com/android/dialer/i18n/DialerBidiFormatter.java b/java/com/android/dialer/i18n/DialerBidiFormatter.java
index e882e06c4..440db1783 100644
--- a/java/com/android/dialer/i18n/DialerBidiFormatter.java
+++ b/java/com/android/dialer/i18n/DialerBidiFormatter.java
@@ -17,141 +17,75 @@
 package com.android.dialer.i18n;
 
 import android.support.annotation.Nullable;
-import android.support.annotation.VisibleForTesting;
-import android.support.v4.text.BidiFormatter;
+import android.telephony.PhoneNumberUtils;
+import android.text.SpannableStringBuilder;
+import android.text.SpannedString;
 import android.text.TextUtils;
 import android.util.Patterns;
-import com.android.dialer.common.Assert;
-import com.google.auto.value.AutoValue;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
-/**
- * An enhanced version of {@link BidiFormatter} that can recognize a formatted phone number
- * containing whitespaces.
- *
- * <p>Formatted phone numbers usually contain one or more whitespaces (e.g., "+1 650-253-0000",
- * "(650) 253-0000", etc). {@link BidiFormatter} mistakes such a number for tokens separated by
- * whitespaces. Therefore, these numbers can't be correctly shown in a RTL context (e.g., "+1
- * 650-253-0000" would be shown as "650-253-0000 1+".)
- */
+/** A formatter that applies bidirectional formatting to phone numbers in text. */
 public final class DialerBidiFormatter {
 
-  private DialerBidiFormatter() {}
-
-  // Regular expression that matches a single space in the beginning or end of a string.
-  private static final String REGEXP_SURROUNDING_SPACE = "^[ ]|[ ]$";
-
-  /**
-   * Divides the given text into segments, applies {@link BidiFormatter#unicodeWrap(CharSequence)}
-   * to each segment, and then reassembles the text.
-   *
-   * <p>A segment of the text is either a substring matching {@link Patterns#PHONE} or one that does
-   * not.
-   *
-   * @see BidiFormatter#unicodeWrap(CharSequence)
-   */
-  public static CharSequence unicodeWrap(@Nullable CharSequence text) {
-    if (TextUtils.isEmpty(text)) {
-      return text;
-    }
-
-    List<CharSequence> segments = segmentText(text);
+  /** Unicode "Left-To-Right Embedding" (LRE) character. */
+  private static final char LRE = '\u202A';
 
-    StringBuilder formattedText = new StringBuilder();
-    for (CharSequence segment : segments) {
-      formattedText.append(BidiFormatter.getInstance().unicodeWrap(segment));
-    }
+  /** Unicode "Pop Directional Formatting" (PDF) character. */
+  private static final char PDF = '\u202C';
 
-    return formattedText.toString();
-  }
+  private DialerBidiFormatter() {}
 
   /**
-   * Segments the given text into a sequence of substrings using the following procedure.
+   * Divides the given text into segments, applies LTR formatting and adds TTS span to segments that
+   * are phone numbers, then reassembles the text.
    *
-   * <ol>
-   *   <li>Separate text matching {@link Patterns#PHONE} from others.
-   *       <p>For example: "Mobile, +1 650-253-0000, 20 seconds" will be segmented into<br>
-   *       {"Mobile, ", "+1 650-253-0000", ", 20 seconds"}
-   *   <li>For each substring produced by the previous step, separate a single whitespace at the
-   *       start/end of it from the rest of the substring.
-   *       <p>For example, the first substring "Mobile, " will be segmented into {"Mobile,", " "}.
-   * </ol>
+   * <p>Formatted phone numbers usually contain one or more whitespaces (e.g., "+1 650-253-0000",
+   * "(650) 253-0000", etc). The system mistakes such a number for tokens separated by whitespaces.
+   * Therefore, these numbers can't be correctly shown in a RTL context (e.g., "+1 650-253-0000"
+   * would be shown as "650-253-0000 1+".)
    *
-   * <p>The final result of segmenting "Mobile, +1 650-253-0000, 20 seconds" is<br>
-   * {"Mobile,", " ", "+1 650-253-0000", ", 20 seconds"}.
+   * <p>This method wraps phone numbers with Unicode formatting characters LRE & PDF to ensure phone
+   * numbers are always shown as LTR strings.
    *
-   * <p>The reason for singling out the whitespace at the start/end of a substring is to prevent it
-   * from being misplaced in RTL context.
+   * <p>Note that the regex used to find phone numbers ({@link Patterns#PHONE}) will also match any
+   * number. As this method also adds TTS span to segments that match {@link Patterns#PHONE}, extra
+   * actions need to be taken if you don't want a number to be read as a phone number by TalkBack.
    */
-  @VisibleForTesting
-  static List<CharSequence> segmentText(CharSequence text) {
-    Assert.checkArgument(!TextUtils.isEmpty(text));
-
-    // Separate text matching the phone number pattern from others.
-    List<CharSequence> segmentsSeparatingPhoneNumbers = segmentText(text, Patterns.PHONE);
-
-    // For each substring, separate a single whitespace at the start/end of it from the rest of the
-    // substring.
-    List<CharSequence> finalSegments = new ArrayList<>();
-    Pattern patternSurroundingSpace = Pattern.compile(REGEXP_SURROUNDING_SPACE);
-    for (CharSequence segment : segmentsSeparatingPhoneNumbers) {
-      finalSegments.addAll(segmentText(segment, patternSurroundingSpace));
+  public static CharSequence format(@Nullable CharSequence text) {
+    if (TextUtils.isEmpty(text)) {
+      return text;
     }
 
-    return finalSegments;
-  }
+    SpannableStringBuilder spannableStringBuilder = new SpannableStringBuilder();
 
-  /** Segments the given text into a sequence of substrings using the provided pattern. */
-  private static List<CharSequence> segmentText(CharSequence text, Pattern pattern) {
-    Assert.checkArgument(!TextUtils.isEmpty(text));
+    // Find the start index and the end index of each segment matching the phone number pattern.
+    Matcher matcher = Patterns.PHONE.matcher(text.toString());
 
-    List<CharSequence> segments = new ArrayList<>();
-
-    // Find the start index and the end index of each segment matching the pattern.
-    Matcher matcher = pattern.matcher(text.toString());
-    List<Range> segmentRanges = new ArrayList<>();
+    int currIndex = 0;
     while (matcher.find()) {
-      segmentRanges.add(Range.newBuilder().setStart(matcher.start()).setEnd(matcher.end()).build());
-    }
+      int start = matcher.start();
+      int end = matcher.end();
 
-    // Segment the text.
-    int currIndex = 0;
-    for (Range segmentRange : segmentRanges) {
-      if (currIndex < segmentRange.getStart()) {
-        segments.add(text.subSequence(currIndex, segmentRange.getStart()));
+      // Handle the case where the input text doesn't start with a phone number.
+      if (currIndex < start) {
+        spannableStringBuilder.append(text.subSequence(currIndex, start));
       }
 
-      segments.add(text.subSequence(segmentRange.getStart(), segmentRange.getEnd()));
-      currIndex = segmentRange.getEnd();
-    }
-    if (currIndex < text.length()) {
-      segments.add(text.subSequence(currIndex, text.length()));
-    }
-
-    return segments;
-  }
+      // For a phone number, wrap it with Unicode characters LRE & PDF so that it will always be
+      // shown as a LTR string.
+      spannableStringBuilder.append(
+          PhoneNumberUtils.createTtsSpannable(
+              TextUtils.concat(
+                  String.valueOf(LRE), text.subSequence(start, end), String.valueOf(PDF))));
 
-  /** Represents the start index (inclusive) and the end index (exclusive) of a text segment. */
-  @AutoValue
-  abstract static class Range {
-    static Builder newBuilder() {
-      return new AutoValue_DialerBidiFormatter_Range.Builder();
+      currIndex = end;
     }
 
-    abstract int getStart();
-
-    abstract int getEnd();
-
-    @AutoValue.Builder
-    abstract static class Builder {
-      abstract Builder setStart(int start);
-
-      abstract Builder setEnd(int end);
-
-      abstract Range build();
+    // Handle the case where the input doesn't end with a phone number.
+    if (currIndex < text.length()) {
+      spannableStringBuilder.append(text.subSequence(currIndex, text.length()));
     }
+
+    return new SpannedString(spannableStringBuilder);
   }
 }
diff --git a/java/com/android/dialer/widget/BidiTextView.java b/java/com/android/dialer/widget/BidiTextView.java
index 6cf1aaedf..5dd15de96 100644
--- a/java/com/android/dialer/widget/BidiTextView.java
+++ b/java/com/android/dialer/widget/BidiTextView.java
@@ -35,6 +35,6 @@ public final class BidiTextView extends TextView {
 
   @Override
   public void setText(CharSequence text, BufferType type) {
-    super.setText(DialerBidiFormatter.unicodeWrap(text), type);
+    super.setText(DialerBidiFormatter.format(text), type);
   }
 }