235763 - Print dialog show list printer descriptions if available

Status: RESOLVED FIXED

(Core :: Printing: Output, enhancement)

Description

[Roland Mainz]

RFE: Print dialog show list printer descriptions if available.

Some print APIs (such as Xprint :) allow that printers have descriptions
additional to their name. It would be nice if we could fetch that information
and display that in the print dialog.

Comment 1

[Roland Mainz]

Attachment: Prototype patch

Comment 2

[Roland Mainz]

There is one problem:
The current selection is not being rendered in the <menupopup>. When I open the
popup everything is rendered OK, but the selection remains "white" (or "blue"
when selected).

Neil/bz:
Any ideas what I am doing wrong ?

Comment 3

[Roland Mainz]

Attachment: Screenshot showing the print dialog after attachment 142433

Comment 4

[Roland Mainz]

Attachment: Patch for 2004-02-28-trunk

Comment 5

[Roland Mainz]

Comment on attachment 142517 [details] [diff] [review]
Patch for 2004-02-28-trunk

Requesting r=/sr= ...

Comment 6

[[e-mail]]

Comment on attachment 142517 [details] [diff] [review]
Patch for 2004-02-28-trunk

>+function buildPrinterDescription(printerName)
>+{
>+  var XHTMLNS = "http://www.w3.org/1999/xhtml";
>+
>+  var name_elem = document.createElementNS(XHTMLNS, "div");
>+  var name_text = document.createTextNode(printerName);
>+  name_elem.appendChild(name_text);
>+
>+  var desc_elem = document.createElementNS(XHTMLNS, "i");
>+  var desc_text = document.createTextNode(" / " + getPrinterDescription(printerName));
>+  desc_elem.style.color = "gray";
>+  desc_elem.appendChild(desc_text);
>+  
>+  name_elem.appendChild(desc_elem);
>+    
>+  return name_elem;
>+}
This is a good example of how not to style chrome.

Comment 7

[Roland Mainz]

[e-mail] wrote:
> (From update of attachment 142517 [details] [diff] [review])
> >+function buildPrinterDescription(printerName)
> >+{
> [snip]
> >+}
> This is a good example of how not to style chrome.

The other "solutions" offered all have their catch, either being non-functional
or an overkill (such as XBL... and then we need CSS... and then it turns out
that the whole mess still needs more adjustments which are beyond my power. And
if someone hacks a solution for that noone except the author him-/herself is
able to maintain that... please: NO.). 

Comment 8

[[e-mail]]

(In reply to comment #7)
>and then we need CSS...
Just how did you think themes work? To take an example, in the Sky Pilot theme
menuitems have a gray background, so your description would be unreadable...

Comment 9

[Daniel Brooks [:db48x]]

bug 235872 has the solution to that part of the problem.

Comment 10

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

Comment on attachment 142517 [details] [diff] [review]
Patch for 2004-02-28-trunk

chrome is really beyond my expertise

Comment 11

[Roland Mainz]

Comment on attachment 142517 [details] [diff] [review]
Patch for 2004-02-28-trunk

db48x:
Can you review the patch and/or adjust it to use the new functionality
introduced with bug 235872, please ?

Comment 12

[Roland Mainz]

Attachment: New patch per db48x's comments (note that this patch depends on the patch in bug 235872)

Comment 13

[Daniel Brooks [:db48x]]

Comment on attachment 142878 [details] [diff] [review]
New patch per db48x's comments (note that this patch depends on the patch in bug 235872)

>+  if ( s != "" )
>+    return s;
>+    
>+  return "-";


Just make this a 'return s'. the - looks funny all by itself.

With that minor change, and provided the sr accepts it, r=db48x

Comment 14

[Roland Mainz]

Attachment: New patch per db48x's comments

Comment 15

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

Comment on attachment 142931 [details] [diff] [review]
New patch per db48x's comments

will this actually work with I18N printer names? Seems like you're stuffing the
UTF8 bytes into the pref name, and then reading the pref via Javascript call to
IDL 'string', which will not translate chars >= 256 to UTF8. Right?

Comment 16

[[e-mail]]

Comment on attachment 142931 [details] [diff] [review]
New patch per db48x's comments

>-      mGlobalPrinterList->AppendString(nsString(NS_ConvertASCIItoUCS2(plist[i].name)));
>+      /* Add name to our list of printers... */
>+      mGlobalPrinterList->AppendString(nsString(NS_ConvertUTF8toUCS2(plist[i].name)));
Doesn't NS_ConvertUTF8toUCS already return an nsString?

>+    s = gPrefs.getCharPref("print.printer_" + printerName + ".printer_description")
roc's right, you'll need to use getComplexValue(prefName,
Components.interfaces.nsISupportsString).data; here.

Comment 17

[Daniel Brooks [:db48x]]

Comment on attachment 142931 [details] [diff] [review]
New patch per db48x's comments

>+          <menulist id="printerList" flex="1" oncommand="setPrinterDefaultsForSelectedPrinter();"/>

this needs type="description", other than that it's good

Comment 18

[Roland Mainz]

Robert O'Callahan wrote:
> (From update of attachment 142931 [details] [diff] [review])
> will this actually work with I18N printer names? Seems like you're stuffing 
> the UTF8 bytes into the pref name, and then reading the pref via Javascript 
> call to IDL 'string', which will not translate chars >= 256 to UTF8. Right?

Right now the print dialog doesn't really work with non-ascii names (the
PostScript printing backend does not support then and for Xprint we have to wait
until I am done with the native charset fixes (one of my other bugs I am working
on)... if that's done I am going to fix everything which needs to be done to
make the backend and frontend "i18n-enabled" (this is beyond the scope of this
bug)).

Comment 19

[Roland Mainz]

Attachment: New patch per db48x's comments

Comment 20

[Roland Mainz]

Comment on attachment 149428 [details] [diff] [review]
New patch per db48x's comments

Requesting r=/sr= and checkin= from roc+moz...

Comment 21

[Roland Mainz]

[e-mail] wrote:
> >+    s = gPrefs.getCharPref("print.printer_" + printerName +
> ".printer_description")
> roc's right, you'll need to use getComplexValue(prefName,
> Components.interfaces.nsISupportsString).data; here.

jshin:
Is that right ?

Comment 22

[Roland Mainz]

Attachment: New patch with added comment about the issue listed in comment #16

Comment 23

[Roland Mainz]

Attachment: Screenshot: CDEnext's print dialog

Comment 24

[Roland Mainz]

Attachment: Screenshot: Mozilla's new printdialog (after attachment #149430)

Comment 25

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

checked in, please mark fixed if it is

Comment 26

[Roland Mainz]

bryner:
Do you want this patch for FireFox, too ?

Comment 27

[Roland Mainz]

> bryner:
> Do you want this patch for FireFox, too ?

The dependicy bug 235872 was marked approval1.7- (since it is a feature, no real
bugfix) so we can forget the idea to include this into firefox/thunderbird...
;-((

... anyway... marking bug as FIXED.

Comment 28

[Roland Mainz]

For the log:
Followup is bug 171809 ("Create |NS_ConvertUnicodeToNative| and
|NS_ConvertNativeToUnicode|") to add the neccesary code to make the print dialog
useable for non-ASCII text.