ICU-20741 Changing SimpleDateTimeFormat::subFormat to only include 1 field at the same position when there is a data fallback
diff --git a/icu4c/source/i18n/smpdtfmt.cpp b/icu4c/source/i18n/smpdtfmt.cpp
index 5fcbb58..6246e8f 100644
--- a/icu4c/source/i18n/smpdtfmt.cpp
+++ b/icu4c/source/i18n/smpdtfmt.cpp
@@ -996,7 +996,8 @@
// Use subFormat() to format a repeated pattern character
// when a different pattern or non-pattern character is seen
if (ch != prevCh && count > 0) {
- subFormat(appendTo, prevCh, count, capitalizationContext, fieldNum++, handler, *workCal, status);
+ subFormat(appendTo, prevCh, count, capitalizationContext, fieldNum++,
+ prevCh, handler, *workCal, status);
count = 0;
}
if (ch == QUOTE) {
@@ -1023,7 +1024,8 @@
// Format the last item in the pattern, if any
if (count > 0) {
- subFormat(appendTo, prevCh, count, capitalizationContext, fieldNum++, handler, *workCal, status);
+ subFormat(appendTo, prevCh, count, capitalizationContext, fieldNum++,
+ prevCh, handler, *workCal, status);
}
if (calClone != NULL) {
@@ -1402,10 +1404,11 @@
//---------------------------------------------------------------------
void
SimpleDateFormat::subFormat(UnicodeString &appendTo,
- UChar ch,
+ char16_t ch,
int32_t count,
UDisplayContext capitalizationContext,
int32_t fieldNum,
+ char16_t fieldToOutput,
FieldPositionHandler& handler,
Calendar& cal,
UErrorCode& status) const
@@ -1853,8 +1856,11 @@
// In either case, fall back to am/pm.
if (toAppend == NULL || toAppend->isBogus()) {
// Reformat with identical arguments except ch, now changed to 'a'.
- subFormat(appendTo, 0x61, count, capitalizationContext, fieldNum,
- handler, cal, status);
+ // We are passing a different fieldToOutput because we want to add
+ // 'b' to field position. This makes this fallback stable when
+ // there is a data change on locales.
+ subFormat(appendTo, u'a', count, capitalizationContext, fieldNum, u'b', handler, cal, status);
+ return;
} else {
appendTo += *toAppend;
}
@@ -1874,9 +1880,11 @@
if (ruleSet == NULL) {
// Data doesn't exist for the locale we're looking for.
// Falling back to am/pm.
- subFormat(appendTo, 0x61, count, capitalizationContext, fieldNum,
- handler, cal, status);
- break;
+ // We are passing a different fieldToOutput because we want to add
+ // 'B' to field position. This makes this fallback stable when
+ // there is a data change on locales.
+ subFormat(appendTo, u'a', count, capitalizationContext, fieldNum, u'B', handler, cal, status);
+ return;
}
// Get current display time.
@@ -1945,8 +1953,11 @@
if (periodType == DayPeriodRules::DAYPERIOD_AM ||
periodType == DayPeriodRules::DAYPERIOD_PM ||
toAppend->isBogus()) {
- subFormat(appendTo, 0x61, count, capitalizationContext, fieldNum,
- handler, cal, status);
+ // We are passing a different fieldToOutput because we want to add
+ // 'B' to field position iterator. This makes this fallback stable when
+ // there is a data change on locales.
+ subFormat(appendTo, u'a', count, capitalizationContext, fieldNum, u'B', handler, cal, status);
+ return;
}
else {
appendTo += *toAppend;
@@ -1990,7 +2001,7 @@
}
#endif
- handler.addAttribute(fgPatternIndexToDateFormatField[patternCharIndex], beginOffset, appendTo.length());
+ handler.addAttribute(DateFormatSymbols::getPatternCharIndex(fieldToOutput), beginOffset, appendTo.length());
}
//----------------------------------------------------------------------
diff --git a/icu4c/source/i18n/unicode/smpdtfmt.h b/icu4c/source/i18n/unicode/smpdtfmt.h
index 79fa817..b4b0e5f 100644
--- a/icu4c/source/i18n/unicode/smpdtfmt.h
+++ b/icu4c/source/i18n/unicode/smpdtfmt.h
@@ -1274,6 +1274,7 @@
int32_t count,
UDisplayContext capitalizationContext,
int32_t fieldNum,
+ char16_t fieldToOutput,
FieldPositionHandler& handler,
Calendar& cal,
UErrorCode& status) const; // in case of illegal argument
diff --git a/icu4c/source/test/intltest/dtfmttst.cpp b/icu4c/source/test/intltest/dtfmttst.cpp
index 93aa951..d4fca1c 100644
--- a/icu4c/source/test/intltest/dtfmttst.cpp
+++ b/icu4c/source/test/intltest/dtfmttst.cpp
@@ -5579,34 +5579,39 @@
const char16_t timeZone[] = u"PST8PDT";
- UnicodeString skeleton = u"EEEEEBBBBB";
- int32_t count = 0;
- const Locale* locales = Locale::getAvailableLocales(count);
- for (int32_t i = 0; i < count; i++) {
- if (quick && (i % 17) != 0) { continue; }
+ UnicodeString skeletons[] = {u"EEEEEBBBBB", u"EEEEEbbbbb"};
- const Locale locale = locales[i];
- LocalPointer<DateTimePatternGenerator> gen(DateTimePatternGenerator::createInstance(locale, status));
- UnicodeString pattern = gen->getBestPattern(skeleton, status);
+ for (int32_t j = 0; j < 2; j++) {
+ UnicodeString skeleton = skeletons[j];
- SimpleDateFormat dateFormat(pattern, locale, status);
- FieldPositionIterator fpositer;
- UnicodeString result;
- LocalPointer<Calendar> calendar(Calendar::createInstance(TimeZone::createTimeZone(timeZone), status));
- calendar->setTime(UDate(0), status);
- dateFormat.format(*calendar, result, &fpositer, status);
+ int32_t count = 0;
+ const Locale* locales = Locale::getAvailableLocales(count);
+ for (int32_t i = 0; i < count; i++) {
+ if (quick && (i % 17) != 0) { continue; }
- FieldPosition curFieldPosition;
- FieldPosition lastFieldPosition;
- lastFieldPosition.setBeginIndex(-1);
- lastFieldPosition.setEndIndex(-1);
- while(fpositer.next(curFieldPosition)) {
- if (curFieldPosition.getBeginIndex() == lastFieldPosition.getBeginIndex() && curFieldPosition.getEndIndex() == lastFieldPosition.getEndIndex()) {
- if (logKnownIssue("20741")) continue;
- assertEquals("Different fields at same position", 'B', PATTERN_CHARS[lastFieldPosition.getField()]);
+ const Locale locale = locales[i];
+ LocalPointer<DateTimePatternGenerator> gen(DateTimePatternGenerator::createInstance(locale, status));
+ UnicodeString pattern = gen->getBestPattern(skeleton, status);
+
+ SimpleDateFormat dateFormat(pattern, locale, status);
+ FieldPositionIterator fpositer;
+ UnicodeString result;
+ LocalPointer<Calendar> calendar(Calendar::createInstance(TimeZone::createTimeZone(timeZone), status));
+ calendar->setTime(UDate(0), status);
+ dateFormat.format(*calendar, result, &fpositer, status);
+
+ FieldPosition curFieldPosition;
+ FieldPosition lastFieldPosition;
+ lastFieldPosition.setBeginIndex(-1);
+ lastFieldPosition.setEndIndex(-1);
+ while(fpositer.next(curFieldPosition)) {
+ assertFalse("Field missing on pattern", pattern.indexOf(PATTERN_CHARS[curFieldPosition.getField()]) == -1);
+ if (curFieldPosition.getBeginIndex() == lastFieldPosition.getBeginIndex() && curFieldPosition.getEndIndex() == lastFieldPosition.getEndIndex()) {
+ assertEquals("Different fields at same position", PATTERN_CHARS[curFieldPosition.getField()], PATTERN_CHARS[lastFieldPosition.getField()]);
+ }
+
+ lastFieldPosition = curFieldPosition;
}
-
- lastFieldPosition = curFieldPosition;
}
}
}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/ChineseDateFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/ChineseDateFormat.java
index 735fac6..a101504 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/ChineseDateFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/ChineseDateFormat.java
@@ -126,12 +126,13 @@
char ch, int count, int beginOffset,
int fieldNum, DisplayContext capitalizationContext,
FieldPosition pos,
+ char patternCharToOutput,
Calendar cal) {
// Logic to handle 'G' for chinese calendar is moved into SimpleDateFormat,
// and obsolete pattern char 'l' is now ignored in SimpleDateFormat, so we
// just use its implementation
- super.subFormat(buf, ch, count, beginOffset, fieldNum, capitalizationContext, pos, cal);
+ super.subFormat(buf, ch, count, beginOffset, fieldNum, capitalizationContext, pos, patternCharToOutput, cal);
// The following is no longer an issue for this subclass...
// TODO: add code to set FieldPosition for 'G' and 'l' fields. This
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/SimpleDateFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/SimpleDateFormat.java
index 090e956..733e540 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/SimpleDateFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/SimpleDateFormat.java
@@ -1395,10 +1395,10 @@
}
if (useFastFormat) {
subFormat(toAppendTo, item.type, item.length, toAppendTo.length(),
- i, capitalizationContext, pos, cal);
+ i, capitalizationContext, pos, item.type, cal);
} else {
toAppendTo.append(subFormat(item.type, item.length, toAppendTo.length(),
- i, capitalizationContext, pos, cal));
+ i, capitalizationContext, pos, item.type, cal));
}
if (attributes != null) {
// Check the sub format length
@@ -1547,7 +1547,7 @@
throws IllegalArgumentException
{
// Note: formatData is ignored
- return subFormat(ch, count, beginOffset, 0, DisplayContext.CAPITALIZATION_NONE, pos, cal);
+ return subFormat(ch, count, beginOffset, 0, DisplayContext.CAPITALIZATION_NONE, pos, ch, cal);
}
/**
@@ -1561,10 +1561,11 @@
protected String subFormat(char ch, int count, int beginOffset,
int fieldNum, DisplayContext capitalizationContext,
FieldPosition pos,
+ char patternCharToOutput,
Calendar cal)
{
StringBuffer buf = new StringBuffer();
- subFormat(buf, ch, count, beginOffset, fieldNum, capitalizationContext, pos, cal);
+ subFormat(buf, ch, count, beginOffset, fieldNum, capitalizationContext, pos, patternCharToOutput, cal);
return buf.toString();
}
@@ -1586,6 +1587,7 @@
char ch, int count, int beginOffset,
int fieldNum, DisplayContext capitalizationContext,
FieldPosition pos,
+ char patternCharToOutput,
Calendar cal) {
final int maxIntCount = Integer.MAX_VALUE;
@@ -1944,7 +1946,10 @@
if (toAppend == null) {
// Time isn't exactly midnight or noon (as displayed) or localized string doesn't
// exist for requested period. Fall back to am/pm instead.
- subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, cal);
+ // We are passing a different patternCharToOutput because we want to add
+ // 'b' to field position. This makes this fallback stable when
+ // there is a data change on locales.
+ subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, 'b', cal);
} else {
buf.append(toAppend);
}
@@ -1959,8 +1964,11 @@
if (ruleSet == null) {
// Data doesn't exist for the locale we're looking for.
// Fall back to am/pm.
- subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, cal);
- break;
+ // We are passing a different patternCharToOutput because we want to add
+ // 'B' to field position. This makes this fallback stable when
+ // there is a data change on locales.
+ subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, 'B', cal);
+ return;
}
// Get current display time.
@@ -2025,7 +2033,11 @@
if (periodType == DayPeriodRules.DayPeriod.AM ||
periodType == DayPeriodRules.DayPeriod.PM ||
toAppend == null) {
- subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, cal);
+ // We are passing a different patternCharToOutput because we want to add
+ // 'B' to field position. This makes this fallback stable when
+ // there is a data change on locales.
+ subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, 'B', cal);
+ return;
}
else {
buf.append(toAppend);
@@ -2089,12 +2101,13 @@
}
// Set the FieldPosition (for the first occurrence only)
+ int outputCharIndex = getIndexFromChar(patternCharToOutput);
if (pos.getBeginIndex() == pos.getEndIndex()) {
- if (pos.getField() == PATTERN_INDEX_TO_DATE_FORMAT_FIELD[patternCharIndex]) {
+ if (pos.getField() == PATTERN_INDEX_TO_DATE_FORMAT_FIELD[outputCharIndex]) {
pos.setBeginIndex(beginOffset);
pos.setEndIndex(beginOffset + buf.length() - bufstart);
} else if (pos.getFieldAttribute() ==
- PATTERN_INDEX_TO_DATE_FORMAT_ATTRIBUTE[patternCharIndex]) {
+ PATTERN_INDEX_TO_DATE_FORMAT_ATTRIBUTE[outputCharIndex]) {
pos.setBeginIndex(beginOffset);
pos.setEndIndex(beginOffset + buf.length() - bufstart);
}
@@ -4368,10 +4381,10 @@
PatternItem item = (PatternItem)items[i];
if (useFastFormat) {
subFormat(appendTo, item.type, item.length, appendTo.length(),
- i, capSetting, pos, fromCalendar);
+ i, capSetting, pos, item.type, fromCalendar);
} else {
appendTo.append(subFormat(item.type, item.length, appendTo.length(),
- i, capSetting, pos, fromCalendar));
+ i, capSetting, pos, item.type, fromCalendar));
}
}
}
@@ -4386,10 +4399,10 @@
PatternItem item = (PatternItem)items[i];
if (useFastFormat) {
subFormat(appendTo, item.type, item.length, appendTo.length(),
- i, capSetting, pos, toCalendar);
+ i, capSetting, pos, item.type, toCalendar);
} else {
appendTo.append(subFormat(item.type, item.length, appendTo.length(),
- i, capSetting, pos, toCalendar));
+ i, capSetting, pos, item.type, toCalendar));
}
}
}
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DateFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DateFormatTest.java
index 5b28a76..42b9ed3 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DateFormatTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DateFormatTest.java
@@ -5462,30 +5462,36 @@
@Test
public void test20741_ABFields() {
+ String [] skeletons = {"EEEEEBBBBB", "EEEEEbbbbb"};
ULocale[] locales = ULocale.getAvailableLocales();
- for (int i = 0; i < locales.length; i++) {
- ULocale locale = locales[i];
- if (isQuick() && (i % 17 != 0)) continue;
+ for (String skeleton : skeletons) {
+ for (int i = 0; i < locales.length; i++) {
+ ULocale locale = locales[i];
+ if (isQuick() && (i % 17 != 0)) continue;
- DateTimePatternGenerator gen = DateTimePatternGenerator.getInstance(locale);
- String pattern = gen.getBestPattern("EEEEEBBBBB");
- SimpleDateFormat dateFormat = new SimpleDateFormat(pattern, locale);
- Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("PST8PDT"));
- calendar.setTime(new Date(0));
+ DateTimePatternGenerator gen = DateTimePatternGenerator.getInstance(locale);
+ String pattern = gen.getBestPattern(skeleton);
+ SimpleDateFormat dateFormat = new SimpleDateFormat(pattern, locale);
+ Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("PST8PDT"));
+ calendar.setTime(new Date(0));
- FieldPosition pos_c = new FieldPosition(DateFormat.Field.DAY_OF_WEEK);
- dateFormat.format(calendar, new StringBuffer(""), pos_c);
- assertFalse("'Day of week' field was not found", pos_c.getBeginIndex() == 0 && pos_c.getEndIndex() == 0);
+ FieldPosition pos_c = new FieldPosition(DateFormat.Field.DAY_OF_WEEK);
+ dateFormat.format(calendar, new StringBuffer(""), pos_c);
+ assertFalse("'Day of week' field was not found", pos_c.getBeginIndex() == 0 && pos_c.getEndIndex() == 0);
- FieldPosition pos_B = new FieldPosition(DateFormat.Field.FLEXIBLE_DAY_PERIOD);
- dateFormat.format(calendar, new StringBuffer(""), pos_B);
- assertFalse("'Flexible day period' field was not found", pos_B.getBeginIndex() == 0 && pos_B.getEndIndex() == 0);
+ if (skeleton.equals("EEEEEBBBBB")) {
+ FieldPosition pos_B = new FieldPosition(DateFormat.Field.FLEXIBLE_DAY_PERIOD);
+ dateFormat.format(calendar, new StringBuffer(""), pos_B);
+ assertFalse("'Flexible day period' field was not found", pos_B.getBeginIndex() == 0 && pos_B.getEndIndex() == 0);
+ } else {
+ FieldPosition pos_b = new FieldPosition(DateFormat.Field.AM_PM_MIDNIGHT_NOON);
+ dateFormat.format(calendar, new StringBuffer(""), pos_b);
+ assertFalse("'AM/PM/Midnight/Noon' field was not found", pos_b.getBeginIndex() == 0 && pos_b.getEndIndex() == 0);
+ }
- FieldPosition pos_a = new FieldPosition(DateFormat.Field.AM_PM);
- dateFormat.format(calendar, new StringBuffer(""), pos_a);
- if (pos_B.getBeginIndex() == pos_a.getBeginIndex() && pos_B.getEndIndex() == pos_a.getEndIndex()) {
- if (logKnownIssue("20741", "Format Fields reports same field position as \"a\" and \"B\"")) continue;
- fail("Duplicated field found");
+ FieldPosition pos_a = new FieldPosition(DateFormat.Field.AM_PM);
+ dateFormat.format(calendar, new StringBuffer(""), pos_a);
+ assertTrue("'AM/PM' field was found", pos_a.getBeginIndex() == 0 && pos_a.getEndIndex() == 0);
}
}
}
