[FIX]Japanese named printer cannot be recognized.

RESOLVED FIXED in Future

Status

()

Core
Printing: Output
P2
normal
RESOLVED FIXED
15 years ago
13 years ago

People

(Reporter: Frank Tang, Assigned: John Keiser (jkeiser))

Tracking

({fixed1.7, intl, qawanted})

Trunk
Future
x86
Windows 98
fixed1.7, intl, qawanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-aviary1.0)

Attachments

(4 attachments, 4 obsolete attachments)

(Reporter)

Description

15 years ago
here is one japanese bug report we saw about Mozilla 1.0

Operating System: 	Win98
Language: 		Japanese
Issue Summary: 		Problem of print
Component: 		International
Doing What: 		Using Japanese localized
Severity: 		SomethingDidNotWorkRight
Can Reproduce: 		Always
Try this URL: 		http://
Issue Detail:
The printer cannot be recognized that Japanese is
included in the name of the
printer used usually.
Email Address: 		toy@fvd.fujitsu.com

and our QA cannot create the environment to test it:
Kasumi Ketron wrote:

Unfortunately, we can't confirm it since we can't change  printer name having
DBCS on server.


Jay Yan: Can you folks create printer name in Chinese and help us debug it ?

Updated

15 years ago
Summary: Japanese named printer cannot be recognized. → Japanese named printer cannot be recognized.

Comment 1

15 years ago
WFM on Win2000 in Chinese locale
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.0) Gecko/20020530

Comment 2

15 years ago
In comment 1, I just tried to setup local printer's name to let it include some
Chinese character. Mozilla1.0 works well. It can list the printer's name and
print use that printer without problem.

Comment 3

15 years ago
On Linux, even locale is Chinese, can't change printer name to Chinese
character. And On Solaris, printer was selected from a printer list, which can't
be modified to Chinese character also. That is to say, on Linux or Solaris this
case can't be reproduced. It's a bug only on Windows.

Updated

15 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2alpha

Comment 4

15 years ago
Problem is probably:
http://lxr.mozilla.org/mozilla/source/embedding/components/printingui/src/win/nsPrintDialogUtil.cpp#777
http://lxr.mozilla.org/mozilla/source/embedding/components/printingui/src/win/nsPrintDialogUtil.cpp#891
http://lxr.mozilla.org/mozilla/source/embedding/components/printingui/src/win/nsPrintDialogUtil.cpp#1198
(this isn't turned on yet)

Fix coming....

Comment 5

15 years ago
Created attachment 98462 [details] [diff] [review]
patch 

Uses the GetACPString, to get the properly localized string.

Updated

15 years ago
Summary: Japanese named printer cannot be recognized. → [FIX]Japanese named printer cannot be recognized.

Comment 6

15 years ago
Comment on attachment 98462 [details] [diff] [review]
patch 

Here is what I think you should do:

@@ -777,7 +777,10 @@
 #ifdef UNICODE
-     prtName.AppendWithConversion((PRUnichar *)aPrintName);
+     prName.Append(aPrintName);  // <====  since aPrintName is WCHAR when
UNICODE is on.
 #else 
-    prtName.AssignWithConversion((char*)aPrintName);
+    PRUnichar* dbStr = GetUCS2String(nsCString(prtName)); // <==== you need to
write GetUCS2String() which calls MultiByteToWideChar()
+    prtName.Assign(dbStr);
+    free(dbStr);
 #endif

----------------------------------
and for the last section, 

@@ -1198,7 +1204,9 @@
 #ifdef UNICODE
     hGlobalDevMode = CreateGlobalDevModeAndInit(printerName, aPrintSettings);
 #else 
-    hGlobalDevMode =
CreateGlobalDevModeAndInit((char*)NS_ConvertUCS2toUTF8(printerName).get(),
aPrintSettings);
+  char* dbStr = GetACPString(nsString(printerName));  // <==== you can do this
since printerName is PRUnichar*
+  hGlobalDevMode = CreateGlobalDevModeAndInit(dbStr, aPrintSettings);
+  free(dbStr);
 #endif
   }
Attachment #98462 - Flags: needs-work+

Updated

15 years ago
Keywords: mozilla1.2, nsbeta1
Whiteboard: [adt2] [ETA Needed]

Comment 7

15 years ago
Created attachment 98716 [details] [diff] [review]
revised patch v2

With Roy's suggestion, plus there is no need to convert the Printer Name using
GetACPString inside CreateGlobalDevModeAndInit, it is always passed in
correctly. 

We could convert the name inside CreateGlobalDevModeAndInit, but all the
platform calls require a LPTSTR, so this is the best way.
Attachment #98462 - Attachment is obsolete: true

Updated

15 years ago
Target Milestone: mozilla1.2alpha → mozilla1.2beta
Comment on attachment 98716 [details] [diff] [review]
revised patch v2

sr=blizzard
Attachment #98716 - Flags: superreview+

Comment 9

15 years ago
How about prName?  Do we need to convert to UCS2 by calling MultiByteToWideChar()?

Comment 10

15 years ago
Created attachment 100602 [details] [diff] [review]
patch v3

I have removed the UNICODE #ifdefs (no one compiles with this on and it only
confuses things)

I also found where I was using UTF8 and shouldn't have been.

Updated

15 years ago
Attachment #98716 - Attachment is obsolete: true

Comment 11

15 years ago
Comment on attachment 100602 [details] [diff] [review]
patch v3

looks good.
/r=yokoyama
Attachment #100602 - Flags: review+
Comment on attachment 100602 [details] [diff] [review]
patch v3

sr=blizzard
Attachment #100602 - Flags: superreview+

Comment 13

15 years ago
fixed

Note this fixed Bug 169400 which is an issue for 7.0 may want to take this in
the respin)
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 14

15 years ago
Got this nomination from Rod via email.  The Blackbird team (adt) will consider
bugs for the release which meet all of the following criteria:
          o Meet Blackbird release criteria
                + Required feature of the release. 
                + Regression caused by changes made in this release.  
                + Top crash
                + Security
          + Other product dependencies:  I.e., a bug blocking a TBird pull
          o Has baked on the trunk
          o Has a patch for the branch including r= and sr= for that patch
          o Passes additional review as required by Blackbird team
          o Has adt1.0.2 keyword
          o Can be tested, given very limited QA resources in this release

If you believe the proposed fix meets the criteria above, please make sure to
add the keyword adt1.0.2 to the bug.  The team looks at bugs regularly and we
will update the bug with additional information we might need, or else a
decision on this nomination.


nsbeta1+
Keywords: nsbeta1 → nsbeta1+

Comment 16

15 years ago
Created attachment 103347 [details] [diff] [review]
patch for the 1.0.2 tree

patch for the branch (applied and built directly from the trunk patch)
Comment on attachment 103347 [details] [diff] [review]
patch for the 1.0.2 tree

sr=blizzard
Attachment #103347 - Flags: superreview+

Comment 18

15 years ago
Comment on attachment 103347 [details] [diff] [review]
patch for the 1.0.2 tree

r=dcone
Attachment #103347 - Flags: review+
Comment on attachment 103347 [details] [diff] [review]
patch for the 1.0.2 tree

a=rjesup@wgate.com for 1.0 branch; change mozilla1.0.2+ to fixed1.0.2 when
checked in
Attachment #103347 - Flags: approval+

Updated

15 years ago
Keywords: mozilla1.0.2+

Updated

15 years ago
Keywords: fixed1.0.2

Comment 20

15 years ago
Can someone from International please verify this and mark.

Yuying, I have changed you to the qa_contact on this bug.

thanks.
QA Contact: sujay → ylong

Comment 21

15 years ago
Like the comment in original report, there is no such kind of printer names
existing in our network server, so it's really hard for us to verify this bug
fix here.

Is there any one who can set Japanese printer name can help us to verify it?

Kat, can we find someone in mozilla-Japan to do this? thanks!

Comment 22

15 years ago
OK, I still haven't get any response from some one who has real network Japanese
named printer.

Here is what I did for verification on Win98-Japanese:
1. Change an exisiting network Enlish printer name to Japanese name.
2. Launch 11-07 branch (1.0.2) build, File | Print.

Result:
There is no print dialog show up, but with an error message instead: can't not
find printer.

IE or Wordpad:
The print dialog will pop-up, and I'm able to see the Japanese printer name. 

3. Change the printer name from Japanese to some ascii characters, then try to
print.
- now the print dialog will show-up.

So, is the problem real get fixed?

Comment 23

15 years ago
I guess the good news is, it doesn't crash, because this was cuasing a crash for
a user.

Does a current build behave the same way?

Comment 24

15 years ago
> Does a current build behave the same way?

I was using the lateset 11-07 branch (1.0.2) build, since there is fixed1.0.2
keyword, I assume the fix was checked in branch build, but the result in comment
#22 seems show the problem is not really get resolved. 

Comment 25

15 years ago
So do you want to reopen this bug in the blackbird branch? If yes, please remove
the fixed1.0.2 keyword. If you are satisfied that atleast it does not crash than
mark this bug verified1.0.2 and open a new bug for the symptoms in comment #22.
Thanks.

Comment 26

15 years ago
Based on my test result and the summary of this bug (the Japanese named printer
does NOT be recognized on current build), I'm re-open this bug and remove the
fixed 1.0.2 keyword.
Status: RESOLVED → REOPENED
Keywords: fixed1.0.2
Resolution: FIXED → ---

Comment 27

15 years ago
I can back the change out and we can test again?

Comment 28

15 years ago
Yuying, does the current 7.0 release behave better and not crash when you have a
Japanese printer name? If so, then we should back it out.

Comment 29

15 years ago
If there was some one has a crash before this patch, the behave of the latest
branch build is better because I didn't see the crash.

And I tried in on N7.0RTM, the result is the same as in latest branch build.

But in order to verify the real problem, I think we better to have some one who
has the real Japanese network printer name to test it.  Because what I did was
just rename the exisiting english printer name to Japanese, and I got a "not
found" error.
-> jkeiser
Assignee: rods → jkeiser
Status: REOPENED → NEW
nsbeta1-. John is overloaded with higher priority issues. 

Adding qawanted to try out a current build on a "real Japanese network printer"
Keywords: mozilla1.2, nsbeta1+ → nsbeta1-, qawanted
Priority: -- → P2
Target Milestone: mozilla1.2beta → Future

Comment 32

14 years ago
We've also encountered reports of unrecognized printers whose name include
Hebrew characters. Judging from this and other bugs (such as Bug 169206), this
issue is not Japanese-specific.

Prog.

Updated

14 years ago
Keywords: intl

Comment 33

14 years ago
*** Bug 187399 has been marked as a duplicate of this bug. ***

Comment 34

14 years ago
Now WinXP is OK. Win9x is not.
http://lxr.mozilla.org/seamonkey/source/embedding/components/printingui/src/win/nsPrintDialogUtil.cpp#763
I think that 
WinXP cat treat unicode (unicode API OpenPrinter
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/gdi/prntspol_9qnm.asp
) but Win 9x in not.
I think that we need to change here like widget/src/windows/nsToolkit.cpp,
NS_IMM_GETCOMPOSITIONSTRINGW of
widget/src/windows/nsWindows.cpp, etc)
and WinXP/2K use unicode and Win9x use OS native coding that depend on
region/language of OS.

Comment 35

13 years ago
Created attachment 143086 [details] [diff] [review]
make convert correct

My guess of comment 34 is wrong.

http://lxr.mozilla.org/seamonkey/source/gfx/src/windows/nsDeviceContextSpecWin.cpp#201

broke the rule of 
http://www.mozilla.org/projects/xpcom/string-guide.html
It is coincidence that it works well recent code on WinXP/JP.

Comment 36

13 years ago
Comment on attachment 143086 [details] [diff] [review]
make convert correct

--- nsDeviceContextSpecWin.cpp.org	2004-02-29 07:33:57.000000000 +0900
+++ nsDeviceContextSpecWin.cpp	2004-03-06 15:56:37.656250000 +0900
@@ -50,6 +50,9 @@
 #include "nsReadableUtils.h"
 #include "nsGfxCIID.h"

+// For NS_CopyNativeToUnicode
+#include "nsNativeCharsetUtils.h"
+
 // File Picker
 #include "nsILocalFile.h"
 #include "nsIFile.h"
@@ -198,12 +201,8 @@
   LPTSTR lpPrtName;
   GlobalPrinters::GetInstance()->GetDefaultPrinterName(lpPrtName);
   nsString str;
-#ifdef UNICODE
-  str.AppendWithConversion((PRUnichar *)lpPrtName);
-#else 
-  str.AssignWithConversion((char*)lpPrtName);
-#endif
-  printerName = ToNewUnicode(str);
+  NS_CopyNativeToUnicode(nsDependentCString((char *)lpPrtName), str);
+  printerName = (PRUnichar *)str.get();
   free(lpPrtName);
   return printerName;
 }

Comment 37

13 years ago
(In reply to comment #36)

>    nsString str;

> +  NS_CopyNativeToUnicode(nsDependentCString((char *)lpPrtName), str);
> +  printerName = (PRUnichar *)str.get();

You're returning a pointer to a buffer in the stack. You should duplicate the
buffer with ToNewUnicode() here. 

Comment 38

13 years ago
Created attachment 143155 [details] [diff] [review]
convert make well

(In reply to comment #37)
Yes I take basically mistake.
It works well on XP and 98Se.

please review it.
Attachment #143086 - Attachment is obsolete: true

Comment 39

13 years ago
If Bug 169206 is differnt from this bug I don't know why this bug still opening.

My patch for gfx\src\windows\nsDeviceContextSpecWin.cpp.org seems to be Bug
169206's one.

I don't know how I do.
But I want to review my patch...

Comment 40

13 years ago
Comment on attachment 143155 [details] [diff] [review]
convert make well

Please review it
Attachment #143155 - Flags: review?(john)

Comment 41

13 years ago
Comment on attachment 143155 [details] [diff] [review]
convert make well

Please review it

Comment 42

13 years ago
Comment on attachment 143155 [details] [diff] [review]
convert make well

It also have I18N aspects. Jshin:
Could you mind sr?
Attachment #143155 - Flags: superreview?(jshin)

Comment 43

13 years ago
Comment on attachment 143155 [details] [diff] [review]
convert make well

I can't sr, but here's r=jshin.

However, this patch is just a temporary measure. Even with this patch, mozilla
can't use a printer with Japanese name on Win2k/XP with the default locale set
to French while MS IE can. I'll work on this later.
Attachment #143155 - Flags: superreview?(jshin)

Comment 44

13 years ago
Attachment 100602 [details] [diff] and attachment 103347 [details] [diff] [review] have that problem too.
Will you fix them?
I once tryed writing patch that is uncomely...
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=2025&action=view

Comment 45

13 years ago
Yes, I'm gonna take care of it as well. 

BTW, it'd be nice if you can replace nsString with nsAutoString here:

@@ -198,11 +201,7 @@
   LPTSTR lpPrtName;
   GlobalPrinters::GetInstance()->GetDefaultPrinterName(lpPrtName);
   nsString str;      <===== here 

Comment 46

13 years ago
Created attachment 143708 [details] [diff] [review]
for unicode on NT

More usefull(?) patch but still not complete.
Comment 45 is not dealed too.

Comment 47

13 years ago
Created attachment 149755 [details] [diff] [review]
path it woks for me both XP and 98SE

This patch have problem
1st There are alike code in MapPaperSizeToNativeEnumA and
+MapPaperSizeToNativeEnumW. I don't know how optimize this patch.
2nd in 98SE assertion occur in gfx/src/windows/nsDeviceContextSpecWin.cpp # 867
Attachment #143155 - Attachment is obsolete: true
Attachment #143708 - Attachment is obsolete: true

Comment 48

13 years ago
Comment on attachment 149755 [details] [diff] [review]
path it woks for me both XP and 98SE

I want reviewer comment.
It works for me.
CJK people want this fix...
Attachment #149755 - Flags: review?(jshin)

Comment 49

13 years ago
Instead of cluttering the tree with two branches of code (for Win9x/ME and
Win2k/XP), we're discussing the possibility of either making Mozilla depend on
MSLU or rolling out our own version of MSLU (namely, 'MZLU' that stands for
Mozilla layer for Unicode). See bug 239279. 
Depends on: 239279

Comment 50

13 years ago
I'm very sorry for holding this up. I'm sure attachment 149755 [details] [diff] [review] does work, but
let's put off supporting printers whose name has characters outside the current
locale for the now. We'll be able to fix it more cleanly once bug 239279 is fixed. 

Instead, let's just go with the trivial fix (attachment 143155 [details] [diff] [review]). 

Comment 51

13 years ago
Comment on attachment 143155 [details] [diff] [review]
convert make well

r=jshin

I'll replace |nsString str| with |nsAutoString str| when checking it in.

Asking for sr and a1.7. This is a trivial fix for  a typical 'misinflation' of
strings in a legacy encoding to UTF-16 I should have reviewed much earlier.
Attachment #143155 - Attachment is obsolete: false
Attachment #143155 - Flags: superreview?(mkaply)
Attachment #143155 - Flags: review?(john)
Attachment #143155 - Flags: review+
Attachment #143155 - Flags: approval1.7.1?

Comment 52

13 years ago
*** Bug 169206 has been marked as a duplicate of this bug. ***

Comment 53

13 years ago
Comment on attachment 149755 [details] [diff] [review]
path it woks for me both XP and 98SE

remove 'r'. once bug 239279 is fixed, we won't have to clutter our code like
this any more.
Attachment #149755 - Flags: review?(jshin)

Comment 54

13 years ago
Comment on attachment 143155 [details] [diff] [review]
convert make well

a=mkaply for 1.7
I'm not rechnically an sr, but this is a platform specific patch, so I am OK
with it.
Attachment #143155 - Flags: superreview?(mkaply)
Attachment #143155 - Flags: superreview+
Attachment #143155 - Flags: approval1.7.1?
Attachment #143155 - Flags: approval1.7.1+

Comment 55

13 years ago
attachment 143155 [details] [diff] [review] replaced |nsString str| with |nsAutoString str|
works well of course in 98SE-ja and XP-ja.
It looks good.

Comment 56

13 years ago
Thanks for sr and a. (I'm sorry I was mistaken you to be able to sr :-))

fixed on the trunk as well as on 1.7branch. I'm keeping this open for printers
whose name include characters not covered by the current system locale. 
Status: NEW → ASSIGNED
Whiteboard: [adt2] [ETA Needed] → [adt2] [ETA Needed],needs-aviary-1.0
Comment on attachment 143155 [details] [diff] [review]
convert make well

>-#ifdef UNICODE
>-  str.AppendWithConversion((PRUnichar *)lpPrtName);
>-#else 
>-  str.AssignWithConversion((char*)lpPrtName);
>-#endif
>+  NS_CopyNativeToUnicode(nsDependentCString((char *)lpPrtName), str);


if |lpPrtName| is *ever* in an encoding like UTF-16 where one byte can be 0 in
the middle of the string, then this patch is incorrect because the length
calculation performed by nsDependentCString's constructor is incorrect for such
use.

Comment 58

13 years ago
(In reply to comment #57)

> if |lpPrtName| is *ever* in an encoding like UTF-16 where one byte can be 0 in

  It's NEVER NEVER in UTF-16 because we use Win32 'A' API (GetProfileString) to
retrieve the value.

Comment 59

13 years ago
(In reply to comment #54)
> (From update of attachment 143155 [details] [diff] [review])
> a=mkaply for 1.7
> I'm not rechnically an sr, but this is a platform specific patch, so I am OK
> with it.
> 

AVIARY_1_0_20040515_BRANCH does not checkin yet.

Updated

13 years ago
Flags: blocking-aviary1.0RC1?
Whiteboard: [adt2] [ETA Needed],needs-aviary-1.0 → [adt2] [ETA Needed],needs-aviary-1.0,needed-aviary1.0

Comment 60

13 years ago
the latest patch is now checked into the aviary 1.0 branch per jshin
Whiteboard: [adt2] [ETA Needed],needs-aviary-1.0,needed-aviary1.0 → fixed-aviary1.0

Comment 61

13 years ago
(In reply to comment #56)

> fixed on the trunk as well as on 1.7branch. I'm keeping this open for printers
> whose name include characters not covered by the current system locale. 

I filed bug 250255 for this and similar problems. Resolving as fixed.

Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago13 years ago
Resolution: --- → FIXED

Updated

13 years ago
Flags: blocking-aviary1.0PR?

Updated

13 years ago
Keywords: fixed1.7
You need to log in before you can comment on or make changes to this bug.