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

VERIFIED FIXED in mozilla1.4beta

Status

()

Core
Printing: Output
P1
blocker
VERIFIED FIXED
15 years ago
15 years ago

People

(Reporter: Daniel Wang, Assigned: Roland Mainz)

Tracking

({regression})

Trunk
mozilla1.4beta
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

831 bytes, patch
kaie
: review+
jag (Peter Annema)
: superreview+
(not reading, please use seth@sspitzer.org instead)
: approval1.4b+
Details | Diff | Splinter Review
(Reporter)

Description

15 years ago
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.*
(Assignee)

Comment 1

15 years ago
Ouch.
Flags: blocking1.4b?
OS: Windows 98 → All
Hardware: PC → All
Target Milestone: --- → mozilla1.4beta
(Assignee)

Comment 2

15 years ago
Daniel Wang wrote:
> regression from bug 185588

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

Comment 3

15 years ago
OK, I can see the same problem on Solaris when printing with Xprint...
confirming and accepting bug...
Severity: normal → blocker
Status: NEW → ASSIGNED
Priority: -- → P1
(Assignee)

Comment 4

15 years ago
Created attachment 122170 [details] [diff] [review]
Patch for 2003-04-25-08-trunk

Comment 5

15 years ago
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.
Attachment #122170 - Flags: superreview-
(Assignee)

Comment 6

15 years ago
Created attachment 122174 [details] [diff] [review]
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()| ?
Attachment #122170 - Attachment is obsolete: true

Comment 7

15 years ago
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.
Attachment #122174 - Flags: superreview+
(Assignee)

Comment 8

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

Requesting r= and a= ...
Attachment #122174 - Flags: review+
Attachment #122174 - Flags: approval1.4b?
Comment on attachment 122174 [details] [diff] [review]
New patch per jag's comments...

a=sspitzer
Attachment #122174 - Flags: approval1.4b? → approval1.4b+

Updated

15 years ago
Attachment #122174 - Flags: review+

Comment 10

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

r=kaie - assuming you have tested it works correctly.
Attachment #122174 - Flags: review+
(Assignee)

Comment 11

15 years ago
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

15 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

15 years ago
v
Status: RESOLVED → VERIFIED
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? 
(Reporter)

Comment 15

15 years ago
WADA, yes
(Assignee)

Comment 16

15 years ago
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... ;-(

Updated

15 years ago
Flags: blocking1.4b?
(Reporter)

Updated

15 years ago
Depends on: 178685
(Reporter)

Updated

15 years ago
No longer depends on: 178685
You need to log in before you can comment on or make changes to this bug.