Skip to content

Always use 24-hours locale #1947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 16, 2018
Merged

Always use 24-hours locale #1947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8af612d
Always use 24-hours locale
Oct 15, 2018
c2631d5
use en_US_POSIX instead of en_GB
Oct 15, 2018
a087967
Update NSDateFormatter locale for messaging tests
Oct 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Example/Messaging/Tests/FIRMessagingContextManagerServiceTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ @implementation FIRMessagingContextManagerServiceTest
- (void)setUp {
[super setUp];
self.dateFormatter = [[NSDateFormatter alloc] init];
self.dateFormatter.locale = [NSLocale localeWithLocaleIdentifier:@"en_US_POSIX"];
[self.dateFormatter setDateFormat:@"yyyy-MM-dd HH:mm:ss"];
self.scheduledLocalNotifications = [NSMutableArray array];
[self mockSchedulingLocalNotifications];
Expand Down
1 change: 1 addition & 0 deletions Firebase/Messaging/FIRMessagingContextManagerService.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ + (BOOL)handleContextManagerMessage:(NSDictionary *)message {
+ (BOOL)handleContextManagerLocalTimeMessage:(NSDictionary *)message {
NSString *startTimeString = message[kFIRMessagingContextManagerLocalTimeStart];
NSDateFormatter *dateFormatter = [[NSDateFormatter alloc] init];
dateFormatter.locale = [NSLocale localeWithLocaleIdentifier:@"en_US_POSIX"];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be [NSLocale autoupdatingCurrentLocale]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want parse date from string regardless of user locale preferences you should use en_US_POSIX. More info you can find here (
https://developer.apple.com/library/archive/qa/qa1480/_index.html)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgive me but I'm not clear about the purpose of this change. It doesn't affect the format of startDate and endDate at all, which basically has no impact on SDK.So I'm not sure this change is necessary for the SDK to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chliangGoogle Try to set 12-hours time in the iPhone's date time settings and parse date from the string (2018-10-15 23:18:00)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using the non 24-hour time, which I assume it's the 12-hours time.
po startDate
2018-10-16 17:24:00 +0000

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On my device if I set date format to 12-hours startDate2 always nil. Maybe it depend on specific locale. I have "en_BY". startDate always return valid date

let dateFormatterPosix = DateFormatter()
dateFormatterPosix.locale = Locale.init(identifier: "en_US_POSIX")
dateFormatterPosix.dateFormat = "yyyy-MM-dd HH:mm:ss"
let startDate = dateFormatterPosix.date(from: "2018-10-15 23:18:00")
        
let dateFormatter = DateFormatter()
dateFormatter.locale = Locale.current
dateFormatter.dateFormat = "yyyy-MM-dd HH:mm:ss"
let startDate2 = dateFormatter.date(from: "2018-10-15 23:18:00")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Apple guide
https://developer.apple.com/documentation/foundation/dateformatter

Working With Fixed Format Date Representations
Important

In macOS 10.12 and later or iOS 10 and later, use the ISO8601DateFormatter class when working with ISO 8601 date representations.
When working with fixed format dates, such as RFC 3339, you set the dateFormat property to specify a format string. For most fixed formats, you should also set the locale property to a POSIX locale ("en_US_POSIX"), and set the timeZone property to UTC.

let RFC3339DateFormatter = DateFormatter()
RFC3339DateFormatter.locale = Locale(identifier: "en_US_POSIX")
RFC3339DateFormatter.dateFormat = "yyyy-MM-dd'T'HH:mm:ssZZZZZ"
RFC3339DateFormatter.timeZone = TimeZone(secondsFromGMT: 0)

/* 39 minutes and 57 seconds after the 16th hour of December 19th, 1996 with an offset of -08:00 from UTC (Pacific Standard Time) */
let string = "1996-12-19T16:39:57-08:00"
let date = RFC3339DateFormatter.date(from: string)
For more information, see Technical Q&A QA1480 “NSDateFormatter and Internet Dates”.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chliangGoogle Please, read and review again🙃

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, @Ramesik. Can you update the tests as well? Also, should this code be explicitly setting the time zone on the date formatter as specified in the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@morganchen12 Seems that we need here device timezone to set local notifications on proper time. Unit tests now up to date.

[dateFormatter setDateFormat:kLocalTimeFormatString];
NSDate *startDate = [dateFormatter dateFromString:startTimeString];

Expand Down