Closed
Bug 100324
Opened 23 years ago
Closed 23 years ago
(patch) hack to detemine width of cyrillic characters
Categories
(Core :: Printing: Output, defect)
Tracking
()
VERIFIED
FIXED
mozilla0.9.9
People
(Reporter: ggromov, Assigned: dcone)
References
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
dcone
:
review+
attinasi
:
superreview+
|
Details | Diff | Splinter Review |
Here is a patch (hack) to define a width of cyrillic characters.
It's much better than nothing.
---
The patch is created by ALTlinux team (www.altlinux.ru) and is used
in their package of mozilla.
Their SRPMs with this patch included are publically distributed since
spring 2001,
from ftp://ftp.altlinux.ru/pub/distributions/ALTLinux/Sisyphus/SRPMS/
(NOTE: their ftp supports passive mode only!)
Please give credit to them for their wonderfull patches somewhere in
release notes and/or documentation, if possible.
Reporter | ||
Comment 1•23 years ago
|
||
Updated•23 years ago
|
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.8
Comment 3•23 years ago
|
||
HELP! Somebody is going to have to get me a new patch, thanks.
Reporter | ||
Comment 4•23 years ago
|
||
Hi! What do you mean?
I just checked the latest version of their patch, it seems to be
the same:
diff -ur mozilla.orig/gfx/src/ps/nsAFMObject.cpp mozilla/gfx/src/ps/nsAFMObject.cpp
--- mozilla.orig/gfx/src/ps/nsAFMObject.cpp Sat Dec 16 17:59:47 2000
+++ mozilla/gfx/src/ps/nsAFMObject.cpp Sat Dec 16 18:08:36 2000
@@ -776,8 +776,10 @@
fwidth = (PRInt32)(mPSFontInfo->mAFMCharMetrics[idx].mW0x);
// if ( (*cptr == 0x0020) || (*cptr == 0x002c) )
// printf("fwidth = %d\n", fwidth);
- if (*cptr & 0xff00)
- fwidth = 1056;
+ if (*cptr & 0xff00)
+ fwidth = 1056;
+ if (*cptr & 0x0400) //cyrillic
+ fwidth = 600; // hack!!!
if ( (*cptr == 0x0020) || (*cptr == 0x002c) )
fwidth = 1056; // space and comma are half size of a CJK width
totallen += fwidth;
Or did you mean anything else?
BTW, why target you set is 0.9.8, why not 0.9.7? :)
Comment 5•23 years ago
|
||
I don't like that exact patch. The patch will not only impact Cyrillic. I think
we can work out a better patch here.
I think the whole block is wrong.
We should not use if (*cptr & 0x0400) //cyrillic
for cyrillic it should be
if ((*cptr & 0xff00) == 0x0400) //cyrillic
if (*cptr & 0x0400)
will match more than 0x0400 - 0x04ff it will ALSO match any other unicode that
bit is on.
if ((*cptr & 0xff00) == 0x0400) will only match cyrillic.
Also the origional code may cause crash ( idx may < 0)
I will propose a better patch, But I need someone to help me to test
the patch is coming.
Comment 6•23 years ago
|
||
Comment 7•23 years ago
|
||
the hack could be better if we can take
http://www.unicode.org/Public/3.1-Update1/EastAsianWidth-5.txt and cacular width
depend on the unicode property (
half width for non CJK, full width for CJK , zero width for combining mark.
read http://www.unicode.org/unicode/reports/tr11/ for details. But I think we
can do that improvement later. file bug 119265 for that. Let's not blocking this
patch untill that got fix. (that will take longer time)
Comment 8•23 years ago
|
||
rods, jshin, katakai and Gena, can you test my patch ?
I think jshin and katakai cover the testing of Japaense, Chinese and Korean and
Gena can cover Cyrillic and western. Rods can cover western.
Thanks
Comment 9•23 years ago
|
||
I think my patch should also STOP a crashing when
(*cptr) is euqal to 0x0100 - 0x011f, 0x0200 - 0x021f,0x0300 - 0x031f.. 0xff00 -
0xff1f where idx is 0xFFFFFFE0-0xFFFFFFF
Comment 10•23 years ago
|
||
I've tried the patch and I could not see the crash issue.
However, Japanese EUC 0xa7a0-0xa7f2 are converted to \u04??
so the behavior will be changed by this patch. Not sure which
is correct.
Updated•23 years ago
|
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Reporter | ||
Comment 11•23 years ago
|
||
Frank, your version of the patch works fine with Cyrillic and western. Thank you
very much for working on it!
Comment 12•23 years ago
|
||
Don, since this is PS I'll let you handle it.
Assignee: rods → dcone
Status: ASSIGNED → NEW
Comment 13•23 years ago
|
||
> However, Japanese EUC 0xa7a0-0xa7f2 are converted to \u04??
> so the behavior will be changed by this patch. Not sure which
> is correct.
The same is true of any encodings based on KS X 1001 (KS C 5601).
I also think that's the case of SC and TC encodings that include
Cyrillic letters. However, I tend to believe that this is an
acceptable fallback until EastAsianWidth is handled by Mozilla
as specified in UTR #11.
BTW, Greek alphabets in 0x370-0x37F should be treated
the same way as Cyrillic alphabets, shouldn't they?
Comment 15•23 years ago
|
||
dcone: will you land this in m0.9.9? if you want me to land it, then please r=
it and reassign it to me.
Assignee | ||
Comment 16•23 years ago
|
||
Comment on attachment 64326 [details] [diff] [review]
better patch also may stop a crash
r=dcone
Attachment #64326 -
Flags: review+
Comment 17•23 years ago
|
||
Comment on attachment 64326 [details] [diff] [review]
better patch also may stop a crash
sr=attinasi
Attachment #64326 -
Flags: superreview+
Comment 18•23 years ago
|
||
if (0x0400 == (*cptr & 0xff00)) { // Cyrillic
Although this patch is for Cyrillic, wouldn't it better
to take care of Greek as well by replacing the above line
with the following?
if (0x370 <= *cptr && *cptr <= 0x04ff) { // Greek and Cyrillic
Or, am I missing something here (i.e. Greek alphabets
are taken care of somewhere else)? In that case,
here goes my apology.
Assignee | ||
Comment 19•23 years ago
|
||
Fixed
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 20•23 years ago
|
||
Gena or someone on the cc: list, can you please verify this bug
and mark it VERIFIED-FIXED please? thanks!
Comment 21•23 years ago
|
||
Just for the log:
I've cleaned-up the postscript module prefs which involves an name change of the
prefs.
All postscript-module specific prefs start now with "print.postscript.".
Example:
"print.psnativefont.ja" --> "print.postscript.nativefont.ja"
"print.psnativecode.ja" --> "print.postscript.nativecode.ja"
Some (the others will follow, but I did not have enougth time to do more
cleanup&&testing) pref items like "print_command" can now be set in a per
printer-specific manner, e.g.
|pref("print.postscript.printer_foobar.print_command", "psview /dev/stdin");|
sets the print command for the printer "foobar" only to "psview /dev/stdin",
overriding the default in unix.js ...
Reporter | ||
Comment 22•23 years ago
|
||
marking verified-fixed.
Thanks for fixing this issue!
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•