Last Comment Bug 167128 - [FIX]Japanese named printer cannot be recognized.
: [FIX]Japanese named printer cannot be recognized.
Status: RESOLVED FIXED
fixed-aviary1.0
: fixed1.7, intl, qawanted
Product: Core
Classification: Components
Component: Printing: Output (show other bugs)
: Trunk
: x86 Windows 98
P2 normal (vote)
: Future
Assigned To: John Keiser (jkeiser)
: Yuying Long
: Jet Villegas (:jet)
Mentors:
: 169206 187399 (view as bug list)
Depends on: mzlu
Blocks:
  Show dependency treegraph
 
Reported: 2002-09-06 10:38 PDT by Frank Tang
Modified: 2004-08-27 07:18 PDT (History)
25 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.60 KB, patch)
2002-09-09 11:39 PDT, rods (gone)
no flags Details | Diff | Splinter Review
revised patch v2 (2.17 KB, patch)
2002-09-11 07:35 PDT, rods (gone)
blizzard: superreview+
Details | Diff | Splinter Review
patch v3 (3.39 KB, patch)
2002-09-25 12:30 PDT, rods (gone)
tetsuroy: review+
blizzard: superreview+
Details | Diff | Splinter Review
patch for the 1.0.2 tree (3.40 KB, patch)
2002-10-18 10:50 PDT, rods (gone)
dcone: review+
blizzard: superreview+
rjesup: approval+
Details | Diff | Splinter Review
make convert correct (605 bytes, patch)
2004-03-05 22:50 PST, Hidehiro Kozawa
no flags Details | Diff | Splinter Review
convert make well (754 bytes, patch)
2004-03-06 18:33 PST, Hidehiro Kozawa
jshin1987: review+
mozilla: superreview+
mozilla: approval1.7.5+
Details | Diff | Splinter Review
for unicode on NT (12.00 KB, patch)
2004-03-12 04:55 PST, Hidehiro Kozawa
no flags Details | Diff | Splinter Review
path it woks for me both XP and 98SE (13.82 KB, patch)
2004-06-01 07:25 PDT, Hidehiro Kozawa
no flags Details | Diff | Splinter Review

Description User image Frank Tang 2002-09-06 10:38:57 PDT
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 ?
Comment 1 User image Pete Zha 2002-09-08 19:05:47 PDT
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 User image Pete Zha 2002-09-08 19:26:54 PDT
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 User image Louie Zhao 2002-09-08 19:40:16 PDT
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.
Comment 5 User image rods (gone) 2002-09-09 11:39:17 PDT
Created attachment 98462 [details] [diff] [review]
patch 

Uses the GetACPString, to get the properly localized string.
Comment 6 User image Roy Yokoyama 2002-09-09 15:02:50 PDT
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
   }
Comment 7 User image rods (gone) 2002-09-11 07:35:29 PDT
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.
Comment 8 User image Christopher Blizzard (:blizzard) 2002-09-23 09:07:26 PDT
Comment on attachment 98716 [details] [diff] [review]
revised patch v2

sr=blizzard
Comment 9 User image Roy Yokoyama 2002-09-23 12:17:04 PDT
How about prName?  Do we need to convert to UCS2 by calling MultiByteToWideChar()?
Comment 10 User image rods (gone) 2002-09-25 12:30:51 PDT
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.
Comment 11 User image Roy Yokoyama 2002-09-25 13:19:38 PDT
Comment on attachment 100602 [details] [diff] [review]
patch v3

looks good.
/r=yokoyama
Comment 12 User image Christopher Blizzard (:blizzard) 2002-09-25 14:28:34 PDT
Comment on attachment 100602 [details] [diff] [review]
patch v3

sr=blizzard
Comment 13 User image rods (gone) 2002-09-27 08:20:18 PDT
fixed

Note this fixed Bug 169400 which is an issue for 7.0 may want to take this in
the respin)
Comment 14 User image Michael Buckland 2002-09-30 12:59:48 PDT
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.


Comment 15 User image Kevin McCluskey (gone) 2002-10-18 10:43:47 PDT
nsbeta1+
Comment 16 User image rods (gone) 2002-10-18 10:50:07 PDT
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 17 User image Christopher Blizzard (:blizzard) 2002-10-23 14:23:51 PDT
Comment on attachment 103347 [details] [diff] [review]
patch for the 1.0.2 tree

sr=blizzard
Comment 18 User image dcone (gone) 2002-11-05 06:35:55 PST
Comment on attachment 103347 [details] [diff] [review]
patch for the 1.0.2 tree

r=dcone
Comment 19 User image Randell Jesup [:jesup] 2002-11-05 12:51:22 PST
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
Comment 20 User image sujay 2002-11-06 10:07:55 PST
Can someone from International please verify this and mark.

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

thanks.
Comment 21 User image Yuying Long 2002-11-06 10:27:02 PST
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 User image Yuying Long 2002-11-07 11:54:25 PST
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 User image rods (gone) 2002-11-07 15:02:03 PST
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 User image Yuying Long 2002-11-07 15:12:53 PST
> 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 User image Gayatri Rimola 2002-11-12 09:34:09 PST
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 User image Yuying Long 2002-11-12 10:21:09 PST
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.
Comment 27 User image rods (gone) 2002-11-12 10:27:51 PST
I can back the change out and we can test again?
Comment 28 User image rods (gone) 2002-11-13 06:38:09 PST
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 User image Yuying Long 2002-11-13 11:00:15 PST
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.
Comment 30 User image Kevin McCluskey (gone) 2003-01-22 17:11:19 PST
-> jkeiser
Comment 31 User image Kevin McCluskey (gone) 2003-01-23 16:39:40 PST
nsbeta1-. John is overloaded with higher priority issues. 

Adding qawanted to try out a current build on a "real Japanese network printer"
Comment 32 User image Prognathous 2003-10-28 09:54:13 PST
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.
Comment 33 User image Prognathous 2003-12-18 04:30:38 PST
*** Bug 187399 has been marked as a duplicate of this bug. ***
Comment 34 User image Hidehiro Kozawa 2004-02-18 06:28:32 PST
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 User image Hidehiro Kozawa 2004-03-05 22:50:33 PST
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 User image Hidehiro Kozawa 2004-03-05 22:57:55 PST
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 User image Jungshik Shin 2004-03-05 23:39:42 PST
(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 User image Hidehiro Kozawa 2004-03-06 18:33:34 PST
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.
Comment 39 User image Hidehiro Kozawa 2004-03-09 05:22:21 PST
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 User image Hidehiro Kozawa 2004-03-11 05:28:24 PST
Comment on attachment 143155 [details] [diff] [review]
convert make well

Please review it
Comment 41 User image Hidehiro Kozawa 2004-03-11 05:30:53 PST
Comment on attachment 143155 [details] [diff] [review]
convert make well

Please review it
Comment 42 User image noririty 2004-03-11 05:55:18 PST
Comment on attachment 143155 [details] [diff] [review]
convert make well

It also have I18N aspects. Jshin:
Could you mind sr?
Comment 43 User image Jungshik Shin 2004-03-11 06:35:04 PST
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.
Comment 44 User image Hidehiro Kozawa 2004-03-11 06:47:15 PST
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 User image Jungshik Shin 2004-03-11 07:03:07 PST
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 User image Hidehiro Kozawa 2004-03-12 04:55:36 PST
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 User image Hidehiro Kozawa 2004-06-01 07:25:25 PDT
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
Comment 48 User image Hidehiro Kozawa 2004-06-01 07:32:58 PDT
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...
Comment 49 User image Jungshik Shin 2004-06-22 17:59:25 PDT
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. 
Comment 50 User image Jungshik Shin 2004-06-24 18:54:20 PDT
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 User image Jungshik Shin 2004-06-24 18:59:51 PDT
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.
Comment 52 User image Hidehiro Kozawa 2004-06-25 18:01:51 PDT
*** Bug 169206 has been marked as a duplicate of this bug. ***
Comment 53 User image Jungshik Shin 2004-06-28 23:45:33 PDT
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.
Comment 54 User image Mike Kaply [:mkaply] 2004-06-29 05:17:17 PDT
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.
Comment 55 User image Hidehiro Kozawa 2004-06-29 07:19:06 PDT
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 User image Jungshik Shin 2004-06-29 07:53:28 PDT
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. 
Comment 57 User image David Baron :dbaron: ⌚️UTC-8 2004-06-29 11:27:49 PDT
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 User image Jungshik Shin 2004-06-29 17:38:51 PDT
(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 User image Hiro 2004-07-02 16:26:54 PDT
(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.
Comment 60 User image Scott MacGregor 2004-07-06 23:42:35 PDT
the latest patch is now checked into the aviary 1.0 branch per jshin
Comment 61 User image Jungshik Shin 2004-07-07 13:37:45 PDT
(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.


Note You need to log in before you can comment on or make changes to this bug.