GNOME Bugzilla – Bug 678507
date doesn't update after changing timezone
Last modified: 2017-10-25 16:25:30 UTC
Created attachment 216882 [details] screenshot I changed timezone earlier today from HK to BOS. The shell clock updated correctly but the calendar still thinks it is tomorrow.
Still happening.
is the bug solved? If not can i have the link to the code?
Created attachment 258734 [details] Screenshot showing other bugs apart from date update On changing the timezone, not only the date does not get updated in the calendar, but tasks are also displayed as scheduled for wrong days. I created tasks in timezone AKDT and changed to HST (Just an example. The bug also occurs with more timezones).
Created attachment 258736 [details] Screenshot showing wrong date detection Dates are also detected wrongly on timezone change. In the screenshot, I've selected 8th November, but the one detected to display tasks is 7th Nov.
Created attachment 258936 [details] [review] Proposed patch to fix the bug reported and the ones mentioned in comments I've attached a patch for this bug as well as for the bugs in events list. The bug was due to JavaScript returning false dates on correct timezones.
Review of attachment 258936 [details] [review]: I'm not sure about the overall patch. So is javascript which is giving bad dates? where? in new Date()? or in toLocaleFormat()? or do you just need to call date.toLocaleFormat each time? (which is fine) ::: js/ui/calendar.js @@ +87,3 @@ + else + date = new Date(); + let locale_formatted_date = date.toLocaleFormat("%Y-%m-%dT%H:%M:%S"); use ratio character instead of semicolons @@ +154,3 @@ + return abbreviations[dayNumber]; +} + C_("list tuesday", "Tuesday"), is this necesary? why not using toLocaleFormat? @@ +212,3 @@ + return abbreviations[monthNumber]; +} + /* Translators: Calendar month for June */ is this necesary? why not using toLocaleFormat? @@ +838,3 @@ if (_sameYear(day, now)) /* Translators: Shown on calendar heading when selected day occurs on current year */ + dayString = _getEventDay(day.getDay()) + ', ' + _getCalendarMonth(day.getMonth()) + ' ' + day.getDate(); Translatable strings have to be single strings, not compose strings. Also, this is not even translatable. You need _(foo) @@ +841,3 @@ else /* Translators: Shown on calendar heading when selected day occurs on different year */ + dayString = _getEventDay(day.getDay()) + ', ' + _getCalendarMonth(day.getMonth()) + ' ' + day.getDate() + ', ' + day.getFullYear(); ditto ::: js/ui/dateMenu.js @@ +127,3 @@ * isn't very big. */ + let updated_now = now.toLocaleFormat("%Y-%m-%dT%H:%M:%S"); need to be translatable and a comment to translators of what it means. Also, what happens with those people who use AM/PM? it's impossible to set with this
(In reply to comment #6) > Review of attachment 258936 [details] [review]: > > I'm not sure about the overall patch. So is javascript which is giving bad > dates? where? in new Date()? or in toLocaleFormat()? > or do you just need to call date.toLocaleFormat each time? (which is fine) > > ::: js/ui/calendar.js > @@ +87,3 @@ > + else > + date = new Date(); > + let locale_formatted_date = date.toLocaleFormat("%Y-%m-%dT%H:%M:%S"); > > use ratio character instead of semicolons No. This breaks LTR languages.
This bug happens because SpiderMonkey caches the timezone information, so when the timezone is changed calls like getDate and getHours still use the old timezone. A function (JS_ClearDateCaches) was added (see bug https://bugzilla.mozilla.org/show_bug.cgi?id=285615) to SpiderMonkey that can be used to invalidate the cache. So the solution to this issue is to call that function when the timezone is changed. I tried to create a patch for this issue, but I'm new to javascript programming and the gnome stack and I couldn't figure out how to properly solve it. I did an ugle hack just so to see if this was indeed the root cause by adding a call to JS_ClearDateCaches to the, global, log function in gjs/context.cpp and a small change to calendar.js in gnome-shell. That did indeed fix the issue. Then I tried to make a proper fix by adding a new function to gjs/context.cpp (I also added it to the global_funcs array) that would call JS_ClearDateCaches, but I couldn't get that to work, when I tried to call that function from calendar.js it just said that there was no such function. That's how far I got and I would like some pointers on how I can proceed.
Created attachment 275812 [details] [review] Patch to gjs to add a wrapper for JS_ClearDateCaches
Created attachment 275813 [details] [review] Rebuild the calendar after a time zone change
Review of attachment 275812 [details] [review]: I'd prefer if this went into modules/system.cpp instead.
During my work with this bug I found that JS_ClearDateCaches behaved strangly, it didn't update the time when changing from certain time zones. I think I found that issue and filed bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1004706
Created attachment 275819 [details] [review] Add wrapper for JS_ClearDateCaches
Created attachment 275820 [details] [review] Rebuild the calendar after a time zone change
Review of attachment 275819 [details] [review]: Would it be possible to add a call to tzset here to work around the SpiderMonkey bug?
(In reply to comment #15) > Review of attachment 275819 [details] [review]: > > Would it be possible to add a call to tzset here to work around the > SpiderMonkey bug? Yes, I tried it and it worked. Should I add it?
Yes. At the same time, could also add a comment in the gnome-shell patch explaining why we have to clear the datetime cache with a link to this bug, and also add a comment around the tzset call that points to the upstream SpiderMonkey bug as well?
(In reply to comment #17) > Yes. At the same time, could also add a comment in the gnome-shell patch > explaining why we have to clear the datetime cache with a link to this bug, and > also add a comment around the tzset call that points to the upstream > SpiderMonkey bug as well? ok, will do
Created attachment 275836 [details] [review] Add wrapper for JS_ClearDateCaches
Created attachment 275837 [details] [review] Rebuild the calendar after a time zone change
*** Bug 734236 has been marked as a duplicate of this bug. ***
Comment on attachment 275836 [details] [review] Add wrapper for JS_ClearDateCaches Moved gjs patch to bug 739790
*** Bug 744631 has been marked as a duplicate of this bug. ***
Created attachment 302186 [details] [review] Rebuild the calendar after a time zone change Updated patch for 3.16
Should this bug not be marked as "fixed"?
(In reply to Oliver Propst from comment #25) > Should this bug not be marked as "fixed"? No, we still need the gnome-shell part of the patch.
Review of attachment 302186 [details] [review]: ::: js/ui/calendar.js @@ +552,3 @@ + updateTimeZone: function() { + // The calendar need to be rebuilt after a time zone update beacuse *because @@ +554,3 @@ + // The calendar need to be rebuilt after a time zone update beacuse + // the date might have changed. + this._rebuildCalendar(); Is this really necessary though? Off-hand the conditions for rebuilding the calendar in _update() look like they should work here as well? ::: js/ui/dateMenu.js @@ +421,3 @@ + System.clearDateCaches(); + + this._calendar.updateTimeZone(); Does it make sense to propagate this to the messageList if the selected date was "today" before the timezone change?
(In reply to Florian Müllner from comment #27) > Review of attachment 302186 [details] [review] [review]: > > ::: js/ui/calendar.js > @@ +552,3 @@ > > + updateTimeZone: function() { > + // The calendar need to be rebuilt after a time zone update beacuse > > *because Fixed > > @@ +554,3 @@ > + // The calendar need to be rebuilt after a time zone update beacuse > + // the date might have changed. > + this._rebuildCalendar(); > > Is this really necessary though? Off-hand the conditions for rebuilding the > calendar in _update() look like they should work here as well? It is necessary because _markedAsToday, which is compared against the now date to determine if the calendar should be rebuilt, is affected by the time zone change. Meaning that it changes with the time zone, so both dates will still be on the same day. > > ::: js/ui/dateMenu.js > @@ +421,3 @@ > + System.clearDateCaches(); > + > + this._calendar.updateTimeZone(); > > Does it make sense to propagate this to the messageList if the selected date > was "today" before the timezone change? I'm not sure I fully understand. But if the user did not change the selected date (it is set to the date (and time) of the creation of the calendar) it will also change along with date. The reason for this is that the selected date is also affected by the time zone change. So if the date is the 13'th and the selected date is also the 13'th and a time zone change caused the date to become the 14'th the selected date will also be the 14'th. However if the user changed the selected date the time is set to around 12 o'clock, which means that in this case the selected date will only change if the time zone difference is greater than 12 hours. All this is bit confusing so it is entirely possible that I have got some details wrong.
Created attachment 309237 [details] [review] Rebuild the calendar after a time zone change
I was using the time zone gui for triggering this bug. However I found that I could no longer trigger a timezone update event using the gui. I tracked down the issue and filed this bug. https://bugzilla.gnome.org/show_bug.cgi?id=753602
(In reply to Martin Andersson from comment #28) > (In reply to Florian Müllner from comment #27) > > Does it make sense to propagate this to the messageList if the selected date > > was "today" before the timezone change? > I'm not sure I fully understand. But if the user did not change the selected > date (it is set to the date (and time) of the creation of the calendar) it > will also change along with date. What I mean is: The message list contains an "Events" section that displays calendar events for the current (or selected) day. When the timezone changes, the list may change and thus requires a rebuild just as the calendar - or is there something I'm overlooking that makes it magically work?
(In reply to Florian Müllner from comment #31) > (In reply to Martin Andersson from comment #28) > > (In reply to Florian Müllner from comment #27) > > > Does it make sense to propagate this to the messageList if the selected date > > > was "today" before the timezone change? > > I'm not sure I fully understand. But if the user did not change the selected > > date (it is set to the date (and time) of the creation of the calendar) it > > will also change along with date. > > What I mean is: > The message list contains an "Events" section that displays calendar events > for the current (or selected) day. When the timezone changes, the list may > change and thus requires a rebuild just as the calendar - or is there > something I'm overlooking that makes it magically work? The "Events" section is also updated when the calendar is rebuilt due to a time zone change. So that already works as expected, however I don't know exactly what triggers that update.
(In reply to Martin Andersson from comment #32) > The "Events" section is also updated when the calendar is rebuilt due to a > time zone change. So that already works as expected, however I don't know > exactly what triggers that update. The date for which events are shown is reset to "today" each time the calendar menu is opened - maybe it's that? (Sorry, can't really test right now)
(In reply to Florian Müllner from comment #31) > The message list contains an "Events" section that displays calendar events > for the current (or selected) day. When the timezone changes, the list may > change and thus requires a rebuild just as the calendar - or is there > something I'm overlooking that makes it magically work? I just noticed this in Ubuntu too. The bug is still present. I tried the patch and it works for me, including events being correct. You can do something like this to reproduce / verify the patch. $ timedatectl set-timezone Europe/London $ timedatectl set-ntp false $ timedatectl set-time 01:00 $ timedatectl set-timezone America/New_York # date changes to previous day raise the calendar, click month forwards and backwards, days are offset by one or correct depending on whether you have this patch applied (there's a trivial conflict to resolve). If possible it'd be great to have this patch in 3.26.1.
Comment on attachment 309237 [details] [review] Rebuild the calendar after a time zone change Some nits in the commit message, otherwises looks good to me.
*** Bug 765897 has been marked as a duplicate of this bug. ***