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)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ssitter, Assigned: jminta)
References
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
13.65 KB,
patch
|
mvl
:
first-review+
|
Details | Diff | Splinter Review |
6.45 KB,
patch
|
mvl
:
first-review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•19 years ago
|
||
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.
Assignee | ||
Comment 2•19 years ago
|
||
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 3•19 years ago
|
||
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 4•19 years ago
|
||
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-
Assignee | ||
Comment 5•19 years ago
|
||
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 6•19 years ago
|
||
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)
Updated•19 years ago
|
Attachment #211516 -
Flags: first-review?(mvl) → first-review+
Assignee | ||
Comment 7•19 years ago
|
||
Patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•19 years ago
|
||
(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)
Reporter | ||
Comment 9•19 years ago
|
||
Reopen this bug as the problem is only partly fixed until now.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 10•19 years ago
|
||
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 11•19 years ago
|
||
Comment on attachment 211677 [details] [diff] [review]
follow-up patch
What does this patch try to fix?
Reporter | ||
Comment 12•19 years ago
|
||
(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 13•19 years ago
|
||
Comment on attachment 211677 [details] [diff] [review]
follow-up patch
ah, ok. looks good.
r=mvl
Attachment #211677 -
Flags: first-review?(mvl) → first-review+
Assignee | ||
Updated•19 years ago
|
Blocks: sunbird-0.3a2
Assignee | ||
Comment 14•19 years ago
|
||
patch checked in.
Status: REOPENED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•