Skip to content

IntlDateTimeFormat: only pass -u-hc- to UDateIntervalFormat when explicitly requested #366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 1 commit into
base: main
Choose a base branch
from
Open

IntlDateTimeFormat: only pass -u-hc- to UDateIntervalFormat when explicitly requested #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,7 @@ void IntlDateTimeFormat::initializeDateTimeFormat(JSGlobalObject* globalObject,
}

hourCycle = parseHourCycle(resolved.extensions[static_cast<unsigned>(RelevantExtensionKey::Hc)]);
impl->m_explicitHourCycle = hourCycle;
impl->m_numberingSystem = resolved.extensions[static_cast<unsigned>(RelevantExtensionKey::Nu)];
impl->m_dataLocale = resolved.dataLocale;

Expand Down Expand Up @@ -1546,16 +1547,27 @@ UDateIntervalFormat* IntlDateTimeFormat::createDateIntervalFormatIfNecessary(JSG

// While the pattern is including right HourCycle patterns, UDateIntervalFormat does not follow.
// We need to enforce HourCycle by setting "hc" extension if it is specified.
//
// We only append "hc" when it was explicitly resolved from the input locale's -u-hc- extension
// or the hourCycle option (m_explicitHourCycle). m_hourCycle is derived from the generated
// pattern and is always populated when an hour field is present, so using it here would append
// a redundant -u-hc- that merely restates the locale default. In ICU < 76 that redundant
// extension triggers ICU-22669: DateTimePatternGenerator::addICUPatterns goes through
// DateFormat::createTimeInstance, which regenerates the style patterns from skeletons when an
// hc keyword is present and loses locale-specific literal separators (ja "H時mm分ss秒" becomes
// "H:mm:ss"), so formatRange and format would disagree. The skeleton we pass already encodes
// the 12/24-hour choice, and the locale default covers the h11/h12 (and h23/h24) preference,
// so an explicit -u-hc- is only needed to override that preference.
StringBuilder localeBuilder;
localeBuilder.append(m_impl->m_dataLocale);
if (!m_impl->m_calendar.isNull() || !m_impl->m_numberingSystem.isNull() || m_impl->m_hourCycle != HourCycle::None) {
if (!m_impl->m_calendar.isNull() || !m_impl->m_numberingSystem.isNull() || m_impl->m_explicitHourCycle != HourCycle::None) {
localeBuilder.append("-u"_s);
if (!m_impl->m_calendar.isNull())
localeBuilder.append("-ca-"_s, m_impl->m_calendar);
if (!m_impl->m_numberingSystem.isNull())
localeBuilder.append("-nu-"_s, m_impl->m_numberingSystem);
if (m_impl->m_hourCycle != HourCycle::None)
localeBuilder.append("-hc-"_s, hourCycleString(m_impl->m_hourCycle));
if (m_impl->m_explicitHourCycle != HourCycle::None)
localeBuilder.append("-hc-"_s, hourCycleString(m_impl->m_explicitHourCycle));
}
CString dataLocaleWithExtensions = localeBuilder.toString().utf8();

Expand Down Expand Up @@ -2433,8 +2445,9 @@ IntlDateTimeFormat::createTemporalIntervalFormat(UDateFormat* tempFormat, Tempor

StringBuilder localeBuilder;
localeBuilder.append(m_impl->m_dataLocale, "-u-ca-"_s, ensureCalendar(), "-nu-"_s, ensureNumberingSystem());
if (m_impl->m_hourCycle != HourCycle::None)
localeBuilder.append("-hc-"_s, hourCycleString(m_impl->m_hourCycle));
// See createDateIntervalFormatIfNecessary for why this uses m_explicitHourCycle, not m_hourCycle.
if (m_impl->m_explicitHourCycle != HourCycle::None)
localeBuilder.append("-hc-"_s, hourCycleString(m_impl->m_explicitHourCycle));
CString localeWithExt = localeBuilder.toString().utf8();

return std::unique_ptr<UDateIntervalFormat, UDateIntervalFormatDeleter>(
Expand Down
6 changes: 6 additions & 0 deletions Source/JavaScriptCore/runtime/IntlDateTimeFormat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,12 @@ class IntlDateTimeFormatImpl : public RefCounted<IntlDateTimeFormatImpl> {
// primary used for ICU formatting.
String m_timeZoneForResolvedOptions;
IntlDateTimeFormat::HourCycle m_hourCycle { IntlDateTimeFormat::HourCycle::None };
// The hour cycle resolved from the locale's -u-hc- extension and the hourCycle option,
// before any value is derived from the generated pattern. None when neither was specified.
// m_hourCycle above is derived from the final pattern and is always populated when an hour
// field is present; this one is only populated on explicit request and is what we pass as
// -u-hc- when opening the UDateIntervalFormat. See ICU-22669.
IntlDateTimeFormat::HourCycle m_explicitHourCycle { IntlDateTimeFormat::HourCycle::None };
IntlDateTimeFormat::Weekday m_weekday { IntlDateTimeFormat::Weekday::None };
IntlDateTimeFormat::Era m_era { IntlDateTimeFormat::Era::None };
IntlDateTimeFormat::Year m_year { IntlDateTimeFormat::Year::None };
Expand Down
Loading