The default bug view has changed. See this FAQ.

All day events should be printed at the top

RESOLVED FIXED in 3.8

Status

Calendar
Printing
RESOLVED FIXED
10 years ago
2 years ago

People

(Reporter: Damian Szczepanik, Assigned: Merouane Atig, Mentored)

Tracking

(Depends on: 1 bug)

unspecified

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070308 Calendar/0.6a1

Bug 349960 has been verified for displaying but the problem still exists for printing

Reproducible: Always

Steps to Reproduce:
1. Create event, set from and to time
2. Create second event: the same date, mark as "All day"
3. Print it, use monthly or weekly view
Actual Results:  
All day event is not printed first

Expected Results:  
All day events should be printed at the top as in bug 349960

Comment 1

10 years ago
Damian,

I know this bug, and it is one of the reasons I developed my printer extension. As long as it is not fixed in Calendar, I would encourage you to use the extention. You can download it from: http://www.grassmann.info/fgPrinters.xpi

As for the actual fix in the source:

I found that adding the following lines to compareItems in calMonthGridPrinters.js and calWeekPrinter.js will do the trick:


        if (a instanceof calIEvent) {
			if (a.startDate.isDate && b.startDate.isDate)
			{
				return a.startDate.compare(b.startDate);
			}
			if (a.startDate.isDate)
			{
				var tmpd = b.startDate.clone();
				tmpd.isDate = true;
				tmpd.normalize();
				if (a.startDate.compare(tmpd) == 0) return -1;
			}
			if (b.startDate.isDate)
			{
				var tmpd = a.startDate.clone();
				tmpd.isDate = true;
				tmpd.normalize();
				if (b.startDate.compare(tmpd) == 0) return 1;
			}
        }

This should be placed right before the "if (a instanceof calIEvent) {"-line.
Assignee: nobody → mschroeder
OS: Windows XP → All
Hardware: PC → All
Status: NEW → ASSIGNED
Assignee: mschroeder → nobody
Status: ASSIGNED → NEW

Comment 2

7 years ago
I have only done a little testing but at first glance the patch for bug 551814 seems to fix this problem too.

Comment 3

7 years ago
Yes it does fix it. I have noticed this problem while printing "All day" events and regular events. This fix is included in bug fix https://bugzilla.mozilla.org/show_bug.cgi?id=551814
Depends on: 551814
Whiteboard: [good first bug]
Assignee: nobody → riteshnpatel1994
Status: NEW → ASSIGNED

Comment 4

3 years ago
Created attachment 825342 [details] [diff] [review]
calender
Flags: needinfo?(damian.publicemail)
Hi Ritesh, have you tried this code in Lightning? I'd appreciate if you could put this together as a patch, feel free to email me for details.
Ritesh, are you interested in bringing this into core Lightning? I would very much appreciate, please contact me for details.
Flags: needinfo?(damian.publicemail) → needinfo?(riteshnpatel1994)
Whiteboard: [good first bug] → [good first bug][mentor=Fallen][lang=js]
Assignee: riteshnpatel1994 → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(riteshnpatel1994)
Mentor: philipp@bugzilla.kewis.ch
Whiteboard: [good first bug][mentor=Fallen][lang=js] → [good first bug][lang=js]
(Assignee)

Comment 7

3 years ago
I'm interested in fixing this behavior. I already have a working development environment for Thunderbird+Lightning.

I'm new to contributing to Mozilla projects and I still struggle with the architecture of Thunderbird. It seems to me that the fix suggested in Comment 1 could be implemented in calPrintUtils.jsm comparePrintItems method but I'm not sure.

Could someone give me some directions?
Hey Merouane, I'm happy to hear you'd like to fix this issue! comparePrintItems looks like the right method to fix this in. I'd suggest testing it there and if it works and you are still motivated, maybe you could do some refactoring so that both the calendar views and the print view shares the same method to compare items.

There is a comparator here for example, and there are surely also other comparators for the other views.

http://mxr.mozilla.org/comm-central/source/calendar/base/content/calendar-month-view.xml#228

For simplicity you could put the common method in calUtils.jsm, but eventually we probably want to move this to calViewUtils.jsm (bug 409950).

If you'd prefer to only do it in the print view for now that is fine with me too.

If you'd like ad-hoc help, please find me on irc.mozilla.org #calendar, my nickname is Fallen. You can also email me if you like.
Assignee: nobody → yahoocbien-dev
Status: NEW → ASSIGNED
(Assignee)

Comment 9

3 years ago
Created attachment 8505021 [details] [diff] [review]
bug_373562.patch

I am not sure I factorized all items comparators into one. I searched for others but I did not find any.
Attachment #8505021 - Flags: review?(philipp)
Comment on attachment 8505021 [details] [diff] [review]
bug_373562.patch

Review of attachment 8505021 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great! Could you upload a new patch that has the following comments fixed, and preferably with a commit message like:

"Fix bug 373562 - All day events should be printed at the top. r=philipp"

::: calendar/base/modules/calViewUtils.jsm
@@ +3,5 @@
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +Components.utils.import("resource://calendar/modules/calUtils.jsm");
> +
> +EXPORTED_SYMBOLS = ["calView"];

I'd suggest calling it cal.view and then just exporting cal, just like in the other files.

@@ +14,5 @@
> +    - * @param b     The second item
> +    - * @return      The usual -1, 0, 1
> +    - */
> +  compareItems: function(a, b) {
> +    if (!a) return -1;

We use 4 space indent in all calendar js files.

::: calendar/test/unit/test_view_utils.js
@@ +10,5 @@
> +  test_compare_startdate();
> +  test_compare_enddate();
> +  test_compare_alldayevent();
> +  test_compare_title();
> +  test_compare_todo();

Thank you so much for writing a test for this! We have far too few of them :-)
Attachment #8505021 - Flags: review?(philipp) → review+
(Assignee)

Comment 11

3 years ago
Created attachment 8510606 [details] [diff] [review]
bug_373562_v2.patch
Attachment #8505021 - Attachment is obsolete: true
Attachment #8510606 - Flags: review?(philipp)
Comment on attachment 8510606 [details] [diff] [review]
bug_373562_v2.patch

Review of attachment 8510606 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay, I could have done this much earlier since its just a followup from the previous review and I don't really need to take a deep look. I'm going to set checkin-needed so this can be pushed as soon as the tree is open again.
Attachment #8510606 - Flags: review?(philipp) → review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/01cf9234f2fb
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 3.8
You need to log in before you can comment on or make changes to this bug.