118880 - What menu item to choose when printing a mailing list?

Status: NEW

(MailNews Core :: Address Book, defect)

Description

[Ninoschka Baca]

Overview: What menu item to choose when printing a list? I looked for this 
informaiton in the spec and it's not clear since I only see "Print Card" and 
Print Address Book" options.

Steps to reproduce:
1. With an Address Book selected in the directory pane, select the list from the 
right pane.
Actual Results: File|Print Card doesn't seem right yet it does print the list.

2. With a list selected in the directory pane.
Actual Results: File|Print Address Book prints the list but this doesn't seem 
right since it's a list.

Expected Results: 
Idea 1: Should there be a "Print List" option? 
Idea 2. Another idea is to only have a "Print" option. Then depending on what is 
selected, it will do the right thing. For instance:
- highlight the address book from the directory pane, File|Print prints the 
entire address book.
- highlight a card in the right pane, File|Print just prints the card
- highlight a list in the right pane, File|Print prints the list
- highlight a list in the directory pane, File|Print prints the list

Comment 1

[Ninoschka Baca]

Marking nsbeta1 since it's confusing when printing a list.

Comment 2

[jglick]

I like the last suggestion, with one change. Menu item should be context 
sensitive if possible.

"Print <Card/Mailing List/Address Book>" depending on what is selected.

My only concern, the accelerator "Ctrl+P" can be used as well. So accelerator 
would also be context sensitive. If user does Menu: File: Print <Card/Mailing 
List/Address Book> they know exactly what they will be printing. If they do 
"Ctrl+P" they might have though they had a card selected but the AB really had 
focus (so they get the whole AB printing). Is that a concern at all?

Comment 3

[scottputterman]

I guess I'd argue that Print Address Book should be available all of the time. 
List and Card should be context sensitive.

Comment 4

[jglick]

That sounds good to me. Two menu items:

Print <Card/Mailing List> --- Ctrl+P  (Context Sensitive)
Print Address Book

Comment 5

[Ninoschka Baca]

Marking nsbeta1. It's currently confusing to determine how to print a mailing list.

Comment 6

[Samir Gehani]

Mail triage team: nsbeta1-

Comment 7

[(not reading, please use [e-mail] instead)]

mass re-assign.

Comment 9

[Mark Banner (:standard8)]

Attachment: SM fix v1

This patch clarifies the situation by

a) Turning two print preview and two print options in the address book file menu into one of each.
b) The one remaining print and print preview options now are context sensitive in that they will display appropriate text for what is selected.

Note that print previewing multiple selected cards only displays the first, this  is how it currently works.

Scott, I know this is for SM only at the moment, but could you take a look and let us know what you think as I plan to do a patch for TB which incorporates this and a port of bug 112959.

Comment 10

[Karsten Düsterloh]

Comment on attachment 260873 [details] [diff] [review]
SM fix v1

>Index: mailnews/addrbook/resources/content/abCommon.js
>===================================================================
>-        return (enabled && (numSelected > 0));
>+        return enabled && (numSelected > 0);

This is inconsequent. Either drop all braces - () has precedence over && - or just leave it untouched.

>+          goSetMenuValue(command,
>+                         gAbView.selection.count ? "valueItems" : "valueItem");

What's the point of showing a different menuitem with a misleading title if the count is zero and nothing will be printed anyway? I think you want to differ between 'one'/'many' and disable the menuitem for 'none'?

> function SelectFirstCard()
> {
>-  if (gAbView && gAbView.selection) {
>+  var abViewSelect = gAbView.selection;
>+  
>+  if (gAbView && gAbView.selection && gAbView.rowCount) {
>     gAbView.selection.select(0);

I think the var line is just old cruft?

>Index: mailnews/addrbook/resources/content/addressbook.js
>===================================================================
>+const kAddrBookPrintPreview = Components.interfaces.nsIMsgPrintEngine.
>+                                MNAB_PRINTPREVIEW_ADDRBOOK;
>+const kAddrBookPrint = Components.interfaces.nsIMsgPrintEngine.
>+                         MNAB_PRINT_ADDRBOOK;
>+const kAddrBookCardPrintPreview = Components.interfaces.nsIMsgPrintEngine.
>+                                    MNAB_PRINTPREVIEW_AB_CARD;
>+const kAddrBookCardPrint = Components.interfaces.nsIMsgPrintEngine.
>+                             MNAB_PRINT_AB_CARD;

I don't see the point in polluting the global namespace with consts which are all used only once. Providing a const for Components.interfaces.nsIMsgPrintEngine could be useful, though.

>+function AbPrintDirPaneItems(doPrintPreview)
>+{
>+  AbPrintAddressBookInternal(doPrintPreview,
>+                             doPrintPreview ? kAddrBookPrintPreview :
>+                               kAddrBookPrint);
>+}
>+
>+function AbPrintSelectedResultPaneItems(doPrintPreview)
>+{
>+  AbPrintCardInternal(doPrintPreview,
>+                      doPrintPreview ? kAddrBookCardPrintPreview :
>+                        kAddrBookCardPrint);
>+}

See above. If you're worried about line length/formatting, just use a local variable for the second parametre.

>Index: mailnews/addrbook/resources/content/addressbook.xul
>===================================================================
>Index: suite/locales/en-US/chrome/mailnews/addressbook/abMainWindow.dtd
>===================================================================
>+<!ENTITY printItem.accesskey                            "P">
>+<!ENTITY printPreviewItem.accesskey                     "V">

You should keep items group by their prefix, so you can see at once what the values for .label, .accesskey etc. are without having to search the whole file.


Furthermore, the preview commands doesn't work for me at all on Mac OS X 10.4.9, all I get is "An unknown error occured while printing."

Comment 11

[Mark Banner (:standard8)]

(In reply to comment #10)
> >+          goSetMenuValue(command,
> >+                         gAbView.selection.count ? "valueItems" : "valueItem");
> What's the point of showing a different menuitem with a misleading title if the
> count is zero and nothing will be printed anyway? I think you want to differ
> between 'one'/'many' and disable the menuitem for 'none'?

It shouldn't be going into that bit - the if statement on the line before it:

+        if (gAbView && gAbView.selection) {

should stop it, and then it'll the function will return false which will disable the menuitem.

> Furthermore, the preview commands doesn't work for me at all on Mac OS X
> 10.4.9, all I get is "An unknown error occured while printing."

Ah, Thunderbird disables this:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/mail/components/addrbook/content/addressbook.xul&rev=1.68&mark=259-262#259

Also ref bug 129398 and bug 347417. Guess we should disable it for mac for now.

Comment 12

[Karsten Düsterloh]

> It shouldn't be going into that bit - the if statement on the line before it:
> 
> +        if (gAbView && gAbView.selection) {
> 
> should stop it, and then it'll the function will return false which will
> disable the menuitem.

I didn't debug what happened (yet), but it doesn't get disabled on Mac. Could be a Mac specific problem again, though. (I had one selected item, hence the 'item' text, deselected that by Cmd-Space and looked into the main menu.)

> Also ref bug 129398 and bug 347417. Guess we should disable it for mac for
> now.

Yes, I think so.

Comment 13

[Stefan [:stefanh]]

On mac, we use the native print dialog (there's a preview option in the dialog) and hide the preview menuitems - see http://lxr.mozilla.org/seamonkey/source/suite/common/mac/platformCommunicatorOverlay.xul#73

Comment 14

[Wayne Mery (:wsmwk)]

make sense to pick this up in the AB refactoring? 
also, card preview and print not working on current trunk.
AB print preview yields Error: 
Source File: addbook://moz-abmdbdirectory/abook.mab?action=print
Line: 2, Column: 1
Source Code:
<?xml-stylesheet type="text/css" href="chrome://messenger/content/addressbook/print.css"?>

Comment 15

[Mark Banner (:standard8)]

(In reply to comment #14)
> also, card preview and print not working on current trunk.
> AB print preview yields Error: 
> Source File: addbook://moz-abmdbdirectory/abook.mab?action=print
> Line: 2, Column: 1
> Source Code:
> <?xml-stylesheet type="text/css"
> href="chrome://messenger/content/addressbook/print.css"?>

Wayne, these are being covered in bug 451143.

Comment 16

[Wayne Mery (:wsmwk)]

In the case of the current beta, the Print menu item is disabled in all cases afaict. Only the context menu print is available.