Possible bug in parseFromEnUsFormat on some IE configurations

[sophiebits]

I was recently reviewing this bug in the Luxon library where their logic to parse an en-US–formatted time string in order to figure out time zones fails in some cases:

• IE - hackyoffset broken if computer date format does not include "/" moment/luxon#849

As I was perusing the source of this polyfill I noticed that parseFromEnUsFormat may have the same issue.

temporal-polyfill/lib/ecmascript.ts

Lines 3422 to 3431 in a44a1bb

	export function parseFromEnUsFormat(datetime: string) {
	const parts = datetime.split(/[^\w]+/);
	if (parts.length !== 7) {
	throw new RangeError(`expected 7 parts in "${datetime}`);
	}
	const month = +parts[0];
	const day = +parts[1];
	let year = +parts[2];

Specifically, it seems that assuming parts[0], parts[1], parts[2] correspond to month, day, year may not always be correct. I haven't been able to confirm for myself so feel free to close if this isn't helpful but thought I'd err on the side of reporting.

[justingrant]

Hi @sophiebits! I'm a big fan of your long-term contributions to React and the rest of the JS ecosystem. Thanks so much for reporting this interesting case. This polyfill is likely vulnerable to the same issue.

I'm not sure that anyone has ever tested our polyfill in IE11, so I wouldn't be surprised if there are edge cases like the one above (or even non-edge cases!) that fail in ancient browsers. So before fixing this bug, we'd probably want to have someone figure out how to run Test262 on IE11 and see which other things fail too.

@12wrigja, what's Google's position on IE support for this polyfill? Is it required, and if so is anyone testing it in IE?

The latest data I could find shows IE at 0.12% market share (source) as of August 2023. Probably down to 0.1% or less by now. And to trigger this bug, if I understand the linked issue correctly, a user would have to override their system settings for the "English (United States)" Windows locale instead of setting their Windows system locale to "English (Canada)", "English (UK)", etc. So the actual number of IE users affected by this is likely to be pretty small, and (again if my understanding of Windows' regional configuration is correct, which it may not be) the workaround would be to change the system locale to the appropriate country instead of United States.

If anyone does want to try to fix this, here's some more info:

To Repro:

1. go to Windows Regional Settings (see screenshot below), set Format to "English (United States)" and edit the "Short date" to a different order than the default M/d/yyyy. I don't have a Windows VM handy at the moment so I'm not sure what's lurking in that dropdown, but yyyy-MM-dd seems most likely to break things loudly. d/M/yyyy will be the sneakiest format because for days <= 12, because month and day would both be valid.

2. Run this code in IE:

Temporal.ZonedDateTime.from('2023-12-20[America/Los_Angeles]').day === 20
// expected: true

If that line returns false or crashes, then the bug exists.

For actually fixing the bug, I assume the best way to do it would be to run some code that uses a known date (3 days before epoch might be convenient to differentiate M/d/yyyy from d/M/yyyy) and checks what format is returned by Intl.DateTimeFormat.p.format, and then adjusts parseFromEnUsFormat to use the format that IE actually returns.

I'm not sure we'd be willing to accept a PR with this fix. Kinda depends on additional bundle size, and whether IE actually works otherwise. I'd be concerned about opening a floodgate of IE issues given its tiny market share. But if the fix is small and IE11 works otherwise, then I'd maybe be willing to consider it.

[12wrigja]

My current team's position is that we would like to be able to support IE11, simply because we can't confirm that every client team of ours doesn't need IE11 support. Some teams are more willing to drop IE11 (e.g. Workspace) than others.

That being said, we don't run test262 against this polyfill on IE11 internally, because trying to align test262 with our internal web testing infrastructure was already difficult enough without trying to get it to run in IE :) It is fairly low down on my to-do list.

I'm fairly certain I can get access to a Windows VM that runs IE11 and test this out and report back.

I agree w/ Justin that if the fix is fairly small (both code-size and runtime performance-wise) then it would be good to accept. My hope is there's just not many users of IE11 who are also changing these settings.

[12wrigja]

Sadly I was wrong about having easy access to an IE11 VM, at least one I can directly interact with to change the region settings as outlined above.

If there is a way to repro this bug without changing Windows settings (just by running JS code in IE11) I can probably test that out fairly easily.

[justingrant]

I think the right next step is to figure out a way to run Test262 against IE11. If IE11 doesn't work for many, many use cases, then the issue reported here (which requires users to manually override some relatively-obscure settings) is probably a pretty low priority.

But if IE11 basically works for mainstream cases, then testing and resolving the issue reported here seems more reasonable to think about.

Regardless, I marked this issue as Help Wanted, so if someone with IE11 access is motivated enough to fix it, then we'll consider the PR!