Last Comment Bug 288634 - non-ASCII font support in Gfx:Pango and port a couple of other changes in Gfx:Xft to Pango
: non-ASCII font support in Gfx:Pango and port a couple of other changes in Gfx...
Status: RESOLVED FIXED
: fixed1.8, intl
Product: Core Graveyard
Classification: Graveyard
Component: GFX: Gtk (show other bugs)
: Trunk
: x86 Linux
: -- normal with 1 vote (vote)
: ---
Assigned To: Jungshik Shin
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-04-01 06:55 PST by Jungshik Shin
Modified: 2009-01-22 10:17 PST (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (12.29 KB, patch)
2005-04-01 18:48 PST, Jungshik Shin
dbaron: superreview+
Details | Diff | Review
patch updated to the trunk (11.77 KB, patch)
2005-07-23 01:52 PDT, Jungshik Shin
jshin1987: superreview+
Details | Diff | Review
patch without 'xft' (12.90 KB, patch)
2005-07-28 05:43 PDT, Jungshik Shin
caillon: review+
jshin1987: superreview+
asa: approval1.8b5+
Details | Diff | Review

Description Jungshik Shin 2005-04-01 06:55:18 PST
Since the fork of Gfx:Pango, there have been a few changes made in Gfx:Xft that
need to ported to Pango Gfx. I'm building a pango build with my patch. After
testing it, I'll upload a patch here.
Comment 1 Jungshik Shin 2005-04-01 18:48:24 PST
Created attachment 179348 [details] [diff] [review]
patch

I got rid of some duplicated code (nsFontConfigUtils.* are compiled in without
this patch so that it's worth to use functions there instead of duping them in
nsFontMetricsPango.cpp).
As for non-ASCII font name support, somehow it doesn't work. Pango may have to
be updated as well? 
I haven't yet ported 'font-list' support.
Comment 2 Jungshik Shin 2005-04-05 03:39:24 PDT
Comment on attachment 179348 [details] [diff] [review]
patch

>-    if (!IsASCIIFontName(aFamily))
>-        return PR_TRUE;
>+    NS_ConvertUTF16toUTF8 name(aFamily);
> 
>-    nsCAutoString name;
>+    // The newest fontconfig does the full Unicode case folding so that 
>+    // we're being lazy here by calling |ToLowerCase| after converting
>+    // to UTF-8  assuming that in virtually all cases, we just have to
>+    // fold [A-Z].  (bug 223653). 
>     name.AssignWithConversion(aFamily.get());

I forgot to remove the above line (|name.AssignWithConversion...|, which is why
fonts with the native names didn't work. With the above line removed, the test
case in bug 223653 gets rendered as it should be.

Asking for r/sr with the above line removed.
Comment 3 Jungshik Shin 2005-06-26 10:59:14 PDT
Comment on attachment 179348 [details] [diff] [review]
patch

trying bryner
Comment 4 Jungshik Shin 2005-07-23 01:52:28 PDT
Created attachment 190231 [details] [diff] [review]
patch updated to the trunk

updated to the trunk. there's a little conflict with the patch for bug 300491
(not in a line changed by this patch but in a line included in the patch as a
'context': pango_xft_get_context -> gdk_pango_context_get). I'll take care of
it if this goes after the patch for bug 300491.
Comment 5 Christopher Aillon (sabbatical, not receiving bugmail) 2005-07-26 10:58:12 PDT
ack, looks like the other patch landed, and we want to not share the "xft" name
if possible.  jshin, can you get an updated patch with the xft-isms removed?
Comment 6 Jungshik Shin 2005-07-28 05:43:53 PDT
Created attachment 190828 [details] [diff] [review]
patch without 'xft'

I replaced 'Xft' with 'Gtk'.
Comment 7 Christian :Biesinger (don't email me, ping me on IRC) 2005-07-28 06:04:43 PDT
Comment on attachment 190828 [details] [diff] [review]
patch without 'xft'

+    NS_ConvertUTF16toUTF8 name(aFamily);

-    nsCAutoString name;
+    // The newest fontconfig does the full Unicode case folding so that 
+    // we're being lazy here by calling |ToLowerCase| after converting
+    // to UTF-8  assuming that in virtually all cases, we just have to
+    // fold [A-Z].  (bug 223653). 
     name.AssignWithConversion(aFamily.get());

do you really want that AssignWithConversion here?
(EnumFontCallback)
Comment 8 Jungshik Shin 2005-07-28 07:02:13 PDT
(In reply to comment #7)
> (From update of attachment 190828 [details] [diff] [review] [edit])
> +    NS_ConvertUTF16toUTF8 name(aFamily);
> 
> -    nsCAutoString name;

>      name.AssignWithConversion(aFamily.get());
> 
> do you really want that AssignWithConversion here?

That line should have been removed. Thanks. I fixed that mistake as I wrote in
comment #2, but I resurrected it by applying attachment 179348 [details] [diff] [review] to the fresh cvs
copy. 


Comment 9 Jungshik Shin 2005-08-21 17:21:15 PDT
ping caillon, can you review it? I've already fixed, in my local tree, what cbie
pointed out.
Comment 10 Christopher Aillon (sabbatical, not receiving bugmail) 2005-08-29 20:08:52 PDT
Comment on attachment 190828 [details] [diff] [review]
patch without 'xft'

r=caillon, sorry for the delay
Comment 11 Jungshik Shin 2005-09-12 04:42:38 PDT
Comment on attachment 190828 [details] [diff] [review]
patch without 'xft'

asking for a 1.8b5

this is a patch for non-default build (pango build), but RedHat ships a
pango-enabled build in FC/REL so that we'd better fix it before 1.5. It's
rather safe (porting what's been available in our default build for a while to
Pango build)
Comment 12 Asa Dotzler [:asa] 2005-09-12 16:36:47 PDT
Comment on attachment 190828 [details] [diff] [review]
patch without 'xft'

what's the risk to non-pango builds here?
Comment 13 Jungshik Shin 2005-09-15 23:16:34 PDT
fixed on both 1.8 branch (today) and trunk (Sept. 4th)

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