Closed Bug 365639 Opened 13 years ago Closed 13 years ago

Navigation in Multiweek view is faulty if 'Previous weeks to show' is not None

Categories

(Calendar :: Calendar Views, defect)

x86
Windows 2000
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ssitter, Assigned: jminta)

References

Details

Attachments

(1 file)

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a2pre) Gecko/20070102 Calendar/0.4a1

If preference 'Previous weeks to show' is set to one week the command 'Next week' does nothing and the command 'Previous week' navigates the view two weeks back.

If preference 'Previous weeks to show' is set to two weeks the command 'Next week' navigates the view one week back and the command 'Previous week' navigates the view three weeks back.

If preference 'Previous weeks to show' is set to None the navigation works as expected.

This is regressed by the checkin for Bug 336287.
Summary: Navigation in Multiweek view is faulty if 'Previous weeks to show' not None → Navigation in Multiweek view is faulty if 'Previous weeks to show' is not None
Since goToDay is going to subtract days in order to display the previous weeks, we need to make sure to add enough to cover that compensation.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #250781 - Flags: second-review?(mvl)
Attachment #250781 - Flags: first-review?(lilmatt)
Attachment #250781 - Attachment is patch: true
Attachment #250781 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 250781 [details] [diff] [review]
compensate for previous weeks

>Index: calendar/base/content/calendar-decorated-multiweek-view.xml
>===================================================================

>                     // aNumber only corresponds to the number of weeks to move
>-                    d1.day += 7*aNumber;
>+                    // make sure to compensate for previous weeks in view too
>+                    d1.day += 7 * (aNumber + getPrefSafe("calendar.previousweeks.inview", 4));

If we can't find the preference, is 4 weeks really the correct value?  I'm not sure where 4 came from, and I'd like to know before approving this.
(In reply to comment #2)

The default value for 'calendar.previousweeks.inview' is 0 (equals to none in the preference dialog).
(In reply to comment #2)
> If we can't find the preference, is 4 weeks really the correct value?  I'm not
> sure where 4 came from, and I'd like to know before approving this.
> 
Stolen from http://lxr.mozilla.org/mozilla/source/calendar/base/content/calendar-decorated-multiweek-view.xml#185
but you're right that that's not the correct default.
Comment on attachment 250781 [details] [diff] [review]
compensate for previous weeks

>+                    // make sure to compensate for previous weeks in view too
>+                    d1.day += 7 * (aNumber + getPrefSafe("calendar.previousweeks.inview", 4));

r=lilmatt with this changed from 4 to 0 as per the discussion.


What strikes me as The Right Fix is to use a constant of some sort, be it a build-type substitution, or something in one of the *Utils.js files so that we define the actual number in one place, and it gets updated everywhere else (including in lightning|sunbird.js).
Attachment #250781 - Flags: first-review?(lilmatt) → first-review+
The Right place fto define this constant only once would be our own prefs file. Then you don't need the fallback anymore, because you know the pref will always be set to something.
Comment on attachment 250781 [details] [diff] [review]
compensate for previous weeks

r2=mvl
Attachment #250781 - Flags: second-review?(mvl) → second-review+
Whiteboard: [needs checkin]
Duplicate of this bug: 369281
patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs checkin]
verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070214 Calendar/0.4a1
Status: RESOLVED → VERIFIED
Whiteboard: [litmus testcase wanted]
Litmus testcase 3021 created
Whiteboard: [litmus testcase wanted]
You need to log in before you can comment on or make changes to this bug.