Consistent Timezone Abbreviations #5002
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Switched `formatLocal` over to use `localTimeZoneAbbr` - `localTimeZone` only produces abbreviated time zones in some browsers - This browser behaviour is implementation specific in the spec - Removed now-unused `localTimeZone`
⚡️ Lighthouse report for the changes in this PRLighthouse tested 2 URLs Report for Article
Report for Front
|
JamieB-gu
requested review from
a team,
marsavar and
iainjchambers-guardian
as code owners
DavidLawes
approved these changes
May 25, 2022
|
"This PR is stale because it has been open 30 days with no activity. Unless a comment is added or the “stale” label removed, this will be closed in 3 days" |
|
This PR was closed because it has been stalled for 3 days with no activity. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why?
TL;DR Spec-compliant differences in browser implementations of the Date API resulted in different variations of the timezone name on Android and iOS.
Publication Timestamp
The publication timestamp at the top of articles looks something like this:
For apps this timestamp and the corresponding timezone are in the user's local time, and so both the datetime and the timezone name can have a large number of values. We calculate this on the user's device rather than setting it on the server. This differs from dotcom, which I believe uses a handful of timezones based on edition, calculated on the server.
The Bug
We've had a long-standing issue where the publication timestamp at the top of AR articles has sometimes shown the full timezone name, rather than an abbreviation. This means users see "British Summer Time" rather than "BST". This is unusual enough that it's previously been reported as a bug.
It turns out that "sometimes" is actually platform specific. Android users are seeing the full timezone, iOS users are seeing the abbreviation. It also transpires that if you view AR in a normal browser, Safari shows the abbreviated timezone but Firefox shows the long-form version. This suggests that Chrome (Android webviews) and Firefox are displaying timezones one way, and Safari (iOS webviews) are displaying them another.
What's Going On?
For the web publication date, AR has been using JavaScript's
Date.toTimeString()method and a regex to retrieve the timezone name. The JavaScriptDateAPI is capable of generating timezone names, but doesn't expose them directly, so you have to extract them from one of the strings produced by other methods (this is awkward).It turns out that Safari's implementation of this method produces a string that looks like this:
whereas Chrome/Firefox produce one that looks like this:
As far as I can tell, this isn't due to a bug but actually an ambiguity in the spec. The spec for that method and the spec for the timezone name state that this name is "implementation-defined". So Chrome, Safari and Firefox have decided to implement this differently 🤷.
The Fix
Thanks to @DavidLawes (⭐️) we have an alternate function called
localTimeZoneAbbrthat instead usesDate.toLocaleStringand its ability to explicitly specify a "short" timezone name. So I've switched over to using that 🙂.I've also dropped the enclosing braces, so the whole timestamp should now appear in a format identical to dotcom's. See the screenshots below.
Node also implements the long-form version, likely because it has the same V8 engine as Chrome. This meant that our tests were previously set up to expect this - I've now updated them to expect the short-form version.
Changes
formatLocalover to uselocalTimeZoneAbbrlocalTimeZoneScreenshots