1425188 - Remove the Postscript option from the GTK print dialog window

Status: RESOLVED FIXED

(Core :: Widget: Gtk, enhancement)

Description

[Jonathan Watt [:jwatt]]

As a step towards removing cairo from the tree, we should remove the Postscript option from the GTK print dialog window. (This option appears when the "Print to File" "Printer" is selected.

(Chromium does not provide the ability to output Postscript FWIW.)

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 3

[Karl Tomlinson (:karlt)]

Comment on attachment 8936755 [details] [diff] [review]
patch

>-    const gchar* fmtGTK =
>-      gtk_print_settings_get(mPrintSettings,
>-                             GTK_PRINT_SETTINGS_OUTPUT_FILE_FORMAT);
>-    if (fmtGTK) {
>-      if (nsDependentCString(fmtGTK).EqualsIgnoreCase("pdf")) {
>-        format = nsIPrintSettings::kOutputFormatPDF;
>-      } else {
>-        format = nsIPrintSettings::kOutputFormatPS;
>-      }

nsPrintSettingsGTK::SetToFileName() can still set "ps" in the
GtkPrintSettings.  Wouldn't this cause a problem if GTK thinks the output is
PS but Gecko thinks it is PDF?

Comment 4

[Jonathan Watt [:jwatt]]

(In reply to Karl Tomlinson (back Jan 3 :karlt) from comment #3)
> Wouldn't this cause a problem if GTK thinks the output is
> PS but Gecko thinks it is PDF?

It would do, *if* that were to happen.

nsPrintSettingsGTK::SetToFileName seems to have three callers:

- nsPrinterEnumeratorGTK::InitPrintSettingsFromPrinter passes a
  hardcoded ".pdf" file name.

- nsPrintOptions::DeserializeToPrintSettings deserializes and
  passes whatever value was saved to cache previously (as opposed
  to creating something that might be .ps).

- nsPrintOptions::ReadPrefs passes the value saved to the
  pref print.print_to_filename (or a
  print.<printer-name>.print_to_filename pref).

Only the latter seems capable of passing a ".ps" file name to SetToFileName, and presumably those prefs can have .ps values if the user last saved to file a Postscript file.

Since this patch would remove the save as Postscript option from the print dialog and only allowing PDF, it would seem to make sense to me to:

- convert any old .ps value from the prefs in nsPrintOptions::ReadPrefs

- replace the .ps part of the if-else check in nsPrintSettingsGTK::SetToFileName
  with an assertion

Comment 5

[Jonathan Watt [:jwatt]]

Good catch BTW.

Comment 6

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8940008 [details] [diff] [review]
patch

>+  MOZ_ASSERT(StringEndsWith(aToFileName, NS_LITERAL_STRING(".ps")),
>+             "We should no longer be asked to save Postscript files");

nsIPrintSettings toFilename is scriptable, and such a restriction in the
filename is not documented, and so I'm not happy with this assertion.
r+ on the rest of the patch if you drop this assertion.

>+      if (StringEndsWith(str, NS_LITERAL_STRING(".ps"))) {
>+        // We only support PDF since bug 1425188 landed.  Users may still have
>+        // prefs with .ps filenames if they last saved a file as Postscript
>+        // though, so we fix that up here.  (The pref values will be
>+        // overwritten the next time they save to file as a PDF.)
>+        str.Truncate(str.Length() - 2);
>+        str.AppendLiteral("pdf");
>+      }

In a build without this patch, if I print to file and set the filename to
mozilla.svg, then I have print.print_to_filename;/home/karl/mozilla.svg, but
printing again, the dialog shows mozilla.pdf ‎so I suspected that manually
changing the pref would be unnecessary.  However, I assume it was
file_printer_output_file_format_changed() changing the suffix, and that
function would not be called when only PDF is supported, and so I expect
different behavior with this patch.

Changing the pref suffix from ps to pdf is therefore probably helpful to
indicate what's changed to any users in this situation, and so r+ on this
change.  If this causes any problems, however, I'm also happy if you drop this
part.  UNIX systems are usually more clever than assuming that a filename
indicates its format.

Comment 8

[Jonathan Watt [:jwatt]]

(In reply to Karl Tomlinson (:karlt) from comment #7)
> However, I assume it was
> file_printer_output_file_format_changed() changing the suffix, and that
> function would not be called when only PDF is supported, and so I expect
> different behavior with this patch.

Yes, that's right. So this part of the patch is indeed needed to make sure we don't save PDF contents into a .ps file for users that last saved to a Postscript file before getting this change.

> UNIX systems are usually more clever than assuming that a filename
> indicates its format.

True, but it would still be poor form to create PDF files with a .ps extension so I'll keep this part of the patch.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c85b83645bb3
Remove the ability to save as Postscript from the print dialog on Linux. r=karlt

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c85b83645bb3