Merge "Fixing android.text.format.Time for non-English locales"

1 files changed, 59 insertions, 78 deletions

diff --git a/core/java/android/text/format/TimeFormatter.java b/core/java/android/text/format/TimeFormatter.java
index ec79b3650cb2..3a63805defe0 100644
--- a/core/java/android/text/format/TimeFormatter.java
+++ b/core/java/android/text/format/TimeFormatter.java
@@ -22,8 +22,7 @@ package android.text.format;
 
 import android.content.res.Resources;
 
-import java.nio.ByteBuffer;
-import java.nio.charset.StandardCharsets;
+import java.nio.CharBuffer;
 import java.util.Formatter;
 import java.util.Locale;
 import java.util.TimeZone;
@@ -31,15 +30,13 @@ import libcore.icu.LocaleData;
 import libcore.util.ZoneInfo;
 
 /**
- * Formatting logic for {@link Time}. Contains a port of Bionic's broken strftime_tz to Java. The
- * main issue with this implementation is the treatment of characters as ASCII, despite returning
- * localized (UTF-16) strings from the LocaleData.
+ * Formatting logic for {@link Time}. Contains a port of Bionic's broken strftime_tz to Java.
  *
  * <p>This class is not thread safe.
  */
 class TimeFormatter {
-    // An arbitrary value outside the range representable by a byte / ASCII character code.
-    private static final int FORCE_LOWER_CASE = 0x100;
+    // An arbitrary value outside the range representable by a char.
+    private static final int FORCE_LOWER_CASE = -1;
 
     private static final int SECSPERMIN = 60;
     private static final int MINSPERHOUR = 60;
@@ -62,10 +59,9 @@ class TimeFormatter {
     private final String dateTimeFormat;
     private final String timeOnlyFormat;
     private final String dateOnlyFormat;
-    private final Locale locale;
 
     private StringBuilder outputBuilder;
-    private Formatter outputFormatter;
+    private Formatter numberFormatter;
 
     public TimeFormatter() {
         synchronized (TimeFormatter.class) {
@@ -84,7 +80,6 @@ class TimeFormatter {
             this.dateTimeFormat = sDateTimeFormat;
             this.timeOnlyFormat = sTimeOnlyFormat;
             this.dateOnlyFormat = sDateOnlyFormat;
-            this.locale = locale;
             localeData = sLocaleData;
         }
     }
@@ -97,19 +92,21 @@ class TimeFormatter {
             StringBuilder stringBuilder = new StringBuilder();
 
             outputBuilder = stringBuilder;
-            outputFormatter = new Formatter(stringBuilder, locale);
+            // This uses the US locale because number localization is handled separately (see below)
+            // and locale sensitive strings are output directly using outputBuilder.
+            numberFormatter = new Formatter(stringBuilder, Locale.US);
 
             formatInternal(pattern, wallTime, zoneInfo);
             String result = stringBuilder.toString();
             // This behavior is the source of a bug since some formats are defined as being
-            // in ASCII. Generally localization is very broken.
+            // in ASCII and not localized.
             if (localeData.zeroDigit != '0') {
                 result = localizeDigits(result);
             }
             return result;
         } finally {
             outputBuilder = null;
-            outputFormatter = null;
+            numberFormatter = null;
         }
     }
 
@@ -132,38 +129,30 @@ class TimeFormatter {
      * {@link #outputBuilder}.
      */
     private void formatInternal(String pattern, ZoneInfo.WallTime wallTime, ZoneInfo zoneInfo) {
-        // Convert to ASCII bytes to be compatible with old implementation behavior.
-        byte[] bytes = pattern.getBytes(StandardCharsets.US_ASCII);
-        if (bytes.length == 0) {
-            return;
-        }
-
-        ByteBuffer formatBuffer = ByteBuffer.wrap(bytes);
+        CharBuffer formatBuffer = CharBuffer.wrap(pattern);
         while (formatBuffer.remaining() > 0) {
-            boolean outputCurrentByte = true;
-            char currentByteAsChar = convertToChar(formatBuffer.get(formatBuffer.position()));
-            if (currentByteAsChar == '%') {
-                outputCurrentByte = handleToken(formatBuffer, wallTime, zoneInfo);
+            boolean outputCurrentChar = true;
+            char currentChar = formatBuffer.get(formatBuffer.position());
+            if (currentChar == '%') {
+                outputCurrentChar = handleToken(formatBuffer, wallTime, zoneInfo);
             }
-            if (outputCurrentByte) {
-                currentByteAsChar = convertToChar(formatBuffer.get(formatBuffer.position()));
-                outputBuilder.append(currentByteAsChar);
+            if (outputCurrentChar) {
+                outputBuilder.append(formatBuffer.get(formatBuffer.position()));
             }
-
             formatBuffer.position(formatBuffer.position() + 1);
         }
     }
 
-    private boolean handleToken(ByteBuffer formatBuffer, ZoneInfo.WallTime wallTime,
+    private boolean handleToken(CharBuffer formatBuffer, ZoneInfo.WallTime wallTime,
             ZoneInfo zoneInfo) {
 
-        // The byte at formatBuffer.position() is expected to be '%' at this point.
+        // The char at formatBuffer.position() is expected to be '%' at this point.
         int modifier = 0;
         while (formatBuffer.remaining() > 1) {
-            // Increment the position then get the new current byte.
+            // Increment the position then get the new current char.
             formatBuffer.position(formatBuffer.position() + 1);
-            char currentByteAsChar = convertToChar(formatBuffer.get(formatBuffer.position()));
-            switch (currentByteAsChar) {
+            char currentChar = formatBuffer.get(formatBuffer.position());
+            switch (currentChar) {
                 case 'A':
                     modifyAndAppend((wallTime.getWeekDay() < 0
                                     || wallTime.getWeekDay() >= DAYSPERWEEK)
@@ -206,7 +195,7 @@ class TimeFormatter {
                     formatInternal("%m/%d/%y", wallTime, zoneInfo);
                     return false;
                 case 'd':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getMonthDay());
                     return false;
                 case 'E':
@@ -218,46 +207,46 @@ class TimeFormatter {
                 case '0':
                 case '^':
                 case '#':
-                    modifier = currentByteAsChar;
+                    modifier = currentChar;
                     continue;
                 case 'e':
-                    outputFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"),
                             wallTime.getMonthDay());
                     return false;
                 case 'F':
                     formatInternal("%Y-%m-%d", wallTime, zoneInfo);
                     return false;
                 case 'H':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getHour());
                     return false;
                 case 'I':
                     int hour = (wallTime.getHour() % 12 != 0) ? (wallTime.getHour() % 12) : 12;
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), hour);
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), hour);
                     return false;
                 case 'j':
                     int yearDay = wallTime.getYearDay() + 1;
-                    outputFormatter.format(getFormat(modifier, "%03d", "%3d", "%d", "%03d"),
+                    numberFormatter.format(getFormat(modifier, "%03d", "%3d", "%d", "%03d"),
                             yearDay);
                     return false;
                 case 'k':
-                    outputFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"),
                             wallTime.getHour());
                     return false;
                 case 'l':
                     int n2 = (wallTime.getHour() % 12 != 0) ? (wallTime.getHour() % 12) : 12;
-                    outputFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"), n2);
+                    numberFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"), n2);
                     return false;
                 case 'M':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getMinute());
                     return false;
                 case 'm':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getMonth() + 1);
                     return false;
                 case 'n':
-                    modifyAndAppend("\n", modifier);
+                    outputBuilder.append('\n');
                     return false;
                 case 'p':
                     modifyAndAppend((wallTime.getHour() >= (HOURSPERDAY / 2)) ? localeData.amPm[1]
@@ -274,27 +263,27 @@ class TimeFormatter {
                     formatInternal("%I:%M:%S %p", wallTime, zoneInfo);
                     return false;
                 case 'S':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getSecond());
                     return false;
                 case 's':
                     int timeInSeconds = wallTime.mktime(zoneInfo);
-                    modifyAndAppend(Integer.toString(timeInSeconds), modifier);
+                    outputBuilder.append(Integer.toString(timeInSeconds));
                     return false;
                 case 'T':
                     formatInternal("%H:%M:%S", wallTime, zoneInfo);
                     return false;
                 case 't':
-                    modifyAndAppend("\t", modifier);
+                    outputBuilder.append('\t');
                     return false;
                 case 'U':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             (wallTime.getYearDay() + DAYSPERWEEK - wallTime.getWeekDay())
                                     / DAYSPERWEEK);
                     return false;
                 case 'u':
                     int day = (wallTime.getWeekDay() == 0) ? DAYSPERWEEK : wallTime.getWeekDay();
-                    outputFormatter.format("%d", day);
+                    numberFormatter.format("%d", day);
                     return false;
                 case 'V':   /* ISO 8601 week number */
                 case 'G':   /* ISO 8601 year (four digits) */
@@ -326,9 +315,9 @@ class TimeFormatter {
                         --year;
                         yday += isLeap(year) ? DAYSPERLYEAR : DAYSPERNYEAR;
                     }
-                    if (currentByteAsChar == 'V') {
-                        outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), w);
-                    } else if (currentByteAsChar == 'g') {
+                    if (currentChar == 'V') {
+                        numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), w);
+                    } else if (currentChar == 'g') {
                         outputYear(year, false, true, modifier);
                     } else {
                         outputYear(year, true, true, modifier);
@@ -342,10 +331,10 @@ class TimeFormatter {
                     int n = (wallTime.getYearDay() + DAYSPERWEEK - (
                                     wallTime.getWeekDay() != 0 ? (wallTime.getWeekDay() - 1)
                                             : (DAYSPERWEEK - 1))) / DAYSPERWEEK;
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), n);
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), n);
                     return false;
                 case 'w':
-                    outputFormatter.format("%d", wallTime.getWeekDay());
+                    numberFormatter.format("%d", wallTime.getWeekDay());
                     return false;
                 case 'X':
                     formatInternal(timeOnlyFormat, wallTime, zoneInfo);
@@ -371,17 +360,17 @@ class TimeFormatter {
                         return false;
                     }
                     int diff = wallTime.getGmtOffset();
-                    String sign;
+                    char sign;
                     if (diff < 0) {
-                        sign = "-";
+                        sign = '-';
                         diff = -diff;
                     } else {
-                        sign = "+";
+                        sign = '+';
                     }
-                    modifyAndAppend(sign, modifier);
+                    outputBuilder.append(sign);
                     diff /= SECSPERMIN;
                     diff = (diff / MINSPERHOUR) * 100 + (diff % MINSPERHOUR);
-                    outputFormatter.format(getFormat(modifier, "%04d", "%4d", "%d", "%04d"), diff);
+                    numberFormatter.format(getFormat(modifier, "%04d", "%4d", "%d", "%04d"), diff);
                     return false;
                 }
                 case '+':
@@ -422,7 +411,6 @@ class TimeFormatter {
                 break;
             default:
                 outputBuilder.append(str);
-
         }
     }
 
@@ -443,14 +431,14 @@ class TimeFormatter {
         }
         if (outputTop) {
             if (lead == 0 && trail < 0) {
-                modifyAndAppend("-0", modifier);
+                outputBuilder.append("-0");
             } else {
-                outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), lead);
+                numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), lead);
             }
         }
         if (outputBottom) {
             int n = ((trail < 0) ? -trail : trail);
-            outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), n);
+            numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), n);
         }
     }
 
@@ -472,24 +460,24 @@ class TimeFormatter {
     }
 
     /**
-     * A broken implementation of {@link Character#isUpperCase(char)} that assumes ASCII in order to
-     * be compatible with the old native implementation.
+     * A broken implementation of {@link Character#isUpperCase(char)} that assumes ASCII codes in
+     * order to be compatible with the old native implementation.
      */
     private static boolean brokenIsUpper(char toCheck) {
         return toCheck >= 'A' && toCheck <= 'Z';
     }
 
     /**
-     * A broken implementation of {@link Character#isLowerCase(char)} that assumes ASCII in order to
-     * be compatible with the old native implementation.
+     * A broken implementation of {@link Character#isLowerCase(char)} that assumes ASCII codes in
+     * order to be compatible with the old native implementation.
      */
     private static boolean brokenIsLower(char toCheck) {
         return toCheck >= 'a' && toCheck <= 'z';
     }
 
     /**
-     * A broken implementation of {@link Character#toLowerCase(char)} that assumes ASCII in order to
-     * be compatible with the old native implementation.
+     * A broken implementation of {@link Character#toLowerCase(char)} that assumes ASCII codes in
+     * order to be compatible with the old native implementation.
      */
     private static char brokenToLower(char input) {
         if (input >= 'A' && input <= 'Z') {
@@ -499,8 +487,8 @@ class TimeFormatter {
     }
 
     /**
-     * A broken implementation of {@link Character#toUpperCase(char)} that assumes ASCII in order to
-     * be compatible with the old native implementation.
+     * A broken implementation of {@link Character#toUpperCase(char)} that assumes ASCII codes in
+     * order to be compatible with the old native implementation.
      */
     private static char brokenToUpper(char input) {
         if (input >= 'a' && input <= 'z') {
@@ -509,11 +497,4 @@ class TimeFormatter {
         return input;
     }
 
-    /**
-     * Safely convert a byte containing an ASCII character to a char, even for character codes
-     * > 127.
-     */
-    private static char convertToChar(byte b) {
-        return (char) (b & 0xFF);
-    }
 }