191346 - RFE: "No printers found" when Xprint not installed should be "Xprint not installed" / there should be only one print error dialog for "no printers found" instead of two

Status: RESOLVED FIXED

(Core Graveyard :: Printing: Xprint, enhancement)

Description

[Bjørn Kutter]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0

When XPrint is not running I get a "No printers" found.
It is better to have a more descriptive error message in the dialog window.

Suggestions:
1. "No XPrint installed"
2. "No printers found and no XPrint available"
3. "No XPRint found, set $XPSERVERLIST"

Reproducible: Always

Steps to Reproduce:
1. unset XPSERVERLIST
2. netscape
3. print
Actual Results:  
Confusing message in the dialog

Expected Results:  
More descriptive error message

Comment 1

[Roland Mainz]

Taking bug...

Comment 2

[George Beckett]

Have encountered the same problem with Mozilla 1.3 running on Solaris 8. To my
knowledge, we do not have an XPrint server running on our system---previous
versions of Mozilla which we have installed have successfully printed despite
this, I assume by piping Postscript into "lpr". However, with the current
release I cannot print from Mozilla---I receive "There was a problem printing,
no printer could be found", and "Printing failed for some reason" messages. Am I
missing something? Thanks!

Comment 3

[Roland Mainz]

Attachment: Prototype patch for 2003-04-09-08-trunk

Comment 4

[Roland Mainz]

Attachment: Patch for 2003-04-09-08-trunk

The patch adds the new error code to give more detailed answers when there are
no print servers available.

Additionally I have fixed the broken error code propagation and that we usually
show two print error dialogs instead of one (failure in
gfx/src/*/nsDeviceContextSpec*.cpp 's
|GlobalPrinters::InitializeGlobalPrinters| _MUST_ call |FreeGlobalPrinters()|
on error - othwise the statement:
-- snip --
  if (PrintersAreAllocated()) {
    return NS_OK;
  }
-- snip --
will shortcut the path but returns |NS_OK| even when the previous call to
|GlobalPrinters::InitializeGlobalPrinters| returned an error (which is totally
WRONG).

Comment 5

[Roland Mainz]

Comment on attachment 120460 [details] [diff] [review]
Patch for 2003-04-09-08-trunk

Requesting r=/sr= ...

Comment 6

[John Keiser (jkeiser)]

Comment on attachment 120460 [details] [diff] [review]
Patch for 2003-04-09-08-trunk

r=me, with two changes:
(1) change nsIPrintContext::AvailablePrinters() to explain that it throws an
error if there are no printers (that will be surprising to consumers, but one
can argue that it is the right behavior until we can have an empty printer
enumerator)
(2) change the nsDeviceContextSpecG part of the patch to do the
mGlobalNumPrinters check at the end of the method, for readability and fewer
LOC.

Comment 7

[Roland Mainz]

OK, ToDo (per IRC discussion with jkeiser):
- Add some comments to
http://lxr.mozilla.org/seamonkey/source/gfx/idl/nsIPrintOptions.idl
- |GlobalPrinters::InitializeGlobalPrinters| should cache the error results
until the object is being destroyed. That will save lots of time if printer
lookup is _slow_ (like for remote print queues if the print server is offine and
we wait for "TCP connect"-timeout)

Comment 8

[Boris Zbarsky [:bzbarsky]]

I will not be getting to this review any time in the near future (like in the
next week or two).

Comment 9

[Roland Mainz]

Attachment: New patch to address jkeiser's comments

Comment 10

[Roland Mainz]

Attachment: New patch for 2003-04-09-08-trunk incl. GTK+ and Xlib gfx changes

Comment 11

[Roland Mainz]

Comment on attachment 120633 [details] [diff] [review]
New patch for 2003-04-09-08-trunk incl. GTK+ and Xlib gfx changes

Requesting r= ...

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think you should add something to the message saying "Check the 'XPSERVERLIST'
environment variable'".

Comment 13

[John Keiser (jkeiser)]

Comment on attachment 120633 [details] [diff] [review]
New patch for 2003-04-09-08-trunk incl. GTK+ and Xlib gfx changes

Eek, nsIPrintOptions was under review for some reason?	Best leave the @status
in, sorry, that was my fault, I thought you were adding it in yourself.

Note that no printers are available is not really an error case for a function
like availablePrinters(), but given that we're throwing an exception, I guess
that's fine.

Could you please move FreeGlobalPrinters() up above the #ifdef USE_XPRINT? 
That way you only need one FreeGlobalPrinters(), not two.  Not a huge deal :)

Comment 14

[Roland Mainz]

Attachment: New patch for 2003-04-09-08-trunk per jkeiser's and roc+moz's comments

Comment 15

[Roland Mainz]

Robert O'Callahan wrote:
> I think you should add something to the message saying "Check the 
> 'XPSERVERLIST' environment variable'".

Fixed.

Comment 16

[Roland Mainz]

John Keiser wrote:
> (From update of attachment 120633 [details] [diff] [review])
> Eek, nsIPrintOptions was under review for some reason?  Best leave the @status
> in, sorry, that was my fault, I thought you were adding it in yourself.

Erm, we should never even _THINK_ about freezing |nsIPrintOptions| nor
|nsIPrintSettings| and some other related stuff (and whoever tries to do that
should be boiled in hot oil... xx@@@!!!). These APIs still need a complete
revamp. If they get frozen we will be horrible in trouble since lots of
bugs/issues can't be fixed after that point...

IMHO we should find a way to sit together (physically!) with a huge whiteboard
and discuss how we can make the current mozilla printing architecture better and
cleaner (we need a Gecko printing guru (jkeiser?), Win32 guru (smontagu?), a
Unix/Linux expert (I can do that) and a Mac god (smfr?) for that... :) ...

[snip]
> Could you please move FreeGlobalPrinters() up above the #ifdef USE_XPRINT? 
> That way you only need one FreeGlobalPrinters(), not two.  Not a huge deal :)

Fixed.

Comment 17

[John Keiser (jkeiser)]

Comment on attachment 120754 [details] [diff] [review]
New patch for 2003-04-09-08-trunk per jkeiser's and roc+moz's comments

I agree the interface should never be frozen, but I'm not the guy who decides
such things, and valeski must have had some reason to put it on the list of
interfaces to review.  We shouldn't remove the status without talking to him.

Regarding fixing pritning: once we fix the layout side of things, which I'm
working on now, we need to fix the init code, which should happen once we
decide not to have multiple presentations at the same time.  This looks likely
to happen: see bug 201767 for reasons and current discussion.  I think things
will largely fall out from that: either we clone the document to print it (my
preference), or we play with the existing presentation (I'm not real happy with
that).	There are major issues with both, maybe fewer issues with the latter
though.  The former will involve grabbing network resources that may no longer
be in cache and have the same problems as view source and save as.

Comment 18

[Roland Mainz]

Comment on attachment 120754 [details] [diff] [review]
New patch for 2003-04-09-08-trunk per jkeiser's and roc+moz's comments

Requesting sr= and checkin= ...

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in