333372 - Title of multiweek view includes one week too much

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[Stefan Sitter [:ssitter]]

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.

Comment 1

[Martin Schröder [:mschroeder]]

I already have a patch for this and will attach it later to this bug.

Comment 2

[Martin Schröder [:mschroeder]]

Attachment: Patch v1

* 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"

Comment 3

[Joey Minta]

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.

Comment 4

[Martin Schröder [:mschroeder]]

Attachment: Patch v2

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.

Comment 5

[Calum Mackay]

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.

Comment 6

[Martin Schröder [:mschroeder]]

(In reply to comment #5)
The patch also solves this problem and takes previous weeks in view into account.

Comment 7

[Martin Schröder [:mschroeder]]

*** Bug 336986 has been marked as a duplicate of this bug. ***

Comment 8

[Joey Minta]

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

Comment 9

[Martin Schröder [:mschroeder]]

Attachment: Patch v3

moved the comment as suggested
carried over r=jminta

Comment 10

[Matthew (lilmatt) Willis]

Patch checked in on trunk and MOZILLA_1_8_BRANCH

Comment 11

[Stefan Sitter [:ssitter]]

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+

Comment 12

[Martin Schröder [:mschroeder]]

Attachment: Fix for correct date in first week of view

Fixes the problem with different week start days.

The variable firstWeekDate should be relative to aDate, which is the selected day.

Comment 13

[Joey Minta]

Comment on attachment 222269 [details] [diff] [review]
Fix for correct date in first week of view

r=jminta

Comment 14

[Joey Minta]

follow-up patch checked in.

Comment 15

[Damian Szczepanik]

verivied with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060929 Sunbird/0.3

Comment 16

[Damian Szczepanik]

Litmus testcase 2616 created