Closed Bug 473547 Opened 11 years ago Closed 11 years ago

'Start the week on' functionality is broken in multiweek/month view

Categories

(Calendar :: Calendar Views, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: andreas.treumann, Assigned: berend.cornelius09)

References

Details

(Keywords: regression)

Attachments

(3 files)

STEPS TO REPRODUCE:
===================

- open the Edit/Preferences (tools/options) dialog
- go to the views tabpage
- change 'Start the week on', e.g. 'Wednesday'
- switch to multiweek/month view

RESULT:
=======

- the startday of the week isn't changed
- wrong days are marked grey (non workweek days)

EXPECTED RESULT:
================

- Wednesday should be the startday of the week
- Saturday and Sunday should be Grey

REPRODUCIBLE:
=============

- always

last build without this bug: 20090105
first with bug: 20090106

possible caused by Bug 447683
Flags: blocking-calendar1.0?
Flags: blocking-calendar1.0? → blocking-calendar1.0+
Attached patch patch v. #1Splinter Review
regression from bug 447683 - Use full weekday name in Day and Week views' captions where I obviously removed this functionality at the same location without providing a substitutional implementation.
Assignee: nobody → Berend.Cornelius
Status: NEW → ASSIGNED
Attachment #357682 - Flags: review?(philipp)
Attachment #357682 - Flags: review?(philipp) → review+
Comment on attachment 357682 [details] [diff] [review]
patch v. #1

>diff --git a/calendar/base/content/calendar-month-view.xml b/calendar/base/content/calendar-month-view.xml
>--- a/calendar/base/content/calendar-month-view.xml
>+++ b/calendar/base/content/calendar-month-view.xml
>@@ -724,6 +724,14 @@
> 
>       <method name="relayout">
>         <body><![CDATA[
>+         // Adjust headers based on the starting day of the week, if necessary
>+         if (this.labeldaybox.firstChild.weekDay != this.weekStartOffset) {
>+             let i = 0;
>+             for each(header in this.labeldaybox.childNodes) {
>+                 header.weekDay = (i++ + this.weekStartOffset) % 7;
You can't iterate childNodes like that, it will also iterate the "length" property. Either use the upcoming iteratators (that are not reviewed yet, requires a mail reviewer), or use for(var i = 0; i < ...; i++) { }

Watch for missing spaces.

r=philipp
pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/90cc9f3ab09f

->fixed
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Duplicate of this bug: 474601
This bug is marked as RESOLVED VERIFIED, but it has NOT been completely resolved.  Changing the "Start the week on" option now affects the Month and Multiweek views (as well as the Week view).  However, in the Month and Multiweek views, the column titles (Monday, Tuesday, etc) are updated incorrectly and are only displayed correctly after re-starting Shredder.

Note, I am now using: 
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090121 Lightning/1.0pre(2009-01-21-04) Shredder/3.0b2pre(2009-01-21)
Attached image screenshot of failure
Confirmed, issue is not fixed completely.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch patch #2Splinter Review
Sorry, when I considered Philipp's comment #2 I forgot to remove the "++" iterator.
Attachment #358369 - Flags: review?(philipp)
Attachment #358369 - Flags: review?(philipp) → review+
patch pushed to comm-central:

http://hg.mozilla.org/comm-central/rev/5e4678c6114c

->fixed
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Checked in lightning and sunbird build 20090125 -> VERIFIED.
Status: RESOLVED → VERIFIED
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.