1864612 - ICU 74 update breaks common date format hack

Status: RESOLVED FIXED

(Web Compatibility :: Site Reports, defect)

Description

[Jan-Erik Rediger [:janerik]]

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.

Comment 1

[Jan-Erik Rediger [:janerik]]

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

Comment 2

[André Bargull [:anba]]

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].

Comment 3

[Dennis Schubert [:denschub]]

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.. :/

Comment 4

[Dennis Schubert [:denschub]]

[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.

Comment 5

[Dennis Schubert [:denschub]]

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.

Comment 6

[Dominik Bay [:eimann]]

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.

Comment 7

[André Bargull [:anba]]

(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":

• https://github.com/unicode-org/cldr/blob/dace6c6b5672cbc70d6135f9ad82107b59142a61/common/main/sv.xml#L2976
• https://github.com/unicode-org/cldr/blob/dace6c6b5672cbc70d6135f9ad82107b59142a61/common/main/root.xml#L1423

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

Comment 8

[André Bargull [:anba]]

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.

Comment 9

[André Bargull [:anba]]

(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.

Comment 10

[Dennis Schubert [:denschub]]

Thank you for the fast analysis, André!

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

Comment 11

[Ryan VanderMeulen [: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."

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 13

[Mathias Bynens]

(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