From 5826bf7ed7f45afd18793c491313b86e2b9385ba Mon Sep 17 00:00:00 2001 From: Frank Tang Date: Thu, 6 Jul 2023 11:52:23 -0700 Subject: [PATCH] ICU-22434 Not calling setFirstDayOfWeek(MONDAY) if the locale has fw 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 --- icu4c/source/i18n/calendar.cpp | 19 +---------- icu4c/source/i18n/iso8601cal.cpp | 8 ++++- icu4c/source/test/cintltst/ccaltst.c | 32 +++++++++++++++++++ icu4c/source/test/intltest/caltest.cpp | 29 ++++++++++++++++- icu4c/source/test/intltest/caltest.h | 2 ++ .../core/src/com/ibm/icu/util/Calendar.java | 9 +++++- .../test/calendar/CalendarRegressionTest.java | 23 +++++++++++-- 7 files changed, 98 insertions(+), 24 deletions(-) diff --git a/icu4c/source/i18n/calendar.cpp b/icu4c/source/i18n/calendar.cpp index d8d544581ea..b9972086c65 100644 --- a/icu4c/source/i18n/calendar.cpp +++ b/icu4c/source/i18n/calendar.cpp @@ -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) { @@ -458,10 +444,7 @@ protected: 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()); diff --git a/icu4c/source/i18n/iso8601cal.cpp b/icu4c/source/i18n/iso8601cal.cpp index 1bc81fac15e..c3288bc6b5d 100644 --- a/icu4c/source/i18n/iso8601cal.cpp +++ b/icu4c/source/i18n/iso8601cal.cpp @@ -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); } diff --git a/icu4c/source/test/cintltst/ccaltst.c b/icu4c/source/test/cintltst/ccaltst.c index ae8acb4dbc4..b61855622c1 100644 --- a/icu4c/source/test/cintltst/ccaltst.c +++ b/icu4c/source/test/cintltst/ccaltst.c @@ -44,6 +44,8 @@ void TestJpnCalAddSetNextEra(void); void TestUcalOpenBufferRead(void); void TestGetTimeZoneOffsetFromLocal(void); +void TestFWWithISO8601(void); + void addCalTest(TestNode** root); void addCalTest(TestNode** root) @@ -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" */ @@ -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 */ diff --git a/icu4c/source/test/intltest/caltest.cpp b/icu4c/source/test/intltest/caltest.cpp index 217fc692fed..92fa2c2d7fb 100644 --- a/icu4c/source/test/intltest/caltest.cpp +++ b/icu4c/source/test/intltest/caltest.cpp @@ -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; } @@ -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 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 diff --git a/icu4c/source/test/intltest/caltest.h b/icu4c/source/test/intltest/caltest.h index bc23702dc91..88f320fd0f0 100644 --- a/icu4c/source/test/intltest/caltest.h +++ b/icu4c/source/test/intltest/caltest.h @@ -330,6 +330,8 @@ public: // package void TestLimitsOrdinalMonth(); void TestActualLimitsOrdinalMonth(); + void TestFWWithISO8601(); + void RunChineseCalendarInTemporalLeapYearTest(Calendar* cal); void RunIslamicCalendarInTemporalLeapYearTest(Calendar* cal); void Run366DaysIsLeapYearCalendarInTemporalLeapYearTest(Calendar* cal); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/util/Calendar.java b/icu4j/main/classes/core/src/com/ibm/icu/util/Calendar.java index 038d483f99c..3893a9796dd 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/util/Calendar.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/util/Calendar.java @@ -12,9 +12,11 @@ import java.io.ObjectInputStream; 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; @@ -1831,7 +1833,12 @@ public abstract class Calendar implements Serializable, Cloneable, Comparable