Skip to content
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.

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

Crash on specific time zones #323

Closed
romaHerman opened this issue Feb 27, 2017 · 5 comments
Closed

Crash on specific time zones #323

romaHerman opened this issue Feb 27, 2017 · 5 comments

Comments

@romaHerman
Copy link

romaHerman commented Feb 27, 2017

First , thanks for such nice tool!!!
It really save tones of tome

But there is one crash though

Crash could be reproduced in example project
steps to reproduce :

  1. select different timezone (e.g. Amman - Jordan)
  2. select formatter dates in ViewController.swift to current
    let startDate = formatter.date(from: "2017 02 01")! let endDate = formatter.date(from: "2017 12 01")!
  3. Run app, scroll to March 31, and select it

App crashes on line 754
let lastDateOfCurrentMonth = calendar.endOfMonth(for: date)!
in file JTAppleCalendatView.swift

I now investigating this ,myself
will post some clue about fixing if I had one

Thanks,
Roman

====UPDATE=====
After investigation I have found that problem is in
File GlobalFunctionsAndExtensions.swift
in function endOfMonth

I replaced functions startOfMonth and endOfMonth with following functions from this answer on stack overflow
http://stackoverflow.com/questions/10717574/get-firstdate-lastdate-of-month

Could you, please review this suggestion and if it's appropriate can you please update code and cocoapod ?

Thanks in advance, Roman

@patchthecode
Copy link
Owner

Hi, i see the problem, but I am trying to understand why the code fails before I merge the fix.
I have pulled out the error code and put it into XCode Playground.

here is the code:

let formatter = DateFormatter()
let timeZone = TimeZone(identifier: "Asia/Amman")!
let locale = Locale(identifier: "ar_JO")

formatter.timeZone = timeZone
formatter.locale = locale
formatter.dateFormat = "yyyy MM dd"

let date = formatter.date(from: "2017 03 31")

In this code, the date equals nil
And I do not know the reason why yet.

For you region in Jordan, this code above is supposed to give me a date right?

@patchthecode
Copy link
Owner

When i do the exact code for my region, it works.

timeZone = TimeZone(identifier: "America/Vancouver")!
locale = Locale(identifier: "en_US")

formatter.timeZone = timeZone
formatter.locale = locale

date = formatter.date(from: "2017 03 31")

Date does not = nil.

Silly question, but does is there a 31-March-2017 for your region? or is there something wrong with the code.

@romaHerman
Copy link
Author

Yes there is 31 of march :)

I think I have figured out the problem

It refers to daylight saving time. And this shift in most countries usually is being performed in march.
Thus this could lead to invalid time zone format

I created following playground to check this
https://gist.github.com/romaHerman/f119885591250a2dc5f0ebed8ac4b37f

Then I checked best practices for that. And it is advised to use UTC time zone instead of local.
http://stackoverflow.com/questions/2532729/daylight-saving-time-and-time-zone-best-practices

So now I think the best fix for this problem will be to use following
formatter.timeZone = TimeZone(identifier: "UTC")!
instead of local time zone when counting end and beginning of month

@patchthecode
Copy link
Owner

patchthecode commented Feb 28, 2017

UTC is a solution thats true, but I wanted to keep this calendar's generated dates friendly with the dates for the region they selected.

Doing this allows this calendar to work well other libraries such as https://github.com/malcommac/SwiftDate without developers doing unnecessary conversions from UTC into the timezone of their region.

After researching I see that DateFormatter's default is midnight (i think). But since in the buggy regions, midnight doesn't exist because of Daylight Saving Time, this causes the formatter to returns nil.

Honestly, I think this feels like an apple bug. I think that it should default to the next legal time if midnight doesn't exist. I think it should only return nil if there are no legal times left for that date. While looking around i've seen people file bugs to Apple about this. But since its been filed for some years now, I doubt they will change this behaviour.

The Fix:
I have changed the formatter.isLenient = true

Here is the new PlayGround code:

//: Playground - noun: a place where people can play
import Foundation

let formatter = DateFormatter()
let timeZone = TimeZone(identifier: "Asia/Amman")!
let locale = Locale(identifier: "ar_JO")

formatter.timeZone = timeZone
//formatter.locale = locale
formatter.dateFormat = "yyyy MM dd"
formatter.isLenient = true

let date1 = formatter.date(from: "2017 03 30") // The date works and starts from midnight (the default)
let date2 = formatter.date(from: "2017 03 31") // The date works and starts from the next legally availablel time

// We can check to see that the time look correct
formatter.dateFormat = "yyyy MM dd HH:MM:SS"
print(formatter.string(from: date1!)) // Starts from midnight
print(formatter.string(from: date2!))  // Starts from 1Am

Here are the results:

screen shot 2017-02-28 at 8 36 04 am

I think this might be a good solution to get the best out of the situation:

  1. the bug is fixed i think
  2. the dates are still in the time zones of the different users

What do you think?

patchthecode added a commit that referenced this issue Feb 28, 2017
Possible fix for
#323
@romaHerman
Copy link
Author

Wow, you did a great research !

Yes I also think that it looks like an apple bug also (time for radar ? :) )

Solution with formatter.isLenient = true is very good because it prevents from UTC conversions back and forth
So I also think that it's the best from all possible ways of handling daylight shift

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants