203937 - an extra "print." is added to printing preference name

Status: VERIFIED FIXED

(Core :: Printing: Output, defect, P1)

Description

[Daniel Wang]

regression from bug 185588

"print." was is twice /gfx/src/nsPrintOptionsImpl.cpp,
first in line 128

    nsCOMPtr<nsIPrefService> prefService = do_GetService(NS_PREFSERVICE_CONTRACTID);
    if (prefService) {
         prefService->GetBranch("print.", getter_AddRefs(mPrefBranch));
    }

then in line 327
   mPrefName.AssignWithConversion(NS_LITERAL_STRING("print."));

so, instead of
   print.printer_HP_LaserJet_1100.*

I now have
   print.print.printer_HP_LaserJet_1100.*

Comment 1

[Roland Mainz]

Ouch.

Comment 2

[Roland Mainz]

Daniel Wang wrote:
> regression from bug 185588

... AFAIK that bug only backed-out another patch. I doubt this is the root of
the problem.

Comment 3

[Roland Mainz]

OK, I can see the same problem on Solaris when printing with Xprint...
confirming and accepting bug...

Comment 4

[Roland Mainz]

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

Comment 5

[jag (Peter Annema)]

Comment on attachment 122170 [details] [diff] [review]
Patch for 2003-04-25-08-trunk

+  mPrefName.AssignWithConversion(NS_LITERAL_STRING(""));

Instead of that, just do

mPrefName.Truncate()

And then instead of
mPrefName.AppendWithConversion(NS_LITERAL_STRING("printer_"));
just do mPrefName.Append("printer_")

Same for the '.'

Note that the AppendWithConversion of aPrinterName will be lossy, which could
be bad if aPrinterName is truly unicode.

Comment 6

[Roland Mainz]

Attachment: New patch per jag's comments...

jag (Peter Annema) wrote:
> (From update of attachment 122170 [details] [diff] [review])
> +  mPrefName.AssignWithConversion(NS_LITERAL_STRING(""));
>
> Instead of that, just do
>
> mPrefName.Truncate()

Fixed.

> And then instead of
> mPrefName.AppendWithConversion(NS_LITERAL_STRING("printer_"));
> just do mPrefName.Append("printer_")

Fixed.

> Same for the '.'

Fixed.

> Note that the AppendWithConversion of aPrinterName will be lossy, which could

> be bad if aPrinterName is truly unicode.

Mhhh, the whole printing prefs stuff isn't really designed to handle unicode
(and noone really cared to fix the mess in the last three years... ;-(( ) ...
... but maybe we can start to clean up now:
What do you suggest to use instead of |AppendWithConversion()| ?

Comment 7

[jag (Peter Annema)]

Comment on attachment 122174 [details] [diff] [review]
New patch per jag's comments...

Append(NS_ConvertUCS2toUTF(aPrinterName)) would do the trick, but if the
printer name was using any non-ascii character in its name, that would then
result in a different mPrefName being generated in this code.

Comment 8

[Roland Mainz]

Comment on attachment 122174 [details] [diff] [review]
New patch per jag's comments...

Requesting r= and a= ...

Comment 9

[(not reading, please use [e-mail] instead)]

Comment on attachment 122174 [details] [diff] [review]
New patch per jag's comments...

a=sspitzer

Comment 10

[Kai Engert [:KaiE:]]

Comment on attachment 122174 [details] [diff] [review]
New patch per jag's comments...

r=kaie - assuming you have tested it works correctly.

Comment 11

[Roland Mainz]

Kai Engert wrote:
> (From update of attachment 122174 [details] [diff] [review])
> r=kaie - assuming you have tested it works correctly.

Sure, otherwise I wouldn't have asked for review... :)

Comment 12

[Kai Engert [:KaiE:]]

checked in

Comment 13

[Daniel Wang]

v

Comment 14

[WADA:World Anti-bad-Duping Agency]

With 2003050211-trunk/Win-Me, duplicate "print." problem has been fixed.
However, printer name portion of print. keyword was still written in Shift_JIS
when printer name contains Japanese characters on Japanese MS Windows Me(This
will cause problem of bug 197271).
I expected the patch will resolve such problems but it wasn't.

Did the patch fix duplicate "print." only? 

Comment 15

[Daniel Wang]

WADA, yes

Comment 16

[Roland Mainz]

WADA wrote:
> Did the patch fix duplicate "print." only?

Yes, this patch was only for fixing the DUPlicate "print.".
I know there are lots of i18n problems with printing prefs, but the problems are
shattered all over the code, mainly with the assumtion that OS uses UTF-8 as
default encoding. In theory the fix for that is easy after bug 171809 has been
fixed - but without that we can do nothing about these issues... ;-(