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

Re-implementation of the Umm Al-Qura Calendar is broken. #328

Open
ZacharyObeid opened this issue Feb 1, 2021 · 2 comments
Open

Re-implementation of the Umm Al-Qura Calendar is broken. #328

ZacharyObeid opened this issue Feb 1, 2021 · 2 comments

Comments

@ZacharyObeid
Copy link

Currently there is an issue with the Umm al-Qura calendar, the pop-up calendar automatically sets the date incorrectly.

  • Before 12 PM the calendar is set to the current date correctly and matches with current date in the official Umm al-Qura calendar.
  • After 12 PM the calendar current date is set to the next day which is incorrect and not does not match with current date in official Umm Al-Qura calendar.

After further investigate I found the issue to be the re-implementation of the UMM Al-Qura calendar. To further verify that this is the issue I tested with version 1.10 and got the correct date. I tested with 1.12 and it is broken. The main issue is with the Date.js file that was changed.

Reproduction Steps:

Get the Um Al-Qura data before 12PM. Then get it after 12PM. You will see that the date has changed. Here is a URL to check the current date of the Umm Al-Qura. https://webspace.science.uu.nl/~gent0113/islam/ummalqura_converter.htm

Below is the URL for the dojo bug that this was filed for and the Pull Request that was merged which broke the Umm Al-Qura date.

We need support to solve this issue as it is urgent.

@dylans
Copy link
Member

dylans commented Feb 4, 2021

I'm not sure any of us on the Dojo team know how to determine correct behavior here so we usually rely on outside contributors. If you can look at the old code that worked in 1.10 and the new code and create a pull request we'll land it, though please author it against the more recent code (post 1.16). Thanks.

@ZacharyObeid
Copy link
Author

ZacharyObeid commented Feb 11, 2021

@dylans Hello Dylan, I have spoken with a dev team I work with out of the Middle East. The correct behavior for the Umm Al-Qura calendar is to change dates after 12AM and not at 12PM. The current implementation is broken.

Also, to further verify this you can go to the website that was referenced by the previous pull request (which is the official Kingdom of Saudi Arabia calendar: http://www.ummulqura.org.sa/ ). There you can see the behavior and how the date changes after 12AM and not 12PM. Please note that the website from the Kingdom of Saudi Arabia (KSA) is in there local time and does not update base on local. It will always be in KSA time and the time is displayed on the right hand side if you switch it to English.

The fix for this would be to revert the changes made in Pull Request #154 to the Date.js file. The original developer added the extra month tables correctly, however one of the other changes in here broke it. There is a slight calculation issue that I cannot find. If I have the time I will create a PR against 1.16 or the latest with the changes that revert the Date.js file but I will keep the extra month tables that he added. I will do my best to debug the issue but I am still unfamiliar with the algorithm.

Thank you for taking the time to respond to the inquiry.

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

No branches or pull requests

2 participants