Closed Bug 333372 Opened 14 years ago Closed 14 years ago

Title of multiweek view includes one week too much

Categories

(Calendar :: Internal Components, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ssitter, Assigned: martinschroeder)

References

Details

Attachments

(2 files, 2 obsolete files)

The displayed title of the multiweek views always includes the next week that is not displayed.

Actual Results:
Example with default setting of four weeks in view: The weeks 15, 16, 17 and 18 are displayed. But the title of the multiweek view reads 'Week 15-19'. Same for the previous/next week buttons.

In the edge case of one week per view the title still says 'Week n - n+1' where it should be only 'Week n'.

Expected Results:
Title should only list the weeks that are really displayed.

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060408 Mozilla Sunbird/0.3a1+ with new profile.
I already have a patch for this and will attach it later to this bug.
Assignee: base → mschroeder
Status: NEW → ASSIGNED
Attached patch Patch v1 (obsolete) — Splinter Review
* added calculation of the first week if previous weeks are showed in the view
* fixed calculation of the last week in the view
* for a single week in the view show "Week X" instead of "Weeks X-Y"
Attachment #217799 - Flags: first-review?(jminta)
Comment on attachment 217799 [details] [diff] [review]
Patch v1

We shouldn't need to do double-computation for previous weeks in view.  Instead, let's just pass the newly-computed first day, from goToDay, into setNavLabels.
Attachment #217799 - Flags: first-review?(jminta) → first-review-
Attached patch Patch v2 (obsolete) — Splinter Review
Fixed issue from review of first version.

We use firstWeekDay and not directly d1 because the adjustment | d1.day -= 7 | can result in a date which is not showed in the view after setDateRange calculated the real date-range with taking weekstart-offset into account. Passing d1 directly in setNavLabels would result in a wrong weektitle.
Attachment #217799 - Attachment is obsolete: true
Attachment #218242 - Flags: first-review?(jminta)
I don't quite see the behaviour described. My 6-week multiweek view has the Title "Weeks 16-22". This bug suggests that it ought to be instead "Weeks 16-21", but I think it should be "Weeks 15-20".

I have one previous week showing, and have weeks starting on a Monday, so my 6-week view has Monday 10th April in the top-left, and Sunday 21st May in the bottom-right. According to my wife's Filofax :), these are weeks 15-20, inclusive.

I'm running my own x86 build of sunbird from cvs, 20060417.
(In reply to comment #5)
The patch also solves this problem and takes previous weeks in view into account.
*** Bug 336986 has been marked as a duplicate of this bug. ***
Comment on attachment 218242 [details] [diff] [review]
Patch v2

+                    // The variable firstWeekDate is a definitely showed date
+                    // in the first week of the view (needed for setNavLabels).
                     var d1 = aDate.startOfWeek.clone();
+                    d1.day -= 7*calBranch.getIntPref("previousweeks.inview");
+                    var firstWeekDate = d1.clone();
+                    firstWeekDate.normalize();
Can you juggle the comment, so that it appears right before var firstWeekDate.  I think it'll make more sense there.

This looks good, apologies for the delay on the review.  r=jminta
Attachment #218242 - Flags: first-review?(jminta) → first-review+
Whiteboard: [needs landing]
Attached patch Patch v3Splinter Review
moved the comment as suggested
carried over r=jminta
Attachment #218242 - Attachment is obsolete: true
Attachment #221486 - Flags: first-review+
Patch checked in on trunk and MOZILLA_1_8_BRANCH
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
This does not work correctly if first day of week is not Sunday.

1. Started Sunbird with clean profile. Set weeks in multiweek view to 1 for easier testing. The title displays 'Week 20'. That is correct.
2. Set first day of week to Monday. The title displays 'Week 19'. That is wrong, 'Week 20' is expected. Note: The normal week view still displays 'Week 20', only the multi week view is wrong.

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060516 Mozilla Sunbird/0.3a2+
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Fixes the problem with different week start days.

The variable firstWeekDate should be relative to aDate, which is the selected day.
Attachment #222269 - Flags: first-review?(jminta)
Comment on attachment 222269 [details] [diff] [review]
Fix for correct date in first week of view

r=jminta
Attachment #222269 - Flags: first-review?(jminta) → first-review+
follow-up patch checked in.
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
verivied with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060929 Sunbird/0.3
Status: RESOLVED → VERIFIED
Whiteboard: [litmus testcase wanted]
Litmus testcase 2616 created
Whiteboard: [litmus testcase wanted]
You need to log in before you can comment on or make changes to this bug.