Closed Bug 1864612 Opened 8 months ago Closed 8 months ago

ICU 74 update breaks common date format hack

Categories

(Web Compatibility :: Site Reports, defect)

defect

Tracking

(Webcompat Priority:P1, firefox-esr115 unaffected, firefox119 unaffected, firefox120 unaffected, firefox121+ fixed)

RESOLVED FIXED
Webcompat Priority P1
Tracking Status
firefox-esr115 --- unaffected
firefox119 --- unaffected
firefox120 --- unaffected
firefox121 + fixed

People

(Reporter: janerik, Unassigned)

References

(Regression)

Details

(Keywords: regression)

It seems the ICU update from bug 1859752 changes some date formatting APIs, which in turn leads to a breakage on a website as they expect proper ISO dates (YYYY-MM-DD) in their API request, but get YYYY-M-D formatting.
Unfortunately the precise failure that happens is behind a login; they do have 14-day free accounts (but it also requires a connected bank account I guess). I'm happy to assist using my existing account though.

STR:

  1. Go to https://$youraccountsubdomain.billomat.net/app/beta/banking-next
  2. Login

What should happen:

The site fetches information and shows a listing of all bank accounts and their transactions.

What happens:

The site shows "Error".
A post request to /graphql returns (trimmed for readability):

{
  "errors": [
    {
      "message": "Variable $query of type SearchAttributes was provided invalid value for filters.and.dateFilter.0.value (Could not coerce value \"2000-1-1\" to ISO8601DateTime)",
      "extensions": {
        "code": "DOWNSTREAM_SERVICE_ERROR",
        "serviceName": "liquidity",
        "value": {
          "filters": {
            "and": {
              "dateFilter": [
                {
                  "operator": "from",
                  "value": "2000-1-1"
                },
                {
                  "operator": "to",
                  "value": "2023-11-14"
                }
              ],
              "transactionType": "all",
              "isHiddenFilter": false,
              "amountsFilter": []
            }
          },
 <snip>
}

A valid request in older Firefox, Chrome or Safari contains this in the request:

"query": {
            "filters": {
                "and": {
                    "dateFilter": [
                        {
                            "operator": "from",
                            "value": "2000-01-01"
                        },
                        {
                            "operator": "to",
                            "value": "2023-11-14"
                        }
                    ],
                    "transactionType": "all",
                    "isHiddenFilter": false,
                    "amountsFilter": []
                }
            },

whereas the Firefox versions with the ICU update contain "value": "2000-1-1".
I found the regression using mozregression between 2023-11-01 and 2023-11-13.
I couldn't find exactly what formatting API they use though, so I'm not able to extract a minimal reproduction right now.

My locales:

Requested: en-US,
Available: en-US,
App: en-US,
Regional pref: en-DE
Default: en-US

OS:
System: en-DE
Regional pref: en-DE

new Intl.DateTimeFormat("sv-SE", {}).format(0) returns:

ICU73/CLDR43 ICU74/CLDR44
"1970-01-01" "1970-1-1"

(sv-SE locale because of misuses promoted by code like this one: https://stackoverflow.com/questions/25050034/get-iso-8601-using-intl-datetimeformat.)

A correct way to print the date in ISO-style is for example: new Date(0).toISOString().split("T")[0].

André, what options do we have to mitigate this?

As you correctly stated, this is, unfortunately, a common(ly wrong) pattern. But it looks like we're the first (and only?) browser shipping this, as Chrome hasn't even started working on that update. We can't be the first to break the web like this - users will use "it works in Chrome" as a reason to switch browsers, and site/library authors will use "it works in Chrome" as a reason to de-prioritize our outreach attempts. I also wouldn't be surprised if, just like last time, Chrome also reverts to find an in-engine workaround here.. :/

Flags: needinfo?(andrebargull)

[Tracking Requested - why for this release]:

As outlined in comment 3, we're the only browser shipping the update, and it breaks a common date format hack. Even though WebDevs are technically wrong on this one, we can't risk this turning into a similar incident to bug 1806042.

Webcompat Priority: --- → P1
Summary: Wrong date formatting leads to broken website after ICU update → ICU 74 update breaks common date format hack

And just to confirm the cause of Jan-Erik's original report: they're indeed using a date localization hack, except that they're using lt-LT as the target locale, and frequently call .toLocaleDateString("lt-LT") on already existing date objects to get the fake-ISO-string. .toISOString().split("T")[0] will work in all the cases I found, and I'll make sure that they're aware of that.

Regardless of that, this is almost impossible to ship interventions for as we'd have to proxy lots of the Date object and the Intl API for that to be reliable, so we probably still can't ship the ICU update. I'm sure other breakages will pop up if this hits beta/release.

Thanks Dennis. I'll forward this info to the Billomat Dev team. We are a paying customer with them and it breaks our ability to use their product. So it's up to them to fix it.

(In reply to Dennis Schubert [:denschub] from comment #3)

André, what options do we have to mitigate this?

It looks like the CLDR resources are actually still using the zero-padded format "y-MM-dd":

So it's possible that the issue is within ICU source code itself when resolving the date-time pattern from the input skeleton.

Okay, it looks like ICU doesn't correctly implement inherited resources (CLDR marker ↑↑↑):

sv.xml has this line:

<dateFormatItem id="yMd">↑↑↑</dateFormatItem>

And root.xml has:

<dateFormatItem id="yMd">y-MM-dd</dateFormatItem>

But additionally sv.xml has:

<dateFormatLength type="short">
	<dateFormat>
		<pattern>↑↑↑</pattern>
		<datetimeSkeleton draft="contributed">↑↑↑</datetimeSkeleton>
	</dateFormat>
</dateFormatLength>

And root.xml:

<dateFormatLength type="short">
	<dateFormat>
		<pattern>y-MM-dd</pattern>
		<datetimeSkeleton>yMMdd</datetimeSkeleton>
	</dateFormat>
</dateFormatLength>

The <dateFormat> entry is copied into the sv.txt ICU resource data:

"y-MM-dd",

But the various <dateFormatItem> entries aren't copied into the sv.txt ICU resource file.

When ICU initialises the DateTime pattern generator, it first adds the <dateFormat> entries and then the <availableFormats> entries. The <availableFormats> entries are processed from child to parent resources. When any duplicate, conflicting resources are found, entries from the parent locale are discarded in DateTimePatternGenerator::addPatternWithSkeleton(...).

So "y-MM-dd" from <dateFormat> is added first and later yMd{"y-MM-dd"} from root is added. Both resources use the same pattern string "y-MM-dd", so it gets flagged as a conflict. And for the root locale entry, override = true and skeletonToUse != nullptr is true, so this condition evaluates to true:

if (duplicatePattern != nullptr && (!entryHadSpecifiedSkeleton || (skeletonToUse != nullptr && !override))) {

And because override = true, the method returns here and the root locale entry isn't added.

Furthermore the <dateFormat> entry has no specified skeleton, so when later on the raw pattern is resolved in DateTimePatternGenerator::getBestPattern(...), the specifiedSkeleton variable is nullptr, which in turn leads to adjusting the raw pattern to use the input field widths, because this block isn't executed in DateTimePatternGenerator::adjustFieldTypes(...).

And the input field widths are specified to use numeric widths and not 2-digit widths, cf. CreateDateTimeFormat, steps 46.d.

Flags: needinfo?(andrebargull)

(In reply to Dennis Schubert [:denschub] from comment #5)

Regardless of that, this is almost impossible to ship interventions for as we'd have to proxy lots of the Date object and the Intl API for that to be reliable, so we probably still can't ship the ICU update. I'm sure other breakages will pop up if this hits beta/release.

I think we have to revert bug 1859752.

Bug 1863960 will need to be reverted, too, because it assumes Unicode 15.1 / CLDR 44 is supported. Not sure if there are already more bugs which also need to be reverted.

Thank you for the fast analysis, André!

Ryan, can you backout bug 1859752 and bug 1863960 as suggested?

Flags: needinfo?(ryanvm)

Redirecting to the right team for backouts. But I do wonder if we need a Try push first to ensure that nothing else is going to break RE: "Not sure if there are already more bugs which also need to be reverted."

Flags: needinfo?(ryanvm) → needinfo?(aryx.bugmail)

The changes in bug 1859752 and bug 1863960 have been backed out. Please reopen this bug if more work shall be tracked here.

Status: NEW → RESOLVED
Closed: 8 months ago
Flags: needinfo?(aryx.bugmail)
Resolution: --- → FIXED

(In reply to André Bargull [:anba] from comment #2)

new Intl.DateTimeFormat("sv-SE", {}).format(0) returns:

ICU73/CLDR43 ICU74/CLDR44
"1970-01-01" "1970-1-1"

(sv-SE locale because of misuses promoted by code like this one: https://stackoverflow.com/questions/25050034/get-iso-8601-using-intl-datetimeformat.)

A correct way to print the date in ISO-style is for example: new Date(0).toISOString().split("T")[0].

This would be a good addition to the locale-compat test in https://github.com/web-platform-tests/wpt/tree/master/ecmascript. PR: https://github.com/web-platform-tests/wpt/pull/43257

You need to log in before you can comment on or make changes to this bug.