Closed Bug 152521 Opened 22 years ago Closed 22 years ago

XIM is disabled by bad font lookup

Categories

(Core :: Internationalization, defect)

Sun
Solaris
defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: masaki.katakai, Assigned: masaki.katakai)

Details

(Keywords: fixedOEM, inputmethod, intl)

Attachments

(1 file, 3 obsolete files)

I got a report that XIM does not work at all on Solaris 8 ru.ansi locale
and ru.koi8-r locales.

gdk_fontset_load("-*-*-medium-r-*-*-16-*-*-*-*-*-*-*");

does not match ansi fonts and koi8-r fonts at all in those
locales. Actually these locales are providing such fonts
but there is not -medium- fonts.

In earlier, we used "-*-*-*-*-*-*-16-*-*-*-*-*-*" for lookup
and changed to "medium-r-*-*-16" because italic/bold fonts
are picked up.

We should try more pattern as the following order
here if the first lookup is failed,

-*-*-medium-r-*-*-16-*-*-*-*-*-*-*
-*-*-*-r-*-*-16-*-*-*-*-*-*-*
-*-*-*-*-*-*-16-*-*-*-*-*-*-*
Attached patch patch (obsolete) — Splinter Review
Frank, could you r= please?
Status: NEW → ASSIGNED
Keywords: intl
Another way is to provide 

"-*-*-medium-r-*-*-%d-*-*-*-*-*-*-*,-*-*-*-r-*-*-%d-*-*-*-*
-*-*-*,-*-*-*-*-*-*-%d-*-*-*-*-*-*-*"

to gdk_fontset_locad() in patch 88088.

Which is better?

Blizzard, could you please give suggestion? and
r=/sr= please?


Blizzard, could you take a look?
hmm, I don't actually have an opinion here.
Attachment #88088 - Attachment is obsolete: true
Thanks Blizzard,

attachment 88235 [details] [diff] [review] seems smarter.

Could you please r=/sr= please?
Attachment #88235 - Flags: review+
Comment on attachment 88235 [details] [diff] [review]
another patch using "*,*,*" form for gdk_fontset_load()

katakai:
> @@ -3583,7 +3585,7 @@
>      gdk_font_unref(gPreeditFontset);
>    }
>    char xlfdbase[128];
> -  sprintf(xlfdbase, "-*-*-medium-r-*-*-%d-*-*-*-*-*-*-*", height);
> +  sprintf(xlfdbase, XIC_FONTSET, height, height, height);

Nit: Could you add a comment here that the number of |height| parameters must
match the |XIC_FONTSET| string (and the ref above the |#define XIC_FONTSET ...|
to adjust the |sprintf()|-call on demand ?), please ?

r=Roland.Mainz@informatik.med.uni-giessen.de
Attached patch add comments (obsolete) — Splinter Review
Attachment #88235 - Attachment is obsolete: true
Thanks, Roland. I added the comments for number of arguments.
Attachment #91330 - Flags: review+
Can someone give sr= for the patch please?  ftang@netscape.com?
yokoyama@netscape.com?  cc yokoyama@netscape.com as well.  Thanks.
Margaret: none of us (ftang,yokoyama) have /sr privilage.  

kin: can you /sr?  I can't think of anyone else. (Solaris/XIM).......
Comment on attachment 91330 [details] [diff] [review]
add comments

Sorry for the delay, I was on vacation all last week.

The changes look ok to me ... but with my security hat on, the fact that we are
calling sprintf with a 128 byte buffer (xlfdbase), a formatting string
(XIC_FONTSET) which is already 94 characters long and expects 3 ints, makes me
wonder if the buffers need to be a bit larger?

On a 32 bit system an int looks like it can contain a number that is 11 chars
long (10 digits plus a sign). 3 of those numbers would put us up to the limit,
but what about 64 bit systems.

I'm not sure where we're getting the heights we're using in the sprintfs()
from, or if their even exploitable.

Just thought I'd ask. :-)
Attached patch use PR_smprintfSplinter Review
Attachment #91330 - Attachment is obsolete: true
Hi Kin, thanks for comment.

Sure. now I use PR_smprintf().

Could you sr= please?
Comment on attachment 95230 [details] [diff] [review]
use PR_smprintf

sr=blizzard
Attachment #95230 - Flags: superreview+
Thank you very much for review, blizzard and Roland.

Patch checked in to Trunk.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: branchOEM
I am marking this bug as VERIFIED. 
Since we don't have a Sun/Solaris here to verify it, please reopen if anything 
wrong.
Status: RESOLVED → VERIFIED
Whiteboard: branchOEM → branchOEM+
patch checked in to OEM branch.
Whiteboard: branchOEM+ → branchOEM+,fixedOEM
Whiteboard: branchOEM+,fixedOEM → fixedOEM
Keywords: fixedOEM
Whiteboard: fixedOEM
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: