Skip to content

Commit

Permalink
ICU-22434 Not calling setFirstDayOfWeek(MONDAY) if the locale has fw
Browse files Browse the repository at this point in the history
The Calendar constructor already take care of the fw override.
We should not set the first day of week for iso8601 to Monday if
we have a fw keyword/type in the locale.

ICU-22434 Fix incorrect calendar keyword extraction
  • Loading branch information
FrankYFTang committed Jul 13, 2023
1 parent 034a808 commit 5826bf7
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 24 deletions.
19 changes: 1 addition & 18 deletions icu4c/source/i18n/calendar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,20 +247,6 @@ static UBool isStandardSupportedKeyword(const char *keyword, UErrorCode& status)
return (calType != CALTYPE_UNKNOWN);
}

// only used with service registration.
static void getCalendarKeyword(const UnicodeString &id, char *targetBuffer, int32_t targetBufferSize) {
UnicodeString calendarKeyword = UNICODE_STRING_SIMPLE("calendar=");
int32_t calKeyLen = calendarKeyword.length();
int32_t keyLen = 0;

int32_t keywordIdx = id.indexOf((char16_t)0x003D); /* '=' */
if (id[0] == 0x40/*'@'*/
&& id.compareBetween(1, keywordIdx+1, calendarKeyword, 0, calKeyLen) == 0)
{
keyLen = id.extract(keywordIdx+1, id.length(), targetBuffer, targetBufferSize, US_INV);
}
targetBuffer[keyLen] = 0;
}
#endif

static ECalType getCalendarTypeForLocale(const char *locid) {
Expand Down Expand Up @@ -458,10 +444,7 @@ class BasicCalendarFactory : public LocaleKeyFactory {
lkey->canonicalLocale(canLoc);

char keyword[ULOC_FULLNAME_CAPACITY];
UnicodeString str;

key.currentID(str);
getCalendarKeyword(str, keyword, (int32_t) sizeof(keyword));
curLoc.getKeywordValue("calendar", keyword, (int32_t) sizeof(keyword), status);

#ifdef U_DEBUG_CALSVC
fprintf(stderr, "BasicCalendarFactory::create() - cur %s, can %s\n", (const char*)curLoc.getName(), (const char*)canLoc.getName());
Expand Down
8 changes: 7 additions & 1 deletion icu4c/source/i18n/iso8601cal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ UOBJECT_DEFINE_RTTI_IMPLEMENTATION(ISO8601Calendar)
ISO8601Calendar::ISO8601Calendar(const Locale& aLocale, UErrorCode& success)
: GregorianCalendar(aLocale, success)
{
setFirstDayOfWeek(UCAL_MONDAY);
UErrorCode fwStatus = U_ZERO_ERROR;
int32_t fwLength = aLocale.getKeywordValue("fw", nullptr, 0, fwStatus);
// Do not set first day of week for iso8601 to Monday if we have fw keyword
// and let the value set by the Calendar constructor to take care of it.
if (U_SUCCESS(fwStatus) && fwLength == 0) {
setFirstDayOfWeek(UCAL_MONDAY);
}
setMinimalDaysInFirstWeek(4);
}

Expand Down
32 changes: 32 additions & 0 deletions icu4c/source/test/cintltst/ccaltst.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ void TestJpnCalAddSetNextEra(void);
void TestUcalOpenBufferRead(void);
void TestGetTimeZoneOffsetFromLocal(void);

void TestFWWithISO8601(void);

void addCalTest(TestNode** root);

void addCalTest(TestNode** root)
Expand All @@ -68,6 +70,7 @@ void addCalTest(TestNode** root)
addTest(root, &TestJpnCalAddSetNextEra, "tsformat/ccaltst/TestJpnCalAddSetNextEra");
addTest(root, &TestUcalOpenBufferRead, "tsformat/ccaltst/TestUcalOpenBufferRead");
addTest(root, &TestGetTimeZoneOffsetFromLocal, "tsformat/ccaltst/TestGetTimeZoneOffsetFromLocal");
addTest(root, &TestFWWithISO8601, "tsformat/ccaltst/TestFWWithISO8601");
}

/* "GMT" */
Expand Down Expand Up @@ -2794,4 +2797,33 @@ TestGetTimeZoneOffsetFromLocal() {
ucal_close(cal);
}

void
TestFWWithISO8601() {
/* UCAL_SUNDAY is 1, UCAL_MONDAY is 2, ..., UCAL_SATURDAY is 7 */
const char* LOCALES[] = {
"",
"en-u-ca-iso8601-fw-sun",
"en-u-ca-iso8601-fw-mon",
"en-u-ca-iso8601-fw-tue",
"en-u-ca-iso8601-fw-wed",
"en-u-ca-iso8601-fw-thu",
"en-u-ca-iso8601-fw-fri",
"en-u-ca-iso8601-fw-sat",
};
for (int32_t i = UCAL_SUNDAY; i <= UCAL_SATURDAY; i++) {
const char* locale = LOCALES[i];
UErrorCode status = U_ZERO_ERROR;
UCalendar* cal = ucal_open(0, 0, locale, UCAL_TRADITIONAL, &status);
if(U_FAILURE(status)){
log_data_err("FAIL: error in ucal_open caldef : %s\n - (Are you missing data?)", u_errorName(status));
}
int32_t actual = ucal_getAttribute(cal, UCAL_FIRST_DAY_OF_WEEK);
if (i != actual) {
log_err("ERROR: ucal_getAttribute(\"%s\", UCAL_FIRST_DAY_OF_WEEK) should be %d but get %d\n",
locale, i, actual);
}
ucal_close(cal);
}
}

#endif /* #if !UCONFIG_NO_FORMATTING */
29 changes: 28 additions & 1 deletion icu4c/source/test/intltest/caltest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,9 @@ void CalendarTest::runIndexedTest( int32_t index, UBool exec, const char* &name,
TESTCASE_AUTO(TestActualLimitsOrdinalMonth);
TESTCASE_AUTO(TestChineseCalendarMonthInSpecialYear);
TESTCASE_AUTO(TestClearMonth);


TESTCASE_AUTO(TestFWWithISO8601);

TESTCASE_AUTO_END;
}

Expand Down Expand Up @@ -5494,6 +5496,31 @@ void CalendarTest::TestChineseCalendarMonthInSpecialYear() {
}
}
}

void CalendarTest::TestFWWithISO8601() {
// ICU UCAL_SUNDAY is 1, UCAL_MONDAY is 2, ... UCAL_SATURDAY is 7.
const char *locales[] = {
"",
"en-u-ca-iso8601-fw-sun",
"en-u-ca-iso8601-fw-mon",
"en-u-ca-iso8601-fw-tue",
"en-u-ca-iso8601-fw-wed",
"en-u-ca-iso8601-fw-thu",
"en-u-ca-iso8601-fw-fri",
"en-u-ca-iso8601-fw-sat"
};
for (int i = UCAL_SUNDAY; i <= UCAL_SATURDAY; i++) {
UErrorCode status = U_ZERO_ERROR;
const char* locale = locales[i];
LocalPointer<Calendar> cal(
Calendar::createInstance(
Locale(locale), status), status);
if (failure(status, "Constructor failed")) continue;
std::string msg("Calendar::createInstance(\"");
msg = msg + locale + "\")->getFirstDayOfWeek()";
assertEquals(msg.c_str(), i, cal->getFirstDayOfWeek());
}
}
#endif /* #if !UCONFIG_NO_FORMATTING */

//eof
2 changes: 2 additions & 0 deletions icu4c/source/test/intltest/caltest.h
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,8 @@ class CalendarTest: public CalendarTimeZoneTest {
void TestLimitsOrdinalMonth();
void TestActualLimitsOrdinalMonth();

void TestFWWithISO8601();

void RunChineseCalendarInTemporalLeapYearTest(Calendar* cal);
void RunIslamicCalendarInTemporalLeapYearTest(Calendar* cal);
void Run366DaysIsLeapYearCalendarInTemporalLeapYearTest(Calendar* cal);
Expand Down
9 changes: 8 additions & 1 deletion icu4j/main/classes/core/src/com/ibm/icu/util/Calendar.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.text.StringCharacterIterator;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.Date;
import java.util.Locale;
import java.util.List;
import java.util.MissingResourceException;

import com.ibm.icu.impl.CalType;
Expand Down Expand Up @@ -1831,7 +1833,12 @@ private static Calendar createInstance(ULocale locale) {
case ISO8601:
// Only differs week numbering rule from Gregorian
cal = new GregorianCalendar(zone, locale);
cal.setFirstDayOfWeek(MONDAY);
String type = locale.getUnicodeLocaleType("fw");
// Only set fw to Monday for ISO8601 if there aer no fw keyword.
// If there is a fw keyword, the Calendar constructor already set it to the fw value.
if (locale.getKeywordValue("fw") == null) {
cal.setFirstDayOfWeek(MONDAY);
}
cal.setMinimalDaysInFirstWeek(4);
break;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2654,15 +2654,31 @@ public void TestRespectUExtensionFw() { // ICU-22226
"en-US-u-fw-sun",
"en-US-u-fw-mon",
"en-US-u-fw-thu",
"en-US-u-fw-sat"
"en-US-u-fw-sat",
// ICU-22434
"en-US-u-ca-iso8601-fw-sun",
"en-US-u-ca-iso8601-fw-mon",
"en-US-u-ca-iso8601-fw-tue",
"en-US-u-ca-iso8601-fw-wed",
"en-US-u-ca-iso8601-fw-thu",
"en-US-u-ca-iso8601-fw-fri",
"en-US-u-ca-iso8601-fw-sat",
};
int[] expectedValues = {
Calendar.SUNDAY,
Calendar.SUNDAY,
Calendar.SUNDAY,
Calendar.MONDAY,
Calendar.THURSDAY,
Calendar.SATURDAY
Calendar.SATURDAY,
// ICU-22434
Calendar.SUNDAY,
Calendar.MONDAY,
Calendar.TUESDAY,
Calendar.WEDNESDAY,
Calendar.THURSDAY,
Calendar.FRIDAY,
Calendar.SATURDAY,
};

assertEquals(
Expand All @@ -2672,7 +2688,8 @@ public void TestRespectUExtensionFw() { // ICU-22226

for (int i = 0; i < localeIds.length; i++) {
assertEquals(
"Calendar.getFirstDayOfWeek() does not seem to respect fw extension u in locale id",
"Calendar.getFirstDayOfWeek() does not seem to respect fw extension u in locale id " +
localeIds[i],
expectedValues[i],
Calendar.getInstance(Locale.forLanguageTag(localeIds[i])).getFirstDayOfWeek());
}
Expand Down

0 comments on commit 5826bf7

Please sign in to comment.