[FIX]Japanese printing does not work at all

VERIFIED FIXED in mozilla0.9.9

Status

()

Core
Printing: Output
--
critical
VERIFIED FIXED
17 years ago
4 years ago

People

(Reporter: Masaki Katakai, Assigned: rods (gone))

Tracking

Trunk
mozilla0.9.9
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

17 years ago
Japanese printing does not work at all on nightly.
All japanese characters are drawn as rectangle.
(Reporter)

Comment 1

17 years ago
At nsPostScriptObj.cpp 1.65 changing, pref name has been
changed but we need to change the length for Cut().

Found a problem, preferences have been changed, but argument
has not been changed.

   if (strstr(aName, "print.postscript.nativefont.")) {
-    lang.Cut(0, 19);
+    lang.Cut(0, 28);
   } else if (strstr(aName, "print.postscript.unicodefont.")) {
-    lang.Cut(0, 20);
+    lang.Cut(0, 29);
   }

Is it better to use strlen() to determine the length?
(Reporter)

Comment 2

17 years ago
Created attachment 70678 [details] [diff] [review]
proposed path
(Reporter)

Comment 3

17 years ago
Rods, can you take this?
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Keywords: nsbeta1
Summary: Japanese printing does not work at all → [FIX]Japanese printing does not work at all
Target Milestone: --- → mozilla0.9.9
(Assignee)

Updated

17 years ago
Attachment #70678 - Flags: review+
Marking nsbeta1+
Keywords: nsbeta1 → nsbeta1+

Comment 5

17 years ago
Comment on attachment 70678 [details] [diff] [review]
proposed path

This is lame.  We already see what happens when you use hard-coded numbers -
things break.  Please at least add an assertion that the magic number used
equals the strlen of the string used.  With that check for correctness, the
next change probably won't cause printing to break.  sr=attinasi assuming that
change is made.
Attachment #70678 - Flags: superreview+

Comment 6

17 years ago
I think NS_NAMED_LITERAL_CSTRING would be more to my liking, if you can wait a
bit I'd like to suggest a code block.

One, can someone please explain why we're using strstr instead of strncmp?

Ok, so here are my suggestions: 

NS_NAMED_LITERAL_CSTRING(psNativeFont, "print.postscript.nativefont.");
  if (strncmp(aName, psNativeFont.get(), psNativeFont.Length())
    lang.Cut(0, psNativeFont.Length());
  }
or.
NS_NAMED_LITERAL_CSTRING(psNativeFont, "print.postscript.nativefont.");
  if (psNativeFont.Equals(Substring(aName, aName+psNativeFont.Length())))
    lang.Cut(0, psNativeFont.Length());
  }

(Assignee)

Comment 7

17 years ago
Created attachment 70757 [details] [diff] [review]
new patch

With Marc's suggestion
Attachment #70678 - Attachment is obsolete: true
(Assignee)

Comment 8

17 years ago
Created attachment 70759 [details] [diff] [review]
final patch

70757 is wrong, this is the good patch
Attachment #70757 - Attachment is obsolete: true
Comment on attachment 70759 [details] [diff] [review]
final patch

It doesn't look like this second patch has review yet, so I'll add
some review-ish comments.

>+// NOTE: If you change either of these two strings, 
>+//       you MUST change the strings in the struct just below this!

This could be automated, e.g., by making a macro for the strings

#define NATIVE_FONT_PREFIX "print.postscript.nativefont."

static const char kNativeFontPrefix[] = NATIVE_FONT_PREFIX; /* needed at all?
*/

{NATIVE_FONT_PREFIX "ja", "Ryumin-Light-EUC-H"},

>+static const char* kNativeFontPrefix  = "print.postscript.nativefont.";

static const char kNativeFontPrefix[] = "...";

would be preferred since it isn't an lvalue (less storage space,
more potential for compiler optimization).

>+  if (strstr(aName, kNativeFontPrefix)) {
>+    lang.Cut(0, strlen(kNativeFontPrefix));
>+
>+  } else if (strstr(aName, kUnicodeFontPrefix)) {
>+    lang.Cut(0, strlen(kUnicodeFontPrefix));

timeless's suggestion for strncmp here seems sound.  It also seems
odd to use strlen (a run-time cost) when you can use (*provided* that
you use the array syntax I mention in my previous comment or use the
literal or a macro that expands to the literal) sizeof(array)-1,
perhaps through a macro (|#define STRLEN(s) (sizeof(s)-1)|) or through
NS_LITERAL_CSTRING.
Maybe you really want these strings to be macros rather than constants?

>   nsStringKey key(lang);

It seems like it would be a bit nicer if these keys were
nsCStringKey (you could convert at the other site, after using GetUnicode.
Then you could use |operator+| with an nsAutoCString (or even
nsDependentCString) when calling the nsAutoCString constructors below.
But that's a bigger change than needed right now.

Then again, the current patch will fix the problem, so if you really
want to just get it in, r=dbaron, although I really don't like the
|strlen| calls.
Attachment #70759 - Flags: review+
(Assignee)

Comment 10

17 years ago
Created attachment 70809 [details] [diff] [review]
patch

using dbaron's and timeless' suggestions.
Attachment #70759 - Attachment is obsolete: true
Comment on attachment 70809 [details] [diff] [review]
patch

>+    lang.Cut(0, psNativeFont.Length()-1);

Shouldn't this just be Length() rather than Length()-1?

Other than that, r=dbaron.
Attachment #70809 - Flags: review+
(Assignee)

Comment 12

17 years ago
Actually not, I took the snipet of code and compiled and ran it on Windows and I
discovered that I needed the -1 to get "lang" to equal to "ja"

Comment 13

17 years ago
Looks like over-engineering to me.  All I wanted was an assertion to prevent
this problem from happening again...
(Assignee)

Comment 14

17 years ago
I have the patch ready to go, thanks Marc.

Comment 15

17 years ago
a=asa (on behalf of drivers) for checkin to 0.9.9
Keywords: mozilla0.9.9+

Comment 16

17 years ago
i'm pretty sure you could put the named_literal_string outside of function 
scope to use it throughout the file.  my snippet wasn't intended to show 
absolute correct usage, merely how the pieces could be used.
(Assignee)

Comment 17

17 years ago
fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 18

17 years ago
Katakai, please verify this bug and mark verified-fixed...thanks!
(Reporter)

Comment 19

17 years ago
Verified. Thanks!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.