respect region time format on NTP

[cezaraugusto]

fix brave/brave-browser#1308

this change allows users with 24-hour format to see time on NTP the same way their system defined. defaults to whichever locale user defined on their system settings.

Test Plan:

1. Go to NTP and assert time there is on the same time format (12-hour or 24-hour) as your system
2. Change your system locale to a language that defaults to the opposite time format (en-US uses the 12-hour system, pt-BR uses the 24-hour system)
3. Relaunch Brave
4. NTP should follow the change

Reviewer Checklist:

• New files have MPL-2.0 license header.
• Request a security/privacy review as needed.
• Adequate test coverage exists to prevent regressions
• Verify test plan is specified in PR before merging to source

After-merge Checklist:

• The associated issue milestone is set to the smallest version that the
  changes has landed on.
• All relevant documentation has been updated.

[petemill]

I don't see that this respects 24-hour clock in macOS @cezaraugusto how did you get the test plan to pass in macOS?

Either way, I think it's better than current

Edit: I don't think there's a way to get the system 24-hour / 12-hour preference. Instead I think we should mark that this PR solves the issue of the incorrect 0 prefix, and keep the 24-hour / 12-hour issue as unsolved untile we make a preference (perhaps double-clicking the clock switches between 24-hour / 12-hour like in the Momentum extension NTP).

[petemill]

Not that this fixes the issue, but according to the linked documentation, the first parameter should be undefined, right?

A string with a BCP 47 language tag, or an array of such strings. To use the browser's default locale, omit this argument or pass undefined.

[cezaraugusto]

good catch, but I think the algorithm ignores it anyway. I just left it with what we had

[cezaraugusto]

updated

[cezaraugusto]

@petemill this fix does solve the issue as stated by OP Please respect region time format on new tab (brave/brave-browser#1308). if you change your system locale to one that uses the 24-hour system (such as Brazilian Portuguese) then restarting the browser should update the time, so region time is indeed respected.

In fact, the above was my test plan, but to avoid asking reviewers to change their system to an unknown language, I changed the test plan for just the time format, which doesn't work to my surprise. In my opinion, if the Web API integration with the system is limited, then there's nothing we can do, and instead of marking the issue as fixed we should mark it as wontfix since there's no action we can do.

worth noting that I also tried using toLocaleDateString() but the output is the same. So it seems to be a truly web API limitation:

const event = new Date(Date.now())
event.toLocaleDateString(undefined, {timeStyle: 'short'})

I'm going to update both the issue and this PR test plan to reflect the above.

[petemill]

LGTM, nice update to the issue and test plan. Let's see why Jenkins is complaining before merging

[petemill]

Just needs rebasing @cezaraugusto

[cezaraugusto]

rebased

[bsclifton]

Tested out, LGTM; comments made in brave/brave-browser#1308 (comment) definitely make sense 👍