1657006 - Investigate whether nsPrintSettingsService::GetLastUsedPrinterName may fail

Status: RESOLVED FIXED

(Core :: Printing: Setup, task, P2)

Description

[Jonathan Watt [:jwatt]]

nsPrintSettingsService::GetLastUsedPrinterName checks to see whether the last used printer is valid, and uses the system default printer if it is not.

However, it's not clear to me whether GetSystemDefaultPrinterName will always be valid. I believe on Windows it comes from a system .ini file and could be stale. That may not have mattered when we used the system printing dialog, since that may have corrected it silently to a valid printer, and we weren't using the name to query for information about that printer anyway. In the new code flow it may break printing until the user prints using the system dialog and resets the last used/default printer...but if a broken printer name breaks printing so that we can't open the new UI to then open the system printing dialog then IT MAY COMPLETELY BREAK PRINTING.

We should probably make nsPrintSettingsService::GetLastUsedPrinterName more robust. Maybe have it do a second loop through the current printer list with the value returned by GetSystemDefaultPrinterName, and choose the first printer in the list if the "system default" is not valid?

Comment 1

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Watt [:jwatt] from comment #0)

However, it's not clear to me whether GetSystemDefaultPrinterName will always be valid. I believe on Windows it comes from a system .ini file and could be stale.

As it happens, the patch I posted in bug 1656733 to reimplement printer enumeration also switches this code to use a standard Win32 API to get the default printer.

(We should still verify how things behave if the user removes their default printer, but I believe the usual behavior here is that Windows will automatically choose a new default, and the Win32 GetDefaultPrinter API should give the appropriate result.)

Comment 2

[Emily McDonough [:alaskanemily]]

It can definitely fail on CUPS backends, but it can't return a bogus printer name. The printer must be in the list of CUPS printers available.

You can break it just by setting the environment variables LPDEST or PRINTER to non-empty (and for PRINTER non "lp") values, or (on OS X) setting the environment variable CUPS_NO_APPLE_DEFAULT. In those cases, we will end up looking for a bogus printer name, fail to find it in the available printers, and GetSystemDefaultPrinterName will result in NS_ERROR_GFX_PRINTER_NO_PRINTER_AVAILABLE.

The CUPS code for this is here for how the default printer is fetched from the list of available printers: https://github.com/apple/cups/blob/56ba2f512297cff5cc629aa920b0b3858bfa304e/cups/dest.c#L1743
And here for how the default name is resolved: https://github.com/apple/cups/blob/56ba2f512297cff5cc629aa920b0b3858bfa304e/cups/dest.c#L2234

Comment 3

[Sean Voisen (:svoisen)]

Emily: (In reply to Emily McDonough [:alaskanemily] from comment #2)

It can definitely fail on CUPS backends, but it can't return a bogus printer name. The printer must be in the list of CUPS printers available.

Do we still need this bug open then?

Comment 4

[Jonathan Watt [:jwatt]]

(In reply to Jonathan Kew (:jfkthame) from comment #1)

(We should still verify how things behave if the user removes their default printer, but I believe the usual behavior here is that Windows will automatically choose a new default, and the Win32 GetDefaultPrinter API should give the appropriate result.)

Would you be able to see if you can verify this?

Comment 5

[Jonathan Kew [:jfkthame]]

In my testing, whenever I remove the default printer, Windows immediately chooses a new default automatically. This doesn't get reflected in our UI if it is currently open (the no-longer-available printer is still shown as a destination), but any attempt to print to it will (naturally enough) fail.

(I filed bug 1664415 for a minor cosmetic issue with the resulting error message, but basically we seem to handle this OK.)

If the UI is then re-opened, the printer is no longer offered as a destination, and I see the new default destination selected. So AFAICS this is behaving as expected.