Closed Bug 794585 Opened 9 years ago Closed 9 years ago

Monthly layout: unnecessary additional empty week row printed if last day of month is last day of print range


(Calendar :: Printing, defect)

Lightning 1.7
Not set


(Not tracked)



(Reporter: ssitter, Assigned: ssitter)



(Keywords: regression)


(2 files, 2 obsolete files)

An unnecessary additional empty week is printed in Monthly layout if last day of month is last day of print range. See attached screenshot. The monthly calendar view on the left is correct, the monthly print output on the right contains the unnecessary additional empty week row.
Attached image screenshot β€”
Attached patch Fix - v1 (obsolete) β€” β€” Splinter Review
This should do it, I'd appreciate if you could give it a spin.
Assignee: nobody → philipp
Attachment #666909 - Flags: review?(ssitter)
Comment on attachment 666909 [details] [diff] [review]
Fix - v1

Patch is empty.
Attachment #666909 - Flags: review?(ssitter) → review-
Attached patch Fix - v1 (obsolete) β€” β€” Splinter Review
Sorry about that
Attachment #666909 - Attachment is obsolete: true
Attachment #666924 - Flags: review?(ssitter)
Comment on attachment 666924 [details] [diff] [review]
Fix - v1

From a first test at least two problems:

1) Weekly print layout seems completely broken with the patch, shows only blank page and the following error:

> Error: 'TypeError: parentNode is undefined' when calling method:[calIPrintFormatter::formatToHtml] = NS_ERROR_XPC_JS_THREW_JS_OBJECT
> Source file: chrome://calendar/content/calendar-print-dialog.js Line: 208

2) Monthly print layout testcase fails:

First day of week is Sunday (default). Calendar view is set to month September and shows 6 weeks (26.08.2012 - 06.10.2012).

Without patch: Print output in monthly format shows the very same -> OK.

With patch: Print output doesn't contain the last day of September anymore, shows only 5 weeks (26.08.2012 - 29.09.2012). In addition a second calendar is printed for October that contains 5 more weeks (30.09.2012 - 03.11.2012).
Attachment #666924 - Flags: review?(ssitter) → review-
I don't understand :-( parentNode is the weekContainer, which is a html element on the template. Why would changing the weekEnd (which is evaluated after getting the element) remove the weekContainer!?

I'm sorry, but I'll have to take care of this after the release...
Out of curiosity: Why was userWeekStart() and userWeekEnd() introduced for the printing code? Can't we just reuse getStartOfWeek() and getEndOfWeek() from calWeekInfoService?
I'm working on it.
Assignee: philipp → ssitter
Blocks: 757902
Patch uses the existing functions to get first and last day of the week and the removes the duplicates that were added with Bug 757902.

Patch was tested mostly for Sunday and Monday as first day of week, so far it looks good. 

The TypeError in Comment 5 happened only if Monday was selected as first day of week. I think it was caused by the usage of calIDateTime::startOfWeek that always returns the Sunday of the week. I assume this somehow messed up the created document.

It seems that fixing this error fixed the behavior described in Bug 792061 too.
Attachment #666924 - Attachment is obsolete: true
Attachment #668155 - Flags: review?(philipp)
Attachment #668155 - Flags: approval-calendar-aurora?
Blocks: 792061
Comment on attachment 668155 [details] [diff] [review]
fix for monthly and weekly print

Looks good, I've also done some simple tests and it seems to work. Approval for aurora, if you push this after the merge (which is today, or was today, not sure) then also approval for beta.
Attachment #668155 - Flags: review?(philipp)
Attachment #668155 - Flags: review+
Attachment #668155 - Flags: approval-calendar-beta+
Attachment #668155 - Flags: approval-calendar-aurora?
Attachment #668155 - Flags: approval-calendar-aurora+
Pushed to
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0
Pushed to
Target Milestone: 2.0 → 1.9
You need to log in before you can comment on or make changes to this bug.