Fix dateByAddingDays

[meypod]

Hi It seems that dateByAddingDays does not work properly in some special timezones, on some special dates. so I have edited it and added tests to make sure it does

[meypod]

I'm not sure if your use case is sensitive to time changes or not but if it is, changing it from 1 hour in ms to 30 minutes should solve the issue, since the timezone changes and the daylight savings are usually rounded to 30 minutes or 1 hour

[meypod]

and actually I don't know if it makes any significant change at all, since this should be mostly a representation issue related to daylight savings

[z3bi]

Hmm so I think the problem here is actually timezone-mock. I recreated other tests for DST change over dates and set my local machine to that timezone and the tests passed.

[meypod]

To make sure it is not a quirk of mock timezone, try setting the time zone to the Brazil east and go to that special dates in tests with dst enabled, observe if going past 24:00 actually makes you go to the previous day's 23:00 or not If it does, it is bound to happen

It doesn't sound illogical to me

[meypod]

But in the end, I'm not sure if this is anything significant, I'm doing some tests with my app tomorrow because I have observed some unusual stuff on my time zone on a few devices for sunnah times

It's probably a bug on my side, but in the case it was fixed by this patch, I'll let you know

[meypod]

Update: the patch is significant and fixed the issue on my faulty devices.

[z3bi]

To make sure it is not a quirk of mock timezone, try setting the time zone to the Brazil east and go to that special dates in tests with dst enabled, observe if going past 24:00 actually makes you go to the previous day's 23:00 or not If it does, it is bound to happen

It doesn't sound illogical to me

This is part of the problem, Brazil no longer observes DST so this test doesn't actually make sense.

[z3bi]

@meypod if you remove timezone-mock we can add tests of different DST cutovers and then use https://github.com/marketplace/actions/set-timezone to run our tests on multiple timezones.

[meypod]

done just as a note, the last special date I added is the date that was mis behaving in my app on my faulty devices and tehran timezone, sunnah times were off by 3+ hours, plus the next day (March 21) was completely wrong (Fajr was around 12, instead of 4-5 am)

[z3bi]

done just as a note, the last special date I added is the date that was mis behaving in my app on my faulty devices and tehran timezone, sunnah times were off by 3+ hours, plus the next day (March 21) was completely wrong (Fajr was around 12, instead of 4-5 am)

I ran the new tests using the old dateByAddingDays function while my machine was in the Tehran timezone and the tests passed. This is not showing me a problem with the current approach.

[meypod]

There's one issue remaining on my faulty devices as of this year, Iran also doesn't see DST this is no problem for newer devices but on old android devices (and I don't know how, but some older devices than those didn't have the issue), DST seems to be causing the issue, hence applying this patch, fixes the weird times on those devices

But still, the time advances by 1 Hour on those devices, which is not ideal. I wish there was someway to disable DST ....

[meypod]

So overall, I have no idea how to test this tricky thing. I have seen it first hand that this patch does fix some issues in some cases.

at least the seemingly broken timezone mock would reproduce the issue and would test the issue

so idk, any suggestions ?

[z3bi]

I'm hesitant to change our date handling logic to try and patch a difficult to reproduce bug that only happens in a few older versions of Android. From everything I have researched our date adjusting logic is correct according to the Javascript specs.

Additionally your change feels fragile to me with a while loop that is just checking to see if the string representation of the date is no longer the same.

If we can get a reproducible use case that doesn't require timezone-mock (which has incorrect DST data) then that's something we can fix.

[meypod]

well the devices were android 9 and 11, so idk if its that old

[meypod]

I agree the change seems fragile, but a better way did not cross my mind.

I can keep the changes local to my project, but I felt responsible to make you aware of this possible issue

[z3bi]

well the devices were android 9 and 11, so idk if its that old

I was going off of your comment saying "old". It seems strange that basic Date math would be broken on Android 11's JavaScript implementation. Can you create the most barebones project that demonstrates this issue on Android and share the code?

[meypod]

I haven't try to reproduce on emulator, but my guess is I won't be able to reproduce, because of timezone and manufacture specific code. because why would a much older Samsung device on android 7 not face the issue, when 9 and 11 (of different brands) do.

I'll try to reproduce, but I wouldn't get my hopes up

[meypod]

hmm, it seems it's reproducible I'll try to create a barebone react native project to demonstrate this (may take some time)

[meypod]

Okay here is the reproduction repo (using expo) https://github.com/meypod/adhan-js-bug/

I'm using a Nexus 6P API 30 (Android 11.0 Google APIs | x86) emulator

[meypod]

I also made a Expo snack (for ease of use): https://snack.expo.dev/@meypod/adhan-js-bug-snack?platform=android

[meypod]

you can use the android emulator provided by default by expo and it will have the bug, luckily:

as you can see, the first date shows times okay, but sunnah times, which are for the next day, have problem.

then there's second date, which is a mess, all of it)

then there's the third date, which is around 10 days before this, and all times are fine.

[meypod]

Also, I'm sorry, I have to correct myself the samsung device is running android 9 the faulty devices are running android 10 and 11 (I know the one running 10 is a huawei honor phone)

(Not like it matters at this point, where it's reproduced on emulator, but anyway)

[meypod]

any news on this ?

[meypod]

nice