Should be possible to print tasks

RESOLVED FIXED in 1.0b2

Status

RESOLVED FIXED
13 years ago
4 years ago

People

(Reporter: ssitter, Assigned: eduardokatayama)

Tracking

(Depends on: 2 bugs, {student-project})

Trunk
1.0b2
student-project
Dependency tree / graph
Bug Flags:
blocking-calendar1.0 -
wanted-calendar1.0 +

Details

(Whiteboard: [good first bug])

Attachments

(7 attachments, 9 obsolete attachments)

2.72 KB, patch
Details | Diff | Splinter Review
34.94 KB, application/octet-stream
Details
17.71 KB, patch
Details | Diff | Splinter Review
14.36 KB, image/png
Details
32.49 KB, patch
Details | Diff | Splinter Review
29.03 KB, patch
Fallen
: review+
Details | Diff | Splinter Review
166.54 KB, image/png
Details
(Reporter)

Description

13 years ago
When clicking into the task list the Print command toolbar button and the Print menu entry are disabled. This is done in
http://lxr.mozilla.org/mozilla/source/calendar/resources/content/unifinderToDo.js#237
http://lxr.mozilla.org/mozilla/source/calendar/resources/content/unifinderToDo.js#245

But there is no code to enable the Print command if you select the views or the event list or ... Thus the Print command stays disabled until program restart.

Comment 1

13 years ago
Created attachment 216282 [details] [diff] [review]
task-list printing

After removing the disabling code, I realized that we probably should offer some similar way to print tasks.  This patch introduces a second category of print-formatters, one designed for tasks.  (Note that the month-grid printer can be registered in both categories.)  It also implements a version of the hCalendar exporter for tasks.  I could not find a standard definition of what to do with VTODOs, so I improvised a bit.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #216282 - Flags: first-review?(mvl)

Comment 2

13 years ago
Created attachment 216283 [details] [diff] [review]
real patch

There were a couple of small proof-reading changes that didn't make it into the previous patch.  This is the real thing.
Attachment #216282 - Attachment is obsolete: true
Attachment #216283 - Flags: first-review?(mvl)
Attachment #216282 - Flags: first-review?(mvl)
This patch looks like a copy of the current html printer. Three questions come to mind:
Can this be structured just like the event list printer, so that you can also export the task list?
Why use a different filename scheme? (-Printer instead of -Formatter)
Is there a way to unify more code?

The rest of this patch looks ok (but I didn't test yet)

Comment 4

13 years ago
*** Bug 335244 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Flags: blocking0.3+

Comment 5

13 years ago
(In reply to comment #3)
> This patch looks like a copy of the current html printer. Three questions come
> to mind:
> Can this be structured just like the event list printer, so that you can also
> export the task list?
It can, however, a couple of issues come to mind.  1.) We don't have a clear picture of what printing options we want to expose and how to expose them.  2.) The hCalendar spec I was looking at only described VEVENTs, I didn't know if there was a spec I was supposed to work off of for VTODO, or whether I would be violating the spec by including the VTODO data.

> Why use a different filename scheme? (-Printer instead of -Formatter)
Because I only wrote it thinking of printing, not exporting.  If we want to do export too (where we again need to consider how we're going to expose options), then I agree it should be called Formatter.
> Is there a way to unify more code?
That depends on the answers to the above questions.

Updated

13 years ago
Whiteboard: [patch in hand]
(In reply to comment #5)
> 1.) We don't have a clear
> picture of what printing options we want to expose and how to expose them.  

That doesn't take away the fact that we can prepare the backend code to already allow exporting. The front-end can come later. And if we do it the right way now, we don't have to rename files etc then.

> 2.) The hCalendar spec I was looking at only described VEVENTs,

If it isn't in the hCalendar spec, we just invented a new spec, called hTodo. Really, hCalendar looked flexible enough. And I really don't see how adding some  class names will hurt anything. We just need to make sure the resulting hTodo block is different enough from a hEvent block to bot be confused.

Comment 7

13 years ago
I'd like to get UI-Review on the way we differentiate between printing events and tasks here before pressing forward with this patch.
Whiteboard: [patch in hand] → [cal-ui-review needed]
(Reporter)

Comment 8

13 years ago
Just for the notes: The intention of this bug was not to disable the print command. I think that is all that needs to be fixed for 0.3. Printing of tasks is something that can be solved after 0.3 in my opinion.
Whiteboard: [cal-ui-review needed] → [cal-ui-review needed][l10n impact]
This bug seems to have morphed into "we should be able to print todos", which we've agreed entrains some UI discussion and therefore shouldn't block 0.3.  However, the bug as filed probably should block 0.3.  In recognition of the fact that most of the discussion in this bug centers around printing todos, I'm gonna spin off the "Print command disabled" to a separate bug.
Flags: blocking0.3+ → blocking0.3-
Keywords: uiwanted
Whiteboard: [cal-ui-review needed][l10n impact] → [l10n impact]
Spun off to bug 351460.
Summary: Print command disabled after click in task list until program end → Should be possible to print tasks
Comment on attachment 216283 [details] [diff] [review]
real patch

Moving first-review to Clint, since mvl's queue is pretty big and he can only
cope with his ui-reviews and 2nd reviews.
Attachment #216283 - Flags: first-review?(mvl) → first-review?(ctalbert.moz)
Component: Sunbird Only → Tasks
QA Contact: sunbird → tasks

Comment 12

12 years ago
Comment on attachment 216283 [details] [diff] [review]
real patch

This patch has bitrotted. More importantly, I do not believe that we should have a separate "print tasks" command, so I disagree with the architecture of the patch. I think that the ability to print tasks as well as events on one page would be desireable, especially if the user has the "show tasks in view" option set.  I think that a better solution can be created that better melds events with tasks under the guise of one Print command with several different layouts.  For example, perhaps a weekly printing of events with a task list in a column on the right side of the printout.

Sorry, but I feel this needs more work. 
ctalbert r-
Attachment #216283 - Flags: first-review?(ctalbert.moz) → first-review-
(Reporter)

Updated

12 years ago
Component: Tasks → Printing
QA Contact: tasks → printing

Comment 13

12 years ago
Re-assigning my bugs to nobody@mozilla.org due to recent developments.
Assignee: jminta → nobody
Status: ASSIGNED → NEW
Duplicate of this bug: 296517
(Reporter)

Updated

11 years ago
Duplicate of this bug: 396933
(Reporter)

Updated

11 years ago
Duplicate of this bug: 428281
(Reporter)

Updated

11 years ago
Whiteboard: [l10n impact]
Version: Trunk → Mozilla 1.8 Branch
(Reporter)

Updated

11 years ago
Duplicate of this bug: 429254

Comment 18

11 years ago
I'd like to toss in a comment on this topic. The ability to print tasks as an independent item is crucial to the task pane being usable. In most organizations I have worked, people use the tasks as mini-project trackers: i.e., more than just 'remember to do this' reminders. For example, I may create a task to track crucial events of a products introduction and periodically update the detail of the task based on events, (i.e. the task is the master item with events being subservient to the task) but need the ability to print the task either individually, or at least print all tasks at once with each task on a different page. Also, I will attach the task to emails to circulate for updates. Just my two cents, as this one item is keeping me from ditching Outlook altogether. Thx, Mike

Comment 19

11 years ago
(In reply to comment #18)
I have to agree - for business use, tracking individual tasks/assignments is very useful, but without the ability to print these tasks, the usefulness of Sunbird is very reduced in my mind.  Additionally, for personal use, I believe many people use tasks as home to-do lists, and my wife would even like to use it as a place to make and update shopping lists, so again, printing should be included for tasks.
We are having several reports in our community forums about this issue, is somebody working on it?

Comment 21

10 years ago
I have never downloaded and installed a patch so I would be unsure exactly how I would do this. I would hope that it would be a completely automated process, just download, press the install button restart Thunderbird and it will work perfectly first time. I can print tasks now but it is a bit fiddly, all you need to do is hold down the ctrl key and click on the Tasks that you wish to print, then go ctrl-p and the print dialogue box box appears, and in the "What to Print" box check "Selected Events" the tasks that have been selected will appear in the print preview. This method does work although there is no scope to format what is being printed, I think that I will stick with this method until I can be sure that I will not totally mess things up with by trying to install a patch. It would be better if you could just download it as an update.

Comment 22

10 years ago
I just wanted to add another note to this. I am using Thunderbird 2.0.0.16 with Lightning nightly updates installed. As of the September 9 update, the "Print selected events" is still grayed out and cannot be selected. It has been this way for a long time. The ability to print tasks is critical to the task function being used effectively, as many users utilize a task as a mini-project list.
Is anyone working on this? This bug renders the whole application useless for me.

Updated

10 years ago
Flags: wanted-calendar1.0+
Flags: wanted-calendar1.0+ → wanted-calendar1.0?
Version: Mozilla 1.8 Branch → Trunk

Comment 24

10 years ago
Requested feature when "printing tasks" is added to Lightning/Sunbird:
When printing tasks within a user-specified data range, it is highly desirable to offer an option to print the recurrence pattern rather than just an indication that it is recurring.  For example:  "Occurs the first Sunday of November, effective 11/4/2007".
Nominating for blocking1.0.

IMO, printing out a daily, weekly or monthly schedule is a key requirement for a 1.0 release. That schedule encompasses your current tasks in my view.
Flags: blocking-calendar1.0?
Keywords: helpwanted
Whiteboard: [good first bug]

Comment 26

10 years ago
I do absolutly agree with this statement. Printing tasks is a basic need.
Although I agree that printing is a quite basic need, unless we have more developers to work on this it won't happen for 1.0. Printing is not an area of focus for the current release.
Flags: wanted-calendar1.0?
Flags: wanted-calendar1.0+
Flags: blocking-calendar1.0?
Flags: blocking-calendar1.0-

Comment 28

10 years ago
"Printing is not an area of focus for the current release."  For a 1.0 release?  Kidding right?  *sigh*

Comment 29

10 years ago
Isn't it about time that this problem was fixed once and for all? I realize that it is been done by volunteers but it has been ongoing for years, I would like to click the update button then install updates and find that I am able to print 'Tasks' and the results look like a page from a business diary like in MS OutLook.
Duplicate of this bug: 487172
This might make a nice student project. Our printing support in general is very primative, depending on the amount of time the student has, he or she may also like to fix a few bugs related to printing events in the current format, or even create new formats.
Keywords: student-project
A group of three students from Brazil in a course about open source is going to be working on this bug as a student-project. Looking forward to your patch(es), thanks Eduardo!
Assignee: nobody → eduardokatayama

Comment 33

9 years ago
Created attachment 401399 [details]
AddOn for Lightning 0.8/0.9 to support printing of tasks

I just wrote an AddOn to support printing of tasks by overriding "getEventsAndDialogSettings()", as outlined in attachment 216283 [details] [diff] [review].

I chose to not differ between 2 print Menu items, but to provide printing of tasks in MonthGrid and WeekGrid. As the current implementation "List" is not capable of printing tasks without Start date or Due date, I included a fourth print layout "tasks".

This is only a workaround for people waiting too long for this and offers only a simple list. Also the wording is not optimal, but I wanted to use only existing strings to support all locales.

Furthermore it could be modified to work with Sunbird as well, but I don't need this. It would only require to change INSTALL.RDF to include Sunbird, but needs to remove Lightning requirement.

Source:
https://addons.mozilla.org/de/thunderbird/addon/5285

Comment 34

9 years ago
Created attachment 401809 [details] [diff] [review]
Fix loop through sorted list for Week and Month Grid Printer

Decathlon pointed out a problem with the existing Week and Month Grid Printers - they sort all tasks before events, but then break whenever a start date is higher than the current day.

This wipes out all events that are before the last task's start day.
The attached patch would just continue on tasks and only break on the first event after the current day.

Comment 35

9 years ago
Created attachment 401815 [details]
Update to fix the mentioned bug in attachment #401809 [details] [diff] [review]

This should now also work with Sunbird.
Attachment #401399 - Attachment is obsolete: true

Updated

9 years ago
Attachment #401815 - Attachment description: Update to fix the mentioned bug in attachement #401809 → Update to fix the mentioned bug in attachment #401809
Sven, I'd appreciate if you could collaborate with Eduardo and friends on how this code could help them with their student-project and maybe how this could be integrated into core code.

Comment 37

9 years ago
Created attachment 402081 [details] [diff] [review]
WIP patch to print a simple task list and tasks in Week and Month Grid

I preferred to upload my WIP patch for everyone here.

I payed attention to Comment #3 and Comment #12, therefore obsoleting attachment #216283 [details] [diff] [review].

This is a very early WIP, but already working in production on my site. The locales should be replaced by new ones (rather then stealing from different dialogs) and there is still one issue outstanding:

* I did not try to get any selected Tasks - I'm happy with disabling this with the "task list" layout.

But since the core code of the effected files did not change since version 0.9, it should also work fine with current nightly builds (I did only check with 0.5, 0.8 and 0.9 - but 0.5 misses some of my stolen locales and handles completed tasks differently).
Attachment #216283 - Attachment is obsolete: true

Comment 38

9 years ago
Created attachment 402082 [details]
Screenshot for Attachment #402081 [details] [diff]

Sorry for German locales, did not have time to change it...
(Assignee)

Comment 39

9 years ago
Created attachment 405288 [details] [diff] [review]
patch to print tasks
Attachment #405288 - Flags: review?(philipp)
Comment on attachment 405288 [details] [diff] [review]
patch to print tasks 

Hello Eduardo and Friends,

sorry for the delay, I've had real life things to do. First of all I'd like to thank you for the effort you have put into this. I'm confident we can have this patch go into the next beta of Lightning. The current beta won't work since strings are changed and we are already in string freeze.


First of all, I'd like to give you a code level review. I'm sorry if you feel this is being picky, but to keep the code readable its necessary to make sure that all code follows the same standards. We have a few hints on https://wiki.mozilla.org/Calendar:Style_Guide that might help you, but I've marked most nits here.

As a next step, I will be testing your patch. I hope to get to that soon, maybe this weekend, otherwise starting next week. Then I'll give you another round of comments and I'm sure we will come to a positive review soon after that.

>  *
>  * Contributor(s):
>  *   Matthew Willis <mattwillis@gmail.com>
>+ *	 Diego Mira David <diegomd86@gmail.com>
Please make sure the indentation is correct here and in other files. This is probably a tabs vs spaces issue.

>+    //Tasks with no due date
Add a space after opening comments, i.e // Tasks with no...

>+    function isTaskWithoutDueDate(item) {
>+	return !item.dueDate && !item.endDate;
>+    }
The new JS allows a shorter syntax:

function isTaskWithoutDueDate(item) !item.dueDate && !item.endDate;


>+    var filteredItems = aItems.filter(isTaskWithoutDueDate);
Please use "let" instead of "var", here and elsewhere

>+    if (filteredItems.length == 0)
>+	return "";
Brackets even for one-line blocks.

>+    var monthName = calGetString("dateFormat", "month." + (date.month +1)+ ".name");
spaces arounds operators (i.e (date.month + 1) + ".name")

>+	if (task.isCompleted) {
>+		taskItem.appendChild(<input checked="checked" type="checkbox"/>);
>+		taskItem.appendChild(<s>{task.title}</s>);
Use 4 spaces instead of tabs

>+	}
>+	else {
} else {



>+function getTasks(aItems) {
It seems you use this function more than once. Instead of copying it, I'd prefer to keep it somewhere common and just call it from each location. Correct me if I'm wrong though.

>-<!ENTITY calendar.print.currentview.label "Events in current view">
>+<!ENTITY calendar.print.currentview.label "Current view">
>-<!ENTITY calendar.print.selected.label "Selected events">
>+<!ENTITY calendar.print.selected.label "Selected events/tasks">
When you change the contents of a string, you need to also change the entity name. Otherwise, localizers will not take notice of the changed string.

> <!ENTITY calendar.error.detail "Details…">
This may be a different glitch, but please be sure the patch you upload is saved in UTF-8 format, otherwise special characters will not show correctly.

>+++ b/calendar/resources/content/print.css	Thu Oct 08 14:10:20 2009 -0300
>+ * Contributor(s): Diego Mira David <diegomd86@gmail.com>
>+ *                 Eduardo Teruo Katayama <eduardo@ime.usp.br>
>+ *                 Glaucus Augustus Grecco Cardoso <glaucus@ime.usp.br>
Please align your names under the "n" of Contributor(s).

>+		  		settings.end.day = settings.end.day + 1;
settings.end.day += 1 should be fine here. Check the alignment in this file also, it looks very strange on bugzilla.

>+          settings.end   = jsDateToDateTime(document.getElementById("end-date-picker").value);
>+          settings.end   = settings.end.getInTimezone(currentTimezone);
Too many spaces, settings.end = settings....
> }
> 
>+
>+
> function printAndClose()
> {
No need to add extra newlines, here and shortly after

>+++ b/calendar/resources/jar.mn	Thu Oct 08 14:10:20 2009 -0300
>+    content/calendar/print.css                             (content/print.css)
This needs to be tested, but I think the print stuff is happening from "content", so you might need to add the css file to a package that has contentaccessible=yes. I'll double check this in the next round of review.



I'm marking your patch as "r-" for now, which means review was denied. This isn't meant personally, I'd just like to see a new patch with nits fixed and possibly also with the upcoming test review comments fixed.


Sven, are you going to continue working on your patch? I really would like to avoid double work, so please collaborate on this (!) and let me know what the outcome is.
Attachment #405288 - Flags: review?(philipp) → review-

Comment 41

9 years ago
Philipp,

I was in contact with Eduardo to avoid double work. I did not reassign this bug at the time of writing the patches - I only uploaded them to share my code...
I stopped working on this after having a quick & dirty solution for my users issue - I believe Eduardo will give a much cleaner and time consuming solution here!
Status: NEW → ASSIGNED
(Assignee)

Comment 42

9 years ago
Created attachment 410887 [details] [diff] [review]
Patch to print tasks
Attachment #405288 - Attachment is obsolete: true
Attachment #410887 - Attachment is patch: true
Attachment #410887 - Attachment mime type: application/octet-stream → text/plain
(In reply to comment #42)
> Created an attachment (id=410887) [details]
> Patch to print tasks

Eduardo, you have to ask for review from Philipp (:Fallen) when you attach a new patch that you want him to look at. :)
Attachment #410887 - Flags: review?(philipp)
Attachment #410887 - Flags: review?(philipp) → review-
Comment on attachment 410887 [details] [diff] [review]
Patch to print tasks 



>diff -r 681e87a38edb calendar/base/modules/calUtils.jsm
>     getDragService: generateServiceAccessor("@mozilla.org/widget/dragservice;1",
>                                                 Components.interfaces.nsIDragService),
> 
>+
>+
>     /**
>      * Loads an array of calendar scripts into the passed scope.

Please clean out redundant changes like these before uploading your patch. Also, in your other files, please try to avoid too many new lines. For example:

>  * ***** END LICENSE BLOCK ***** */
> 
>+Components.utils.import("resource://calendar/modules/calTaskUtils.jsm");
>+ 
>+ 
> /**

One newline between Components... and /** is sufficient.

Also, there are still tabs and other things in your patch. Please replace all tabs with 4 spaces, remove whitespaces at the end of added or changed lines, and make sure you don't have redundant newlines. These are the most important, please take another look at the style guide. 

>-<!ENTITY calendar.print.currentview.label "Events in current view">
>+<!ENTITY calendar.print.currentView.label "Current view">
I'm not sure changing case is sufficient to make this show up as a new locale string. If you are out of ideas for a label name, go ahead and take ...print.currentView2.label.

>-<!ENTITY calendar.error.detail "Details…">
>+<!ENTITY calendar.error.detail "Details...">
It seems your text editor is not set to save in UTF-8 mode. The character you saw was not three dots, but one character with three dots on it. Please leave these in place.

>+++ b/calendar/resources/content/print.css	Fri Nov 06 21:43:47 2009 -0200
We're trying to move files out of resources/content. I'd suggest to put this file in base/themes/common/calendar-printing.css and make sure you update the jar.mn

>+}
>\ No newline at end of file

Also make sure you save this file so it has a newline at the end of the file. Editors like Netbeans don't do this, you need to insert an extra newline in such editors.

>diff -r 681e87a38edb calendar/resources/content/printDialog.js
>diff -r 681e87a38edb calendar/resources/content/printDialog.xul
We have moved these files to base/content/dialogs, please update your patch.


>-      <groupbox id="whatToPrintGroup">nk
>+     </groupbox>
>+      <groupbox>
Please make sure the groupbox has an id, so that extensions can hook into this dialog easier. The same goes for the <grid>, <columns>,<column>,<rows>,<row>,<caption>,<splitter>. We usually use quite verbose ids here, check our other xul files for examples.


Since this patch changes strings, we can't land it before the beta. I'd appreciate a new patch with comments fixed, since the old one doesn't apply. r- for now to get a new patch. Keep up the good work though, you are heading in the right direction!

Please request review from me on the next patch. Do so by setting the "review" dropdown to "?" and then entering ":Fallen" into the textbox.
Created attachment 414140 [details]
Lint warnings for attachment 410887 [details] [diff] [review]

Oh, just to give you an idea what style fixes need to minimally be made, here the output of my lint script.
(Assignee)

Comment 46

9 years ago
Created attachment 417391 [details] [diff] [review]
[long,test] Patch for bug 325137
Attachment #410887 - Attachment is obsolete: true
Attachment #417391 - Flags: review?(philipp)
Comment on attachment 417391 [details] [diff] [review]
[long,test] Patch for bug 325137

Eduardo, thank you very much for the new patch. I'm one of the users who would love to see this bug fixed, so I appreciate the work you do! I just thought maybe I can help to ease Philipp's review by identifying the last few coding style nits (unfortunately I can't help with the code itself). Eduardo, perhaps you can update your patch with these nits fixed?

>+    case '': //just in case
>+        settings.start = theView.startDay;
>+        settings.end   = theView.endDay;

Pls remove spaces:
settings.end = theView.endDay;

>    -   Martin Schroeder <mschroeder@mozilla.x-home.org>
>+   -                 Joey Minta <jminta@gmail.com>
>+   -                 Diego Mira David <diegomd86@gmail.com>
>+   -                 Eduardo Teruo Katayama <eduardo@ime.usp.br>
>+   -                 Glaucus Augustus Grecco Cardoso <glaucus@ime.usp.br>

Pls remove spaces to align your names with Martin Schroeder:
    -   Martin Schroeder <mschroeder@mozilla.x-home.org>
+   -   Joey Minta 
[etc.]

>+.main-table {
>+  font-size: 26px;
>+  font-weight:bold;

Pls add space
font-weight: bold;

>+++ b/calendar/locales/en-US/chrome/calendar/calendar.dtd	Sun Dec 13 20:17:32 2009 -0200
>@@ -22,16 +22,19 @@
>    -                 Mike Potter <mikep@oeone.com>
>    -                 Chris Charabaruk 
> [...]
>    -                 Philipp Kewisch <mozilla@kewis.ch>
>+   -                 Diego Mira David <diegomd86@gmail.com>
>+   -                 Eduardo Teruo Katayama <eduardo@ime.usp.br>
>+   -                 Glaucus Augustus Grecco Cardoso <glaucus@ime.usp.br>

It's not your fault, but reading Philipp's comment 40, all of the names (existing and new ones) should be aligned under the "n" of "Contributors", as I'll show in the correction of the following code:

> # Contributor(s): ArentJan Banck <ajbanck@planet.nl>
> #                 Eric Belhaire <belhaire@ief.u-psud.fr>
> #                 Philipp Kewisch <mozilla@kewis.ch>
> #                 Berend Cornelius <berend.cornelius@sun.com>
>+#                 Diego Mira David <diegomd86@gmail.com>
>+#                 Eduardo Teruo Katayama <eduardo@ime.usp.br>
>+#                 Glaucus Augustus Grecco Cardoso <glaucus@ime.usp.br>

Same as above (not your fault), but all the names should be aligned like this:

# Contributor(s):
#   ArentJan Banck <ajbanck@planet.nl>
#   Eric Belhaire <belhaire@ief.u-psud.fr>
[etc.]

These are all very small details, but maybe by fixing the style we can lure Philipp into reviewing the code ;)
Attachment #417391 - Attachment description: Patch for bug 325137 → [long,test] Patch for bug 325137
Comment on attachment 417391 [details] [diff] [review]
[long,test] Patch for bug 325137

Eduardo, calTaskutils.jsm is missing from this patch. Could you create a patch with all files?

You might want to use hg's "mq" extension this makes it easier to add new files, see https://developer.mozilla.org/en/Mercurial_Queues
Attachment #417391 - Flags: review?(philipp) → review-
(Assignee)

Comment 49

9 years ago
Created attachment 430006 [details] [diff] [review]
New patch 

added calTaskutils.jsm
fixed coding style nits from Thomas D.  (Thanks Thomas, sorry for the long delay)
Attachment #417391 - Attachment is obsolete: true
Attachment #430006 - Flags: review?(philipp)
Attachment #430006 - Attachment is patch: true
Attachment #430006 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 430006 [details] [diff] [review]
New patch 

Eduardo, sorry for not noticing earlier, but this time calendar-printing.css is missing from the patch.

Please make sure all new files are added. You might want to look into using mq, this way you can automatically track new files and have them added to your patch.

See https://developer.mozilla.org/en/Mercurial_Queues
Attachment #430006 - Flags: review?(philipp) → review-

Comment 51

9 years ago
Philipp, apart from that fact could you comment on the remaining open issues?

Although I have to admit, that it's important to submit clean patches, this sometimes sounds like only passing the buck to someone else...
Sven, I'm not trying to pass the buck to someone else, I'm sorry if my comments give this impression. I do want to see this fixed and I'm happy to concentrate more on the functional aspects, but to do that I need all files to do the testing.

Could you summarize the functional issues that are still open for discussion? I seem to be missing them due to all of my style comments.
I see the files are in the older patch versions. I'll extract them and see what happens.
Created attachment 434495 [details] [diff] [review]
Combined Patch

I've debitrotted the patch and added calendar-printing.css from the next to last version. The patch works and provides Task printing capabilities as expected. A few points we should look into:

Things I think should be taken care in this bug:

* In the Weekly/Monthly Planer, its possible to check/uncheck the Tasks without due date. I'd suggest to catch the clicks and not let the user change this, since its the preview. Check out the XUL allowevents attribute. If that doesn't work, you may need to add an event listener that stops propagation.

* In the list view, completed tasks are not distinguishable from not completed tasks. I'd suggest the line-through style, or a label (i.e:  a new task (completed))

* It would be nice if you could change the background color for the print preview to white, since the paper will be white too. I'm fine with postponing it to a different bug, but it would be nice to take care here.

Things I think can be taken care of in a different bug (you don't have to fix these yourself, if you don't want to):

* In the list view, tasks without start or due date are specifically labeled as such (in the "When" section). I think we should just drop the "When" section in this case.

* Tasks with the same start and due date are shown as:
   [ 02:00 PM-02:00 PM a new task ]
  If possible I'd prefer just showing:
   [ 02:00 PM a new task ]

* Some other task properties (i.e progress, category, ...) are not shown in the print view.

What do you think? Does this sound reasonable?
Attachment #430006 - Attachment is obsolete: true
Attachment #414140 - Attachment is obsolete: true
(Assignee)

Comment 55

9 years ago
Created attachment 434501 [details] [diff] [review]
New patch with calendar-printing.css

Hello, sorry for forgetting the file, 
Will use mq next time :)
Attachment #434501 - Flags: review?(philipp)
Eduardo, could you reply to comment 54?
(Assignee)

Comment 57

9 years ago
(In reply to comment #54)
> Created an attachment (id=434495) [details]
> Combined Patch
> 
> I've debitrotted the patch and added calendar-printing.css from the next to
> last version. The patch works and provides Task printing capabilities as
> expected. A few points we should look into:
> 
> Things I think should be taken care in this bug:
> 
> * In the Weekly/Monthly Planer, its possible to check/uncheck the Tasks without
> due date. I'd suggest to catch the clicks and not let the user change this,
> since its the preview. Check out the XUL allowevents attribute. If that doesn't
> work, you may need to add an event listener that stops propagation.
> 
> * In the list view, completed tasks are not distinguishable from not completed
> tasks. I'd suggest the line-through style, or a label (i.e:  a new task
> (completed))
> 
> * It would be nice if you could change the background color for the print
> preview to white, since the paper will be white too. I'm fine with postponing
> it to a different bug, but it would be nice to take care here.
> 
> Things I think can be taken care of in a different bug (you don't have to fix
> these yourself, if you don't want to):
> 
> * In the list view, tasks without start or due date are specifically labeled as
> such (in the "When" section). I think we should just drop the "When" section in
> this case.
> 
> * Tasks with the same start and due date are shown as:
>    [ 02:00 PM-02:00 PM a new task ]
>   If possible I'd prefer just showing:
>    [ 02:00 PM a new task ]
> 
> * Some other task properties (i.e progress, category, ...) are not shown in the
> print view.
> 
> What do you think? Does this sound reasonable?
Sure, I'm gonna work on the three first ones. When I finish this, I can work on the laters.
(Assignee)

Comment 58

9 years ago
(In reply to comment #54)
> Created an attachment (id=434495) [details]
> Combined Patch
> 
> I've debitrotted the patch and added calendar-printing.css from the next to
> last version. The patch works and provides Task printing capabilities as
> expected. A few points we should look into:
> 
> Things I think should be taken care in this bug:
> 
> * In the Weekly/Monthly Planer, its possible to check/uncheck the Tasks without
> due date. I'd suggest to catch the clicks and not let the user change this,
> since its the preview. Check out the XUL allowevents attribute. If that doesn't
> work, you may need to add an event listener that stops propagation.
Hello, any problem if I just disable the checkbox? Is it better to do the above?
 


> * In the list view, completed tasks are not distinguishable from not completed
> tasks. I'd suggest the line-through style, or a label (i.e:  a new task
> (completed))
> 
> * It would be nice if you could change the background color for the print
> preview to white, since the paper will be white too. I'm fine with postponing
> it to a different bug, but it would be nice to take care here.
> 
> Things I think can be taken care of in a different bug (you don't have to fix
> these yourself, if you don't want to):
> 
> * In the list view, tasks without start or due date are specifically labeled as
> such (in the "When" section). I think we should just drop the "When" section in
> this case.
> 
> * Tasks with the same start and due date are shown as:
>    [ 02:00 PM-02:00 PM a new task ]
>   If possible I'd prefer just showing:
>    [ 02:00 PM a new task ]
> 
> * Some other task properties (i.e progress, category, ...) are not shown in the
> print view.
> 
> What do you think? Does this sound reasonable?
Regards,
Eduardo
I believe if you just disable the checkbox, it looks gray. I'd prefer a normal looking checkbox that is just not clickable. Go ahead and test it though, it may work.
Attachment #434501 - Attachment is patch: true
Attachment #434501 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 434501 [details] [diff] [review]
New patch with calendar-printing.css

The code for this patch looks great, it would have r+. Removing review request to wait for a patch with the mentioned functional issues.
Attachment #434501 - Flags: review?(philipp)
(Assignee)

Comment 61

9 years ago
Created attachment 435607 [details] [diff] [review]
New patch

Updated:
-> I just disabled the checkbox to test, if you think it needs to be a normal checkbox that is not clickable, just tell me that I will fix it :)
-> In the list view, completed tasks are distinguishable from not completed tasks. (Added a label. i.e:  a new task (completed))
-> Changed the background color for the print preview to white.
-> In the list view, tasks without start and due date dont have a "When" section.
-> Tasks with the same start and due date are shown as: [ 02:00 PM a new task ]

Any idea, about how the task properties should be shown?
Attachment #434501 - Attachment is obsolete: true
Attachment #435607 - Flags: review?(philipp)

Comment 62

9 years ago
(In reply to comment #61)
> -> I just disabled the checkbox to test, if you think it needs to be a normal
> checkbox that is not clickable, just tell me that I will fix it :)
I would prefer a disabled checkbox (as it is now), just to prevent user's calls "Why doesn't this checkbox accept to be checked?"

> Any idea, about how the task properties should be shown?
How do they display now? Any screenshot?
(Assignee)

Comment 63

9 years ago
(In reply to comment #62)
> (In reply to comment #61)
> > -> I just disabled the checkbox to test, if you think it needs to be a normal
> > checkbox that is not clickable, just tell me that I will fix it :)
> I would prefer a disabled checkbox (as it is now), just to prevent user's calls
> "Why doesn't this checkbox accept to be checked?"
> 
> > Any idea, about how the task properties should be shown?
I think it shows title, description, location, date.
I just uploaded a print preview :)

> How do they display now? Any screenshot?
> Any idea, about how the task properties should be shown?
(Assignee)

Comment 64

9 years ago
Created attachment 435810 [details]
SS - print preview - list
Eduardo, thanks for the patience and diligence with which you work on this bug. It will be a great improvement when it lands.

Looking at the current preview (screenshot attachment 435810 [details]), some thoughts:
1) We could save a lot of space (i.e. printout length, paper, and trees...) with a small re-arrangement of the properties. We currently have:

/When/
  When-property
/Location/
  Location-property
/Description/
  Description-property

Re-arranging this into a tabular structure will produce a much more compact printout in many cases, especially when we add more properties as suggested by Philipp's comment 54:

/When:/        When-property
/Location:/    Location-property
/Category:/    Category-property
/Status:/      Status-properties (inkl. % completed)
/Repeat:/      Repeat-property
/Description:/ Description-property

Eduardo, would this be a difficult change? Of course, we should only print properties whose value has been set/changed by user, so e.g. if category is "none", just skip the category-property altogether, as you already do for the when-property.

Benefit of tabular structure: For most cases (with short property-values), we're saving one printed line per property. That's saving up to 6 lines or more per task if all properties are defined. For a lot of cases (property-values of up to ca. 1.6 lines), we're not loosing printed space compared to the current layout. For a few cases (property-values of more than 1.6 lines), we loose a small percentage of space compared to the current layout (e.g. for every 5 lines property-length, we loose one line because of the tabular structure).

Philipp, do you think that we should print Calendar-property and Reminder-property as well? I'm not sure. In the long run, we'll need a way of customizing which properties should be printed or not. But that's surely beyond the scope of this bug. I'd propose to skip Calendar and Reminder for printout until such time that we'll have customizable printout. Otherwise, some people might go mad that we print a full line per task for Calendar-property where they might only have one calendar, or be printing just one calendar out of many.
(Assignee)

Comment 66

9 years ago
(In reply to comment #65)
> Eduardo, thanks for the patience and diligence with which you work on this bug.
> It will be a great improvement when it lands.
> 
> Looking at the current preview (screenshot attachment 435810 [details]), some thoughts:
> 1) We could save a lot of space (i.e. printout length, paper, and trees...)
> with a small re-arrangement of the properties. We currently have:
> 
> /When/
>   When-property
> /Location/
>   Location-property
> /Description/
>   Description-property
> 
> Re-arranging this into a tabular structure will produce a much more compact
> printout in many cases, especially when we add more properties as suggested by
> Philipp's comment 54:
> 
> /When:/        When-property
> /Location:/    Location-property
> /Category:/    Category-property
> /Status:/      Status-properties (inkl. % completed)
> /Repeat:/      Repeat-property
> /Description:/ Description-property
> 
> Eduardo, would this be a difficult change? Of course, we should only print
> properties whose value has been set/changed by user, so e.g. if category is
> "none", just skip the category-property altogether, as you already do for the
> when-property.
> 
> Benefit of tabular structure: For most cases (with short property-values),
> we're saving one printed line per property. That's saving up to 6 lines or more
> per task if all properties are defined. For a lot of cases (property-values of
> up to ca. 1.6 lines), we're not loosing printed space compared to the current
> layout. For a few cases (property-values of more than 1.6 lines), we loose a
> small percentage of space compared to the current layout (e.g. for every 5
> lines property-length, we loose one line because of the tabular structure).
> 
Ok, Will work on this :)

Regards, 
Eduardo

> Philipp, do you think that we should print Calendar-property and
> Reminder-property as well? I'm not sure. In the long run, we'll need a way of
> customizing which properties should be printed or not. But that's surely beyond
> the scope of this bug. I'd propose to skip Calendar and Reminder for printout
> until such time that we'll have customizable printout. Otherwise, some people
> might go mad that we print a full line per task for Calendar-property where
> they might only have one calendar, or be printing just one calendar out of
> many.
I'll comment more elaborately later, but unless its not too much work, I think its sensible to move the change to a tabular layout to a different bug. I'd like to get this fixed soon so we at least have the basics :-)
Attachment #435607 - Attachment is patch: true
Attachment #435607 - Attachment mime type: application/octet-stream → text/plain
I'm about ready to take this patch! I've done some testing and it (almost) seems to work as expected. I have one issue: I can't get the dialog to show completed tasks, regardless of what state the "Completed tasks" checkbox has.

I couldn't quite find out the reason, the code itself looks like it should work. I'll do some further testing, if I find out its a local issue I'll r+ the patch.
Comment on attachment 435607 [details] [diff] [review]
New patch

Turns out to be a local issue, some of my tasks were not being returned in the getItems calls. No idea why this was happening, it works for all tasks newly marked as completed.

To speed up things, I've fixed all the following issues locally, so you don't need to do any further steps. I will check this in soon (probably today).

>+function getFilter(settings) {
>+    let filter = "";
filter should be an integer

>+        if (settings.printCompletedTasks)
>+            filter |= Components.interfaces.calICalendar.ITEM_FILTER_COMPLETED_ALL;
>+        else
>+            filter |= Components.interfaces.calICalendar.ITEM_FILTER_COMPLETED_NO;
Add {}'s here

>+
>+EXPORTED_SYMBOLS = ["utils"];
>+let utils = {
We should rather use the cal. namespace prefix here, as other extensions may also easily use "utils.". I also think we should rename this file to calPrintUtils.jsm, since the function is rather print related.


>+    getTasksWithoutDueDate: function getTasksWithoutDueDate(aItems, date, calGetString) {
Why do you need to pass calGetString here? We can just take this from calUtils. This function also has indentation issues and needs a convert var->let.

>-                <div class='value summary'>{item.title}</div>
>+                <div class='value summary'>{item.isCompleted ? item.title + " (completed)" : item.title}</div>
Since this is user-visible, it needs to be localized.

>+
>+        function isTasksWithStartOrDueDate(startDate, endDate) {
>+            return (startDate != null || endDate != null);
>+        }
>+
>+        if (isTasksWithStartOrDueDate(startDate, endDate)) {
Since this is the only location you use the closure function, you can directly use the condition and add a comment.

>diff -r dee3ed5022c3 calendar/locales/en-US/chrome/calendar/calendar.properties

> #    %6$S will be replaced with the year of the end date
> dayIntervalBetweenYears=%1$S %2$S, %3$S – %4$S %5$S, %6$S

Looks like your editor is not saving in utf8, please fix for the next patches.


r=philipp
Attachment #435607 - Flags: review?(philipp) → review+
Eduardo, I'd appreciate if you could file follow-up bugs for all open issues we have identified here.
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/0c96e2838da9>
-> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b2
Keywords: helpwanted, uiwanted
could not print task when:

1. create task with start and due date
2. from menu select "tasks in view"
 -> new created task is visible in view
3. go to menu file -> print
 -> in preview all events (from the same date as task) are visible but task is not printed
4. try to print out anyway
  -> task is not printed, only events are


tested with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.11pre) Gecko/20100514 Calendar/1.0b2pre
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 73

9 years ago
(In reply to comment #72)

Damian, please remember that the Sunbird builds are still based on the 1.9.1 branch. None of the patches that were checked in after the 1.0 Beta 1 release are contained in this builds.
(Reporter)

Comment 74

9 years ago
Setting back to fixed because steps from Comment 72 work in Lightning 1.0b2pre.
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
(In reply to comment #70)
> Eduardo, I'd appreciate if you could file follow-up bugs for all open issues we
> have identified here.

Have those bugs been filed already?
(In reply to comment #75)
> (In reply to comment #70)
> > Eduardo, I'd appreciate if you could file follow-up bugs for all open issues
> > we have identified here.
> Have those bugs been filed already?

I think not. We still need to file followup bugs.
Here's one, to begin with:

570210 Cannot print completed tasks
Depends on: 570210
Another followup for this bug's comment 65:

Bug 570227 - Printing tasks: Save space by rearranging task properties into a table, add missing properties to printout
Depends on: 570227
Duplicate of this bug: 448590
You need to log in before you can comment on or make changes to this bug.