Closed Bug 251953 Opened 16 years ago Closed 6 years ago

Implement / add "Print preview..." option when composing mail (cannot preview drafts, missing file menu item / command)

Categories

(MailNews Core :: Composition, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 27.0

People

(Reporter: craig, Assigned: sshagarwal)

References

Details

(Whiteboard: [tb31features])

Attachments

(3 files, 8 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040113
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040113

This is very inconvenient because I use mail to reformat html snippets for
printing regularly (e.g. from say multiple rental accomodation searches, copy
and paste into email, edit a little, and print a page with all the addresses to
visit).  This is an excellent way to take parts of multiple webpages and dump
them onto paper.

The work-around I use is to save as a draft, open the draft, print-preview, then
re-edit the draft.

There is a "Print" option when composing mail, it would be very useful to me if
there was also a "print preview" option.

Thanks :)

Reproducible: Always
Steps to Reproduce:
1. Compose a new email
2. Hit "Alt-F", you will see there is no "Print Preview" option.
This also happens in Thunderbird 0.7.2.
Product: MailNews → Core
This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/
This bug has been automatically resolved after a period of inactivity (see above
comment). If anyone thinks this is incorrect, they should feel free to reopen it.
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Resolution: --- → EXPIRED
This bug still exists.

There is no print preview on when composing mail.  I am using Mozilla 1.8b2

Printing from compose would be very useful for printing parts of webpages (copy
and pasted).  I use the same workaround as Bogdan, which is quite inconvenience.
 Basically save as a draft, and the open the draft, and print from there. 
Especially inconvenient if the draft needs re-editing, as you lose the print
preview again when you edit the email.
Status: RESOLVED → UNCONFIRMED
Resolution: EXPIRED → ---
(personally I don't see the point, but) confirming
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
I think this would be a very useful feature.

Mozilla has excellent partial page copying features (i.e. you can grab part of a table or chunks of a webpage).  By pasting these into a "compose" window, you can build up a temporary webpage, and print it.  For example, say you are searching for rental properties, and the search comes back with 50 results, spread over three pages.  You might be interested in, say 10 of these.  You copy and paste them into a compose email, print preview it, and if happy, print it.  Then you have a list of properties to visit.

Or even just for printing a page with heaps of ad's on it, just copy and paste the whole page, then drop out bits you don't want.  Print preview, then print.
I believe this is still the case in version 1.5.0.7 (20060909).

Whilst it may not be a high priority, it is little things like this that can make the difference between general users accepting a product or rejecting it.  I personally didn't notice it; I was first alerted to this issue when my father asked if it was possible.  I had to get him to do the [[Save -> Open "Drafts" -> Select message -> File or Right-click -> Print Preview]] work-around.
sorry for the spam.  making bugzilla reflect reality as I'm not working on these bugs.  filter on FOOBARCHEESE to remove these in bulk.
Assignee: sspitzer → nobody
Duplicate of this bug: 393827
Duplicate of this bug: 374794
QA Contact: composition
Product: Core → MailNews Core
I would like to add my voice for a request for print preview while composing. It is inconvenient to have to save to draft before being able to see what the document is going to look like.  Thunderbird is an excellent program and I will keep using it but this bug should be fixed. Thanks.
omg, why oh why... (6 years later, still)

Does this require new code to render an unsaved draft into a print preview or is it only a matter of adding the menu item to the file menu?
Depends on: 374794
Summary: No print preview option when composing mail → Implement / add "Print preview..." option when composing mail (cannot preview drafts, missing file menu item / command)
Suyash, I suspect this could be simple - might just work by adding a menuitem with command="cmd_printPreview" to composition's file menu, and perhaps a few lines to enable/execute that command, as seen in 
http://mxr.mozilla.org/comm-central/source/mail/base/content/mail3PaneWindowCommands.js#776 (search for cmd_printPreview in that file to see the rest):

776       case "cmd_printpreview":
777         PrintEnginePrintPreview();
778         return;
Whiteboard: [good first bug]
(In reply to Thomas D. from comment #13)
> Suyash, I suspect this could be simple - might just work by adding a
> menuitem with command="cmd_printPreview" to composition's file menu, and
> perhaps a few lines to enable/execute that command, as seen in 
> http://mxr.mozilla.org/comm-central/source/mail/base/content/
> mail3PaneWindowCommands.js#776 (search for cmd_printPreview in that file to
> see the rest):
> 
> 776       case "cmd_printpreview":
> 777         PrintEnginePrintPreview();
> 778         return;
Attached patch Patch (obsolete) — Splinter Review
This doesn't work as expected. I get this error:
Timestamp: 07/09/2013 06:18:29 AM
Error: ReferenceError: SidebarGetState is not defined
Source File: chrome://editor/content/editor.js
Line: 505
SidebarGetState() isn't defined in mail/

Moreover, I didn't find any call to such function here:
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/printing/content/printUtils.js#60

So, can someone please help me out in fixing this Sidebar error?
needinfo vv

(In reply to Suyash Agarwal (:sshagarwal) from comment #15)
> Created attachment 772236 [details] [diff] [review]
> Patch
> 
> This doesn't work as expected. I get this error:
> Timestamp: 07/09/2013 06:18:29 AM
> Error: ReferenceError: SidebarGetState is not defined
> Source File: chrome://editor/content/editor.js
> Line: 505
> SidebarGetState() isn't defined in mail/
> 
> Moreover, I didn't find any call to such function here:
> http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/
> printing/content/printUtils.js#60
> 
> So, can someone please help me out in fixing this Sidebar error?
Flags: needinfo?
And please ignore my TimeStamp, it isn't correct. :)
No longer depends on: 374794
Duplicate of this bug: 374794
See Also: → 318955
I see the exception after applying the patch.
The function is defined at http://mxr.mozilla.org/comm-central/source/suite/common/sidebar/sidebarOverlay.js#1189 . 

You can try copying some necessary pieces to TB. The sidebar-box element seems to exist in TB:
http://mxr.mozilla.org/comm-central/source/mail/components/compose/content/messengercompose.xul#784
Flags: needinfo?
aceman sir, thanks for your help, I have fixed the sidebar issues.

Still few other issues keep getting created. This is the same command that is used by all the print preview commands everywhere, even setting designmode off doesn't seem to work. So, either I can internally save this as a draft and then show the print preview (a seriously boring way of doing it) or please suggest me how to proceed.
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #772236 - Attachment is obsolete: true
Can you please suggest me how to proceed?
Flags: needinfo?(neil)
OK, so you got as far as adding a cmd_printpreview to your default controller, which is good. The design mode is a red herring, forget that. Your mistake was to try to use editor's PrintPreviewListener rather than creating a new one for the the compose window. You can copy and past most of the editor's code, but you will need to tweak it in places: getPrintPreviewBrowser will need tweaking to insert the browser element in the correct place in the DOM, and getNavToolbox will need to return the compose window's toolbox. You will also need to copy and tweak the toggleAffectedChrome function, since the Thunderbird compose sidebar works differently to the SeaMonkey sidebar, and again there are references to elements that need to be updated to work with the compose window.

To avoid your copied code conflicting with the shared editor code, you will need to move the editor's PrintPreviewListener and toggleAffectedChrome from editor.js into editingOverlay.js so that the compose window doesn't load it.
Flags: needinfo?(neil)
Attached patch Patch v3 (obsolete) — Splinter Review
Okay so the print preview works now!
Thanks to Neil :)
Assignee: nobody → syshagarwal
Attachment #782240 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #813483 - Flags: feedback?(neil)
Attachment #813483 - Flags: feedback?(acelists)
Screenshot showing print preview in action.
Attachment #813484 - Attachment description: printtest.png → Screenshot showing print preview of Compose.
Comment on attachment 813483 [details] [diff] [review]
Patch v3

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

So this seems to work nicely. Tested on TB on linux. Print preview works and it is possible to print directly from it (tested on the "Save to file printer").
There is only one glitch I noticed. It does not show the message header (recipients). Is it a problem?
Otherwise very nice, thanks!

Do you need to add both of those PrintPreviewListeners? Is one for TB and one for SM?

::: editor/ui/composer/content/editor.js
@@ -1509,5 @@
>  function SetEditMode(mode)
>  {
>    if (!IsHTMLEditor())
>      return;
> -

Why remove this?

::: mail/components/compose/content/MsgComposeCommands.js
@@ +301,5 @@
> +    return "collapsed";
> +  return "visible";
> +}
> +
> +function toggleAffectedChrome(aHide)

Is all this added code actually used? Can you add some comment what it is for?

::: mail/components/compose/content/messengercompose.xul
@@ +115,5 @@
>    <command id="cmd_sendNow" oncommand="goDoCommand('cmd_sendNow')"/>
>    <command id="cmd_sendWithCheck" oncommand="goDoCommand('cmd_sendWithCheck')"/>
>    <command id="cmd_sendLater" oncommand="goDoCommand('cmd_sendLater')"/>
>    <command id="cmd_printSetup" oncommand="goDoCommand('cmd_printSetup')"/>
> +  <command id="cmd_printpreview" oncommand="goDoCommand('cmd_printpreview')"/>

cmd_printPreview seems to be the style here.

@@ +191,5 @@
>    <key id="key_close" key="&closeCmd.key;" command="cmd_close"  modifiers="accel"/>
>    <key id="key_save" key="&saveCmd.key;" command="cmd_saveDefault" modifiers="accel"/>
>    <key id="key_send" keycode="&sendCmd.keycode;" observes="cmd_sendWithCheck" modifiers="accel"/>
>    <key id="key_sendLater" keycode="&sendLaterCmd.keycode;" observes="cmd_sendLater" modifiers="accel, shift"/>
> +  <key id="key_printPreview" keycode="&printPreviewCmd.key;" command="cmd_printPreview" modifiers="accel"/>

cmd_printPreview

@@ +473,5 @@
>            <menuitem label="&sendNowCmd.label;" accesskey="&sendNowCmd.accesskey;" key="key_send" command="cmd_sendNow" id="menu-item-send-now"/>
>            <menuitem label="&sendLaterCmd.label;" accesskey="&sendLaterCmd.accesskey;" key="key_sendLater" command="cmd_sendLater"/>
>            <menuseparator/>
>            <menuitem id="printSetupMenuItem" label="&printSetupCmd.label;" accesskey="&printSetupCmd.accesskey;" command="cmd_printSetup"/>
> +          <menuitem id="printPreviewMenuItem" label="&printPreviewCmd.label;" accesskey="&printPreviewCmd.accesskey;" key="key_printPreview" command="cmd_printpreview"/>

cmd_printPreview
Attachment #813483 - Flags: feedback?(acelists) → feedback+
This is the view when headers are included. I thought they are not needed in the print preview part and also the preview document is included in the body, so I removed it. If you would like it back please let me know.

Thanks.
Attachment #813696 - Flags: feedback?(acelists)
No, that is not it. The header WIDGETS should be hidden. But the values of the recipients should be on the page in the preview and be printed. See how a received message is printed.
(In reply to :aceman from comment #28)
> No, that is not it. The header WIDGETS should be hidden. But the values of
> the recipients should be on the page in the preview and be printed. See how
> a received message is printed.

But when I am printing the compose message, then too these headers aren't being included in the printed pdf file.

So, if this is expected, I think we will need to make the change in the Print function.
Oh, I see now. The existing Print function also prints only the body of the message. So your Print preview shows the same output. Then it is all fine. Thanks.
If we wanted to print also the recipients, it would be for another bug.
Comment on attachment 813696 [details]
Screen shot of print preview of compose with headers

While this UI seems interesting, we probably don't want it right now.
Attachment #813696 - Flags: feedback?(acelists) → feedback-
Attachment #813696 - Attachment is obsolete: true
(In reply to :aceman from comment #30)
> Oh, I see now. The existing Print function also prints only the body of the
> message. So your Print preview shows the same output. Then it is all fine.
> Thanks.
> If we wanted to print also the recipients, it would be for another bug.

+1. It's Bug 297702, and we need to fix that separately which involves changing the hidden DOM doc created for the actual print/preview. That's certainly a lot trickier than this bug. Good news is I met Steffen Wilberg on the Summit who might be able to assist/advice us with that as he's knowledgeable in Toolkit print stuff which underlies this.
Comment on attachment 813483 [details] [diff] [review]
Patch v3

>diff --git a/editor/ui/composer/content/editingOverlay.js b/editor/ui/composer/content/editingOverlay.js
This should of course be simply a cut and paste of the code from editor.js and not what you've actually pasted.

>-
And there shouldn't be any stray changes in editor.js either.

>+var PrintPreviewListeners = {
Nit: might want to stick to PrintPreviewListener for consistency with viewSource.js and everywhere else.

>+function sidebar_is_collapsed() {
I don't know how Thunderbird's addressing sidebar works but there might already be functions for it.

>+function PrintEnginePrintPreview()
Do you need this?
Attachment #813483 - Flags: feedback?(neil)
Attached patch Patch v3.1 (obsolete) — Splinter Review
Fixed the nits.
Attachment #813483 - Attachment is obsolete: true
Attachment #815979 - Flags: review?(mkmelin+mozilla)
Comment on attachment 815979 [details] [diff] [review]
Patch v3.1

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

Looks good to me, with a few small changes r=mkmelin
(You still need an /editor review too)

::: mail/components/compose/content/MsgComposeCommands.js
@@ +278,5 @@
> +function sidebar_is_hidden() {
> +  var sidebar_title = document.getElementById('sidebar-title-box');
> +  var sidebar_box = document.getElementById('sidebar-box');
> +  return sidebar_box.getAttribute('hidden') == 'true'
> +         || sidebar_title.getAttribute('hidden') == 'true';

copy-pasted, but we should put the or operator on the previous row

@@ +1646,5 @@
> +function DoCommandPrintPreview()
> +{
> +  try {
> +    PrintUtils.printPreview(PrintPreviewListener);
> +    } catch(ex) {dump("#PRINT PREVIEW ERROR: " + ex + "\n");}

Components.utils.reportError(ex); instead

But on second thought, no need to try/catch it at all. Then the error will still be in the console, and with proper line numbers

::: mail/locales/en-US/chrome/messenger/messengercompose/messengercompose.dtd
@@ +50,5 @@
>  <!ENTITY sendLaterCmd.accesskey "L">
>  <!ENTITY printSetupCmd.label "Page Setup…">
>  <!ENTITY printSetupCmd.accesskey "u">
> +<!ENTITY printPreviewCmd.label "Print Preview">
> +<!ENTITY printPreviewCmd.key "T">

I don't think we need to have a hotkey for print preview. Let's save it for something more useful. We don't have a hotkey for it in the main window either. 

And is it really free? Didn't see a usage from a quick glance, but the compose window has a lot of quite hidden commands.
Attachment #815979 - Flags: review?(mkmelin+mozilla) → review+
Whiteboard: [good first bug]
Attachment #815979 - Flags: review?(ehsan)
Attachment #815979 - Flags: review?(neil)
Comment on attachment 815979 [details] [diff] [review]
Patch v3.1

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

I'm not the right reviewer for this code.
Attachment #815979 - Flags: review?(ehsan)
(In reply to comment #33)
>(From update of attachment 813483 [details] [diff] [review])
>>diff --git a/editor/ui/composer/content/editingOverlay.js b/editor/ui/composer/content/editingOverlay.js
>This should of course be simply a cut and paste of the code from editor.js
>and not what you've actually pasted.
I don't see any significant change to editingOverlay.js in your latest patch.
Attached patch Patch v3.5 (obsolete) — Splinter Review
Made the suggested nits.
Attachment #815979 - Attachment is obsolete: true
Attachment #815979 - Flags: review?(neil)
Attachment #819441 - Flags: review?(neil)
Comment on attachment 819441 [details] [diff] [review]
Patch v3.5

>+var PrintPreviewListener = {
Nit: editor.js had toggleAffectedChrome before var PrintPreviewListener

>+  //   (*) statusbar
>+  if (!gChromeState)
Nit: editor.js had a blank line between these two lines

r=me for the editor changes with those fixed.

>+  <command id="cmd_printPreview" oncommand="goDoCommand('cmd_printPreview')"/>
I didn't look at the mail/ part of the patch very carefully but I did happen to notice that for whatever reason everywhere else the print preview command is cmd_printpreview all in lower case. I don't know why, probably historical.
Attachment #819441 - Flags: review?(neil) → review+
Attached patch Suite portSplinter Review
Attaching this here because I haven't filed a bug get.
*yet
Attached patch Final Patch (obsolete) — Splinter Review
Made the suggested changes.
Carrying over review from mkmelin and Neil.
Attachment #819441 - Attachment is obsolete: true
Attachment #819872 - Flags: review+
Comment on attachment 813484 [details]
Screenshot showing print preview of Compose.

Screenshot showing the print preview of compose window.
Attachment #813484 - Flags: ui-review?(bwinton)
Comment on attachment 813484 [details]
Screenshot showing print preview of Compose.

On Mac, we get this from the Print dialog, and so don't want it.
On Windows, I _think_ we get this from the Print dialog, and don't want it, but someone needs to check.
On Linux, I think we _should_ get this from the Print dialog, but I would believe that we don't, so someone needs to check that, too.
Attachment #813484 - Flags: ui-review?(bwinton) → ui-review-
Okay, here I remove the menu option for mac os.
Attachment #819872 - Attachment is obsolete: true
Attachment #822469 - Flags: ui-review?(richard.marti)
Indeed it's ifdef macosx in the main menu - http://mxr.mozilla.org/comm-central/source/mail/base/content/mailWindowOverlay.xul#2317

On linux we don't get print preview from the print dialog. Haven't checked windows, but at least a print preview menu is common there. (Availability on windows could also depend on the printer, I suspect.)
Comment on attachment 822469 [details] [diff] [review]
Final Patch removing the preview option for OS X

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

ui-r=me only when you add in messengercompose.dtd a <!ENTITY printPreviewCmd.label "Print Preview">. Without this, opening the compose window fails with a XUL error.

I know this isn't the focus of this bug and should be investigated in a new bug: The preview (and also the printout) shows only the content. Shouldn't it also show the header with from, to and subject like the preview/printout of received messages is doing?
Attachment #822469 - Flags: ui-review?(richard.marti) → ui-review+
(In reply to Richard Marti [:Paenglab] from comment #47)
> Comment on attachment 822469 [details] [diff] [review]
> Final Patch removing the preview option for OS X
> 
> Review of attachment 822469 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ui-r=me only when you add in messengercompose.dtd a <!ENTITY
> printPreviewCmd.label "Print Preview">. Without this, opening the compose
> window fails with a XUL error.
Thanks.
 
> I know this isn't the focus of this bug and should be investigated in a new
> bug: The preview (and also the printout) shows only the content. Shouldn't
> it also show the header with from, to and subject like the preview/printout
> of received messages is doing?

Ya, that is bug 297702 stated in comment 32.
Fixed the suggested nits.
Carrying over review from Neil and mkmelin and ui-review from paenglab.
Attachment #822469 - Attachment is obsolete: true
Attachment #822863 - Flags: ui-review+
Attachment #822863 - Flags: review+
Neil, can you please add headers in your patch for Suite Port? or will we be landing it in another bug?
I can add other headers, I just don't know the value of the Parent field for the patch.
Flags: needinfo?(neil)
No, I need to file a new bug and get review first, I just attached it here because it helped me test your patch.
Flags: needinfo?(neil)
Okay thanks :)
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/48852b6ad434
Status: ASSIGNED → RESOLVED
Closed: 14 years ago6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 27.0
Blocks: 953407
Depends on: 953403
Whiteboard: [tb31features]
You need to log in before you can comment on or make changes to this bug.