Closed Bug 321560 Opened 19 years ago Closed 19 years ago

Wrong day selected in new views when using minimonth or datepicker

Categories

(Calendar :: Sunbird Only, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ssitter, Assigned: jminta)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Wrong day selected in new views when using minimonth or datepicker If you click a date on the minimonth or select a date using the 'Go Date' command the wrong date is selected in the view. The date selected/highlighted in the view is always one day too little. For example click 28-Dec-2005 in minimonth --> 27-Dec-2005 is selected in view. Happened in all views. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051226 Mozilla Sunbird/0.3a1+ with clean profile.
Most likely a issue in converting the jsDate to a calDateTime. Eventually, I'd like most of the jsDate's that are being used in reference to the views to disappear. I had planned to do that when we killed gCalendarWindow, but now it seems that this may need to be done sooner.
The problem here is that since we were just using jsDateTo DateTime, we were getting a date in UTC, rather than in the default timezone. This patch uses getInTimezone in the appropriate places, and also converts code that previously used gCalendarWindow.currentView.goToDay to using something a bit more sensible, since the gCalendarWindow.currentView object is actually a bad compatibility hack. Note that this may help with bug 321562 and bug 321563, since sending a goToDay command with a UTC time would throw them off.
Attachment #206874 - Flags: first-review?(mvl)
Comment on attachment 206874 [details] [diff] [review] use getInTimezone, consolidate goToDay calls >Index: mozilla/calendar/resources/content/calendarWindow.js > CalendarWindow.prototype.pickAndGoToDate = function calWin_pickAndGoToDate( ) >- currentView.goToDay( jsDateToDateTime(pickedDate) ); >+ currentView.goToDay(jsDateToDateTime(pickedDate).getInTimezone(calendarDefaultTimezone())); I'm a bit uneasy about having the callers set the right timezone. In the end, the view is what cares about the timezone. Shouldn't that set the timezone?
Comment on attachment 206874 [details] [diff] [review] use getInTimezone, consolidate goToDay calls >Index: mozilla/calendar/resources/content/calendarWindow.js >- currentView.goToDay( jsDateToDateTime(pickedDate) ); >+ currentView.goToDay(jsDateToDateTime(pickedDate).getInTimezone(calendarDefaultTimezone())); So the caller sets the timezone of the view by setting the timezone of the startdate? I think i can live with that, if it's properly documented. >Index: mozilla/calendar/resources/content/unifinder.js >@@ -302,22 +302,21 @@ function unifinderOnSelect( event ) >+ if (gCalendarWindow.currentView.getVisibleEvent(calendarEvent) == false) { >+ var currentView = document.getElementById("view-deck").selectedPanel; >+ currentView.goToDay(calendarEvent.startDate); >+ } But then won't this set the view's timezone to whatever the timezone of the selected element happens to be? Sounds wrong. Maybe we shouldn't use the timezone of the starttime as timezone of the view. It it too confusing, in cases like this. Setting the timezone explicitly might be better.
Attachment #206874 - Flags: first-review?(mvl) → first-review-
Updated to reflect previous review comments. Introduces a timezone attribute to the views. They'll try to put everything in that timezone. Tested in both GMT+ and GMT- tzs without a problem.
Assignee: mostafah → jminta
Attachment #206874 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #211516 - Flags: first-review?(mvl)
Comment on attachment 211516 [details] [diff] [review] set view timezones explicitly looks good, thanks! r=mvl (we might consider optimizing calDateTime.getInTimezone for cases where the timezone is the same as the current timezone, to speed things up somewhat)
Attachment #211516 - Flags: first-review?(mvl) → first-review+
Patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Attached patch follow-up patchSplinter Review
(This is the patch from IRC) Follow-up patch based on some feedback by ssitter. Basically we're just changing the order of where getInTimezone is called.
Attachment #211677 - Flags: first-review?(dmose)
Reopen this bug as the problem is only partly fixed until now.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 211677 [details] [diff] [review] follow-up patch This is sunbird only, so I suppose it probably should have been mvl from the beginning.
Attachment #211677 - Flags: first-review?(dmose) → first-review?(mvl)
Comment on attachment 211677 [details] [diff] [review] follow-up patch What does this patch try to fix?
(In reply to comment #11) > (From update of attachment 211677 [details] [diff] [review] [edit]) > What does this patch try to fix? Happens because of my UTC+1 timezone offset: Week view: If you click first day of week in minimonth the previous week is displayed. Multiweek view: If you click first day of week in minimonth there is one more previous week displayed as configured. Month view: If you click first day of month in minimonth the previous month is displayed.
Comment on attachment 211677 [details] [diff] [review] follow-up patch ah, ok. looks good. r=mvl
Attachment #211677 - Flags: first-review?(mvl) → first-review+
patch checked in.
Status: REOPENED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: