263312 - Print dialog should use file checkbox instead of radio buttons

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[Peter Weilbacher]

When I select "Print to: File" in print dialog this does not disable any of the
printer related widgets in this dialog (the other way around it does). It should
at least disable display of the printer selection list dropdown, the properties
button and the printer description text.
Strange that I didn't find a bug about this, as far as I remember it has been
like this for ages...

Comment 1

[Peter Weilbacher]

Attachment: A try at a fix

As far as I can see this simple fix does exactly what I want.

Comment 2

[Peter Weilbacher]

Attachment: A better fix

Hmm, on second thoughts, it is probably a better solution to disable the
corresponding Labels, too, as it is already done for the fileLabel

Comment 3

[Kenneth Herron]

I disagree with the original comment. As a practical matter, the properties
button triggers a dialog which lets the user set the paper size,
color/grayscale, and printable region margins. These are all important to a
print-to-file operation.

Beyond that, print-to-file means to print as if to the selected printer, but
save the result in a file. So if the print driver knows enough about the printer
to affect the print job (avoiding postscript level 2 features when printing to a
level 1 printer, for example), then the selected printer is still relevant.

Comment 4

[Peter Weilbacher]

OK, at least that gives me an idea why I didn't find any bugs about this strange
behaviour.
But how am I supposed to know that the printer settings (the "Properties..."
button) also apply to the printer? If they do the button should be called "File
and Print settings" or something like that and be positioned somewhere else,
probably at the bottom of the "Printer" box.

And at least the print command of the Postscript/default printer does not apply,
I cannot use psnup -2 to print to file as I can use it to print to the printer.
So maybe that field should be disabled if print to file is selected.

Comment 5

[Peter Weilbacher]

Another idea: how about making the print to file one option in the list of
printers and if it is selected replace the print command or plex mode field with
a filename field? This is the way gimp-print seems to handle it and I find it
quite intuitive.
Hmm, then it would be a bit awkward to need to go into the sub-dialog to be able
to select the filename. Is the print dialog already too large to accomodate it
there?

Comment 6

[Roland Mainz]

Peter Weilbacher wrote:
> When I select "Print to: File" in print dialog this does not disable any of 
> the printer related widgets in this dialog (the other way around it does). It 
> should at least disable display of the printer selection list dropdown, the 
> properties button and the printer description text.
> Strange that I didn't find a bug about this, as far as I remember it has been
> like this for ages...

This is very BAD idea
 "Print to file" just means that the output of the driver (Postscript module or
Xprint server) is redirected to a file instead of being send to the printer.
For example when yous elect the builtin Postscript/default driver you get
Postscript output, but when you select a Xprint server which is corrected to a
PCL printer then you get PCL data in the file. Or you select a queue which sends
SVGprint data then you get SVG data in the file etc.

Comment 7

[Roland Mainz]

Peter Weilbacher wrote:
> OK, at least that gives me an idea why I didn't find any bugs about this 
> strange behaviour.
> but how am I supposed to know that the printer settings (the "Properties..."
> button) also apply to the printer?

> If they do the button should be called 
> "File and Print settings" or something like that and be positioned somewhere 
> else, probably at the bottom of the "Printer" box.

Then the people are likely confused. The option to redirect the job data to a
file is just an small feature, the _primary_ task of the print and
printjoboptions dialogs is to select the printer and the printer+job attributes.

> And at least the print command of the Postscript/default printer does not 
> apply,
> I cannot use psnup -2 to print to file as I can use it to print to the 
> printer. So maybe that field should be disabled if print to file is selected.

Mhhh... intersting idea. It's slightly tricky to implement it right because
Xprint (at least the spec, Mozilla's Xprint support doesn't support it right
now) supports "filters" which can be applied to a print job.

Comment 8

[Roland Mainz]

s/Xprint server which is corrected to a/Xprint queue which is connected to a/

Comment 9

[Peter Weilbacher]

OK, now I think I have gotten close to understanding what this dialog really does...
I guess what is needed to visually represent what is happening is a checkbox in
place of the radiobuttons. That would convey the message "use the above settings
to print to file" which the radiobuttons do not.
Additionally, depending on the format that the printer driver writes, the
default filename should change. Writing SVG data (if Xprint is set up to do
that) and suggesting a filename mozilla.ps is really stupid. But that is bug 112779.

Comment 10

[jag (Peter Annema)]

On Windows, there is such a checkbox. When you check it and hit print, it asks
for the filename (through some simple dialog, instead of the Windows file picker).

Not saying we must replicate that exact UI, but there is a precedent, and the
checkbox did convey to me "Print to file (the bits you would've sent to the
printer)".

Perhaps on Linux our UI could be something like:

+- Printer -------------------------------------------------------+
|                                                                 |
| Name: [xp_ps_spooldir_HOME_Xprintjobs@:64 \/] ( Properties... ) |
|                                               [ ] Print to file |
|                                                                 |
+-----------------------------------------------------------------+
+- Print Range ------------------------++- Copies ----------------+
|                                      ||                         |

  etc.

Hmm, then we're back to looking almost like the (native) Windows print dialog,
except for the printer information.

When the user hits Print, if the checkbox is checked, we can pop up the linux
file picker in save mode.

It'd be nice if hitting cancel in the file picker just brings you back to the
print dialog, hitting Save will start printing and dismiss the print dialog.
On Windows right now the print dialog gets closed, we start printing, and then
(I guess) the print driver brings up the dialog asking which file to save it to.
If you cancel that you get some error message saying "Printing failed when
starting the document." But I digress.

Comment 11

[jag (Peter Annema)]

Oops, missed the "Printer Description" line. Imagine I did put it there ;-)

Btw, on Mac things work slightly differently. Everything goes to and from PDF,
so their "print to file" is simply "Save As PDF..."

Hmmm, and here's how Gnome does things:

http://www.gnome.org/projects/gnumeric/doc/figures/print-worksheet.png

Which is closer to what we have right now, but with the radio buttons in front
of the two lines instead of above.

KDE does things slightly differently:

http://printing.kde.org/screenshots/pics/printdialog.png
http://docs.kde.org/en/HEAD/koffice/kformula/green2.png

"Print To File (PostScript)" suggests in the context of the print dialog, files
are just considered another kind of printer (one that "prints" bits to disk
instead of to paper), where the meaning given to the bits is specified in the
parentheses. I'm guessing/hoping there's also a "Print to File (PDF)". For them
the file isn't an intercept and redirect target, it's just another printer, so
for them it makes sense to integrate it the way they did.

Anyway, the reason I think we should remove the file name from the main print
dialog is that it adds unnecessary confusion for a feature that isn't used all
that often. Bringing up the file picker when you need it is a lot less intrusive.

Comment 12

[Peter Weilbacher]

I looked at the screenshots from other systems but to me it seems that none of
those really apply in the Mozilla case. The GNOME way is close but somehow the
Properties button in the Mozilla dialog does not fit into that.

I mostly agree with jag's design suggestion from comment 10 and had a first go
at it, I will attach the patch shortly.

On Linux it works fine, at least with a GTK+ 2.x nightly. On OS/2 there is a
strange effect that after I selected the file to print to and the print dialog
closes I get a window titled "Print to a File" prompting me to enter a file name
with "Enter file name of output print data". That does not seem to come from
Mozilla but from the OS/2 printer driver. I don't understand how this happens or
how this was bypassed before...

Additionally, my solution "fixes" bug 112779 because the file picker is not
prefilled.

Comment 13

[Peter Weilbacher]

Attachment: dialog with "Print to File" checkbox

It seems to me that the part with the sfile.exists() check (now in function
chooseFile(), previously in function onAccept()) is not needed any more. The
file pickers I tried (OS/2 and GTK+ 2.x) seem to do the check already.

Comment 14

[Peter Weilbacher]

Attachment: 2nd try, this one really works

OK, in the old patch I forgot to actually set gPrintSettings.toFileName so this
info never reached the appropriate function. Now it works on OS/2, and GTK+ 1.x
and 2.x. I also removed the extra sfile.exists() check that is already handled
by the file picker on all platforms.

Comment 15

[Peter Weilbacher]

Comment on attachment 164888 [details] [diff] [review]
2nd try, this one really works

jag, this patch seems to do what you had in mind...

Comment 16

[Roland Mainz]

Comment on attachment 164888 [details] [diff] [review]
2nd try, this one really works

Erm, does this patch mean that the file picker dialog now comes everytimes,
regardless whether the user has set a custom pref value as his default location
or not ?

Comment 17

[Peter Weilbacher]

You mean the value that before was filled into the entry field in the dialog?
Hmm, which pref was that again?
As there is no way to check or change that location before pressing the Print
button it seemed to make sense to present the file picker every time. That is a
drawback, I agree. Any suggestion how to represent it in the new scheme?

Comment 18

[jag (Peter Annema)]

If the user has set a pref saying "by default, save print output to this file",
we could just skip the dialog. This won't let you ever change the location
though. I see now how the current UI easily supports both having a default
location (visible in the textbox) and the option to override the default.

As pointed out, the radio buttons (specifically, their placement) make it look
like you're choosing between either a printer to pick or a file to save to,
while really you're not. You're choosing between sending the data, formatted as
specified by the selected printer, to said printer, or to a file. Keeping the
current behaviour of disabling/enabling the file selector, I think the simplest
fix would be to just replace the two radio buttons with one checkbox.

Roland, do you know why someone would set a default print-to-file location?

Comment 19

[[e-mail]]

Comment on attachment 164888 [details] [diff] [review]
2nd try, this one really works

While you argue over the pref, here are some style nits:

>+      if (chooseFile()) {
>+         gPrintSettings.toFileName = gFileFromPicker;
>+      } else {
>         return false;
>       }
Don't use an else clause just for a return or break.
if (c)
  x();
else
  return;
y();
is better written as
if (!c)
  return;
x();
y();

>+    if (fp.show() == Components.interfaces.nsIFilePicker.returnCancel || !fp.file)
>+      return false;
>+    if (fp.file && fp.file.path.length > 0)
>+      gFileFromPicker = fp.file.path;
>+    else
>+      return false;
>   } catch(ex) {
>     dump(ex);
>   }
>+
>+  return true;
> }
I don't think you want to return true if an exception occurred. I think you
should return false; at the end, and only return true if you succesfully
retrieved a path.
if (fp.show() != returnCancel && fp.file && fp.file.path) {
  gPrintSettings.toFileName = fp.file.path;
  return true;
}

Comment 20

[Julien Lafon (Hitachi)]

(In reply to comment #17)
> As there is no way to check or change that location before pressing the Print
> button it seemed to make sense to present the file picker every time. That is a
> drawback, I agree. Any suggestion how to represent it in the new scheme?

Replacing te radio buttons with a checkbox is fine for me. But I am against the
proposal to bring up the file picker dialog each time - even if the user has set
up a location for his batch work. Its annoying and treats users like small
children (rumor says that Longhorn made it optional).
Just my $0.05.

Comment 21

[Peter Weilbacher]

I like these "children" arguments, too. :-) But can you give a scenario for this
batch work you are talking about? There are certainly better apps to do that
than Mozilla, like htmldoc <http://www.htmldoc.org/> or various commercial stuff
for Windows. For Mozilla that would also mean that you overwrite the file every
time you print or you need to change it every time, right?

As an alternative, would it be good enough to honor the setting (which prefs are
we talking about here?) and let experienced users who need it change them
through about:config? Oh, gPrintSettings.toFileName does not even make it to the
prefs, right? Hmm...

Comment 22

[Peter Weilbacher]

Attachment: patch v3, following comments

OK, I slightly changed the patch per neil's comments. This now operates
directly on gPrintSettings.toFileName instead of using the extra
gFileFromPicker variable (just like the comment seemed to imply).

There were no further comments/scenarios from the
anti-select-filename-every-time fraction although I would like to improve the
patch so that everybody is finally happy with the print dialog. If there are no
further ideas and the reviewers are happy, could this not just be checked in as
a test? And if after the next nightly (or the 1.8a5 release) too many people
complain we back it out and just change the radiobuttons to a checkbox? Hmm,
no, I don't think that's how the Mozilla UI development process works, is it?

Alternatively, I could put together Linux and OS/2 builds that include the new
printer dialog and announce them somewhere and see what comments I get. (Could
have done that two weeks ago...)

Comment 23

[[e-mail]]

Comment on attachment 166251 [details] [diff] [review]
patch v3, following comments

>         <row align="center">
>           <hbox align="center" pack="end">
>+            <label id="descTextLabel" control="descText" value="&descText.label;"/>
>           </hbox>
>+          <label id="descText" flex="1" value="-" />
>         </row>
>       </rows>
>     </grid>
>+    <hbox pack="end">
>+      <checkbox id="fileCheck" checked="false" label="&fileCheck.label;" pack="end"/>
>+    </hbox>
I just looked at the visual effect and I think that this checkbox looks out of
place here, it would look better in the bottom right corner of the grid.

Comment 24

[[e-mail]]

Comment on attachment 166251 [details] [diff] [review]
patch v3, following comments

>+          <label id="descText" flex="1" value="-" />
... also I know you only moved this but as you'll get blamed for it, the flex
and value are probably unnecessary, so if you do make a new patch please try
removing them. Everything else looks OK though.

Comment 25

[Peter Weilbacher]

(In reply to comment #23)
> I think that this checkbox looks out of
> place here, it would look better in the bottom right corner of the grid.

You mean still on the same line with the printer description, like this:
        <row align="center">
          <hbox align="center" pack="end">
            <label id="descTextLabel" control="descText" value="&descText.label;"/>
          </hbox>
          <label id="descText"/>
          <checkbox id="fileCheck" checked="false" label="&fileCheck.label;"
pack="end"/>
        </row>
      </rows>
    </grid>

(For some reason I had thought before that if the printer description gets too
long, it would overlay. But of course that cannot happen...)

Comment 26

[Peter Weilbacher]

Another idea for the filename option. To not rob users of UI preferences like FF
does we could add a setting for the seldom used print filename option on the
Advanced page of the prefs notebook (there is still a lot of currently unused
space):

+- Features that help interpret web pages ------------------------+
|                                                                 |
| [ ] Enable Java                                                 |
|                                                                 |
+-----------------------------------------------------------------+
+- Print to file -------------------------------------------------+
|                                                                 |
| [ ] Always use the same filename for print to file              |
|                                                                 |
| Filename: [________________________________] ( Choose File... ) |
|                                                                 |
+-----------------------------------------------------------------+

(The Filename field is enabled when the checkbox is enabled.)
So that power users can, for whatever reason, specify a fixed filename there (or
in about:config) that is reflected in some pref (e.g. print.print_to_file.name)
and automatically used by the print dialog. There, the print to file checkbox
could be bordered in red (or theme dependent) color when pressed to signal the
user that this option is set.
I think this goes beyond this bug, but any thoughts on that?

Comment 27

[Peter Weilbacher]

*** Bug 67735 has been marked as a duplicate of this bug. ***

Comment 28

[[e-mail]]

(In reply to comment #25)
>(In reply to comment #23)
>>I think that this checkbox looks out of
>>place here, it would look better in the bottom right corner of the grid.
>You mean still on the same line with the printer description, like this:
That looks great, thanks!

Comment 29

[jag (Peter Annema)]

Comment on attachment 166251 [details] [diff] [review]
patch v3, following comments

r=jag, but before you check in make sure you make the change you suggested and
neil approved of in comment 25 and comment 28 respectively.

Comment 30

[Peter Weilbacher]

Attachment: final patch

OK, thanks a lot! This is the final patch, ready for checkin. It includes the
changes as in comment 25. Carrying over r+/sr+.

I don't have a CVS account yet, could someone be so kind to check this in?

Comment 31

[Stefan Borggraefe]

Patch checked in.