Closed Bug 794585 Opened 7 years ago Closed 7 years ago
Monthly layout: unnecessary additional empty week row printed if last day of month is last day of print range
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.
This should do it, I'd appreciate if you could give it a spin.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #666909 - Flags: review?(ssitter)
Comment on attachment 666909 [details] [diff] [review] Fix - v1 Patch is empty.
Attachment #666909 - Flags: review?(ssitter) → review-
Sorry about that
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
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.
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.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0
Target Milestone: 2.0 → 1.9
You need to log in before you can comment on or make changes to this bug.